Skip to content

Commit

Permalink
src: unload addons when environment quits
Browse files Browse the repository at this point in the history
This is an alternative to nodejs#23319
which attaches the loaded addons to the environment and closes them
when the environment is destroyed.
  • Loading branch information
Gabriel Schulhof committed Dec 6, 2018
1 parent c991280 commit 005cbc9
Show file tree
Hide file tree
Showing 12 changed files with 154 additions and 99 deletions.
1 change: 1 addition & 0 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,7 @@
'src/node_api.h',
'src/node_api_types.h',
'src/node_binding.h',
'src/node_binding-inl.h',
'src/node_buffer.h',
'src/node_constants.h',
'src/node_context_data.h',
Expand Down
9 changes: 9 additions & 0 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,15 @@ inline uv_loop_t* Environment::event_loop() const {
return isolate_data()->event_loop();
}

inline void Environment::TryLoadAddon(const char* filename,
int flags,
std::function<bool(binding::DLib*)> was_loaded) {
loaded_addons_.emplace_back(filename, flags);
if (!was_loaded(&loaded_addons_.back())) {
loaded_addons_.pop_back();
}
}

inline Environment::AsyncHooks* Environment::async_hooks() {
return &async_hooks_;
}
Expand Down
5 changes: 5 additions & 0 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,11 @@ Environment::~Environment() {

TRACE_EVENT_NESTABLE_ASYNC_END0(
TRACING_CATEGORY_NODE1(environment), "Environment", this);

// Dereference all addons that were loaded into this environment.
for (auto& addon : loaded_addons_) {
addon.Close();
}
}

void Environment::Start(const std::vector<std::string>& args,
Expand Down
10 changes: 8 additions & 2 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,20 @@
#endif
#include "handle_wrap.h"
#include "node.h"
#include "node_binding-inl.h"
#include "node_http2_state.h"
#include "node_options.h"
#include "req_wrap.h"
#include "util.h"
#include "uv.h"
#include "v8.h"

#include <list>
#include <stdint.h>
#include <vector>
#include <functional>
#include <list>
#include <unordered_map>
#include <unordered_set>
#include <vector>

struct nghttp2_rcbuf;

Expand Down Expand Up @@ -636,6 +638,9 @@ class Environment {
inline v8::Isolate* isolate() const;
inline uv_loop_t* event_loop() const;
inline uint32_t watched_providers() const;
inline void TryLoadAddon(const char* filename,
int flags,
std::function<bool(binding::DLib*)> was_loaded);

static inline Environment* from_timer_handle(uv_timer_t* handle);
inline uv_timer_t* timer_handle();
Expand Down Expand Up @@ -921,6 +926,7 @@ class Environment {
inline void ThrowError(v8::Local<v8::Value> (*fun)(v8::Local<v8::String>),
const char* errmsg);

std::list<binding::DLib> loaded_addons_;
v8::Isolate* const isolate_;
IsolateData* const isolate_data_;
uv_timer_t timer_handle_;
Expand Down
2 changes: 1 addition & 1 deletion src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
// USE OR OTHER DEALINGS IN THE SOFTWARE.

#include "node_binding.h"
#include "node_binding-inl.h"
#include "node_buffer.h"
#include "node_constants.h"
#include "node_context_data.h"
Expand Down
2 changes: 1 addition & 1 deletion src/node_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
#define NAPI_EXPERIMENTAL
#include "js_native_api_v8.h"
#include "node_api.h"
#include "node_binding.h"
#include "node_binding-inl.h"
#include "node_errors.h"
#include "node_internals.h"

Expand Down
59 changes: 59 additions & 0 deletions src/node_binding-inl.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
#ifndef SRC_NODE_BINDING_INL_H_
#define SRC_NODE_BINDING_INL_H_

#include "node_binding.h"

namespace node {

namespace binding {

inline DLib::DLib(const char* filename, int flags):
filename_(filename), flags_(flags), handle_(nullptr) {}

#ifdef __POSIX__
inline bool DLib::Open() {
handle_ = dlopen(filename_.c_str(), flags_);
if (handle_ != nullptr) return true;
errmsg_ = dlerror();
return false;
}

inline void DLib::Close() {
if (handle_ == nullptr) return;
dlclose(handle_);
handle_ = nullptr;
}

inline void* DLib::GetSymbolAddress(const char* name) {
return dlsym(handle_, name);
}
#else // !__POSIX__
inline bool DLib::Open() {
int ret = uv_dlopen(filename_.c_str(), &lib_);
if (ret == 0) {
handle_ = static_cast<void*>(lib_.handle);
return true;
}
errmsg_ = uv_dlerror(&lib_);
uv_dlclose(&lib_);
return false;
}

inline void DLib::Close() {
if (handle_ == nullptr) return;
uv_dlclose(&lib_);
handle_ = nullptr;
}

inline void* DLib::GetSymbolAddress(const char* name) {
void* address;
if (0 == uv_dlsym(&lib_, name, &address)) return address;
return nullptr;
}
#endif // !__POSIX__

} // end of namespace binding

} // end of namespace node

#endif // SRC_NODE_BINDING_INL_H_
114 changes: 23 additions & 91 deletions src/node_binding.cc
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
#include "node_binding.h"
#include "node_binding-inl.h"
#include "node_internals.h"
#include "node_native_module.h"

#if defined(__POSIX__)
#include <dlfcn.h>
#endif

#if HAVE_OPENSSL
#define NODE_BUILTIN_OPENSSL_MODULES(V) V(crypto) V(tls_wrap)
#else
Expand Down Expand Up @@ -126,74 +122,6 @@ extern "C" void node_module_register(void* m) {

namespace binding {

class DLib {
public:
#ifdef __POSIX__
static const int kDefaultFlags = RTLD_LAZY;
#else
static const int kDefaultFlags = 0;
#endif

inline DLib(const char* filename, int flags)
: filename_(filename), flags_(flags), handle_(nullptr) {}

inline bool Open();
inline void Close();
inline void* GetSymbolAddress(const char* name);

const std::string filename_;
const int flags_;
std::string errmsg_;
void* handle_;
#ifndef __POSIX__
uv_lib_t lib_;
#endif
private:
DISALLOW_COPY_AND_ASSIGN(DLib);
};

#ifdef __POSIX__
bool DLib::Open() {
handle_ = dlopen(filename_.c_str(), flags_);
if (handle_ != nullptr) return true;
errmsg_ = dlerror();
return false;
}

void DLib::Close() {
if (handle_ == nullptr) return;
dlclose(handle_);
handle_ = nullptr;
}

void* DLib::GetSymbolAddress(const char* name) {
return dlsym(handle_, name);
}
#else // !__POSIX__
bool DLib::Open() {
int ret = uv_dlopen(filename_.c_str(), &lib_);
if (ret == 0) {
handle_ = static_cast<void*>(lib_.handle);
return true;
}
errmsg_ = uv_dlerror(&lib_);
uv_dlclose(&lib_);
return false;
}

void DLib::Close() {
if (handle_ == nullptr) return;
uv_dlclose(&lib_);
handle_ = nullptr;
}

void* DLib::GetSymbolAddress(const char* name) {
void* address;
if (0 == uv_dlsym(&lib_, name, &address)) return address;
return nullptr;
}
#endif // !__POSIX__

using InitializerCallback = void (*)(Local<Object> exports,
Local<Value> module,
Local<Context> context);
Expand Down Expand Up @@ -247,8 +175,8 @@ void DLOpen(const FunctionCallbackInfo<Value>& args) {
}

node::Utf8Value filename(env->isolate(), args[1]); // Cast
DLib dlib(*filename, flags);
bool is_opened = dlib.Open();
env->TryLoadAddon(*filename, flags, [&](DLib* dlib) {
const bool is_opened = dlib->Open();

// Objects containing v14 or later modules will have registered themselves
// on the pending list. Activate all of them now. At present, only one
Expand All @@ -258,37 +186,38 @@ void DLOpen(const FunctionCallbackInfo<Value>& args) {
uv_key_set(&thread_local_modpending, nullptr);

if (!is_opened) {
Local<String> errmsg = OneByteString(env->isolate(), dlib.errmsg_.c_str());
dlib.Close();
Local<String> errmsg = OneByteString(env->isolate(), dlib->errmsg_.c_str());
dlib->Close();
#ifdef _WIN32
// Windows needs to add the filename into the error message
errmsg = String::Concat(
env->isolate(), errmsg, args[1]->ToString(context).ToLocalChecked());
#endif // _WIN32
env->isolate()->ThrowException(Exception::Error(errmsg));
return;
return false;
}

if (mp == nullptr) {
if (auto callback = GetInitializerCallback(&dlib)) {
if (auto callback = GetInitializerCallback(dlib)) {
callback(exports, module, context);
} else if (auto napi_callback = GetNapiInitializerCallback(&dlib)) {
} else if (auto napi_callback = GetNapiInitializerCallback(dlib)) {
napi_module_register_by_symbol(exports, module, context, napi_callback);
} else {
dlib.Close();
dlib->Close();
env->ThrowError("Module did not self-register.");
return false;
}
return;
return true;
}

// -1 is used for N-API modules
if ((mp->nm_version != -1) && (mp->nm_version != NODE_MODULE_VERSION)) {
// Even if the module did self-register, it may have done so with the wrong
// version. We must only give up after having checked to see if it has an
// appropriate initializer callback.
if (auto callback = GetInitializerCallback(&dlib)) {
if (auto callback = GetInitializerCallback(dlib)) {
callback(exports, module, context);
return;
return true;
}
char errmsg[1024];
snprintf(errmsg,
Expand All @@ -305,17 +234,17 @@ void DLOpen(const FunctionCallbackInfo<Value>& args) {

// NOTE: `mp` is allocated inside of the shared library's memory, calling
// `dlclose` will deallocate it
dlib.Close();
dlib->Close();
env->ThrowError(errmsg);
return;
return false;
}
if (mp->nm_flags & NM_F_BUILTIN) {
dlib.Close();
dlib->Close();
env->ThrowError("Built-in module self-registered.");
return;
return false;
}

mp->nm_dso_handle = dlib.handle_;
mp->nm_dso_handle = dlib->handle_;
mp->nm_link = modlist_addon;
modlist_addon = mp;

Expand All @@ -324,11 +253,14 @@ void DLOpen(const FunctionCallbackInfo<Value>& args) {
} else if (mp->nm_register_func != nullptr) {
mp->nm_register_func(exports, module, mp->nm_priv);
} else {
dlib.Close();
dlib->Close();
env->ThrowError("Module has no declared entry point.");
return;
return false;
}

return true;
});

// Tell coverity that 'handle' should not be freed when we return.
// coverity[leaked_storage]
}
Expand Down
34 changes: 34 additions & 0 deletions src/node_binding.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,14 @@

#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

#if defined(__POSIX__)
#include <dlfcn.h>
#endif

#include "node.h"
#define NAPI_EXPERIMENTAL
#include "node_api.h"
#include "util.h"
#include "uv.h"
#include "v8.h"

Expand Down Expand Up @@ -46,6 +52,32 @@ extern bool node_is_initialized;

namespace binding {

class DLib {
public:
#ifdef __POSIX__
static const int kDefaultFlags = RTLD_LAZY;
#else
static const int kDefaultFlags = 0;
#endif

DLib(const char* filename, int flags);

bool Open();
void Close();
void* GetSymbolAddress(const char* name);

const std::string filename_;
const int flags_;
std::string errmsg_;
void* handle_;
#ifndef __POSIX__
uv_lib_t lib_;
#endif

private:
DISALLOW_COPY_AND_ASSIGN(DLib);
};

// Call _register<module_name> functions for all of
// the built-in modules. Because built-in modules don't
// use the __attribute__((constructor)). Need to
Expand All @@ -60,5 +92,7 @@ void DLOpen(const v8::FunctionCallbackInfo<v8::Value>& args);

} // namespace node

#include "node_binding-inl.h"

#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
#endif // SRC_NODE_BINDING_H_
Loading

0 comments on commit 005cbc9

Please sign in to comment.