Skip to content

Commit

Permalink
worker: improve integration with native addons
Browse files Browse the repository at this point in the history
Allow loading add-ons from multiple Node.js instances if they are
declared context-aware; in particular, this applies to N-API addons.

Also, plug a memory leak that occurred when registering N-API addons.

Refs: #23319

PR-URL: #26175
Fixes: #21481
Fixes: #21783
Fixes: #25662
Fixes: #20239
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
  • Loading branch information
addaleax committed Feb 25, 2019
1 parent 3446419 commit 07b2ee2
Show file tree
Hide file tree
Showing 11 changed files with 322 additions and 29 deletions.
5 changes: 5 additions & 0 deletions doc/api/addons.md
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,11 @@ down. If necessary, such hooks can be removed using
`RemoveEnvironmentCleanupHook()` before they are run, which has the same
signature.
In order to be loaded from multiple Node.js environments,
such as a main thread and a Worker thread, an add-on needs to either:
- Be an N-API addon, or
- Be declared as context-aware using `NODE_MODULE_INIT()` as described above
### Building
Once the source code has been written, it must be compiled into the binary
Expand Down
9 changes: 3 additions & 6 deletions doc/api/worker_threads.md
Original file line number Diff line number Diff line change
Expand Up @@ -360,11 +360,8 @@ Notable differences inside a Worker environment are:
being invoked.
- IPC channels from parent processes are not accessible.
- The [`trace_events`][] module is not supported.

Currently, the following differences also exist until they are addressed:

- The [`inspector`][] module is not available yet.
- Native addons are not supported yet.
- Native add-ons can only be loaded from multiple threads if they fulfill
[certain conditions][Addons worker support].

Creating `Worker` instances inside of other `Worker`s is possible.

Expand Down Expand Up @@ -580,7 +577,6 @@ active handle in the event system. If the worker is already `unref()`ed calling
[`WebAssembly.Module`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WebAssembly/Module
[`Worker`]: #worker_threads_class_worker
[`cluster` module]: cluster.html
[`inspector`]: inspector.html
[`port.on('message')`]: #worker_threads_event_message
[`port.postMessage()`]: #worker_threads_port_postmessage_value_transferlist
[`process.abort()`]: process.html#process_process_abort
Expand All @@ -602,6 +598,7 @@ active handle in the event system. If the worker is already `unref()`ed calling
[`worker.postMessage()`]: #worker_threads_worker_postmessage_value_transferlist
[`worker.terminate()`]: #worker_threads_worker_terminate_callback
[`worker.threadId`]: #worker_threads_worker_threadid_1
[Addons worker support]: addons.html#addons_worker_support
[HTML structured clone algorithm]: https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Structured_clone_algorithm
[Signals events]: process.html#process_signal_events
[Web Workers]: https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API
Expand Down
2 changes: 1 addition & 1 deletion src/node_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ void napi_module_register_by_symbol(v8::Local<v8::Object> exports,
void napi_module_register(napi_module* mod) {
node::node_module* nm = new node::node_module {
-1,
mod->nm_flags,
mod->nm_flags | NM_F_DELETEME,
nullptr,
mod->nm_filename,
nullptr,
Expand Down
233 changes: 221 additions & 12 deletions src/node_binding.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#include "env-inl.h"
#include "node_native_module.h"
#include "util.h"
#include <atomic>

#if HAVE_OPENSSL
#define NODE_BUILTIN_OPENSSL_MODULES(V) V(crypto) V(tls_wrap)
Expand Down Expand Up @@ -96,6 +97,130 @@
NODE_BUILTIN_MODULES(V)
#undef V

#ifdef _AIX
// On AIX, dlopen() behaves differently from other operating systems, in that
// it returns unique values from each call, rather than identical values, when
// loading the same handle.
// We try to work around that by providing wrappers for the dlopen() family of
// functions, and using st_dev and st_ino for the file that is to be loaded
// as keys for a cache.

namespace node {
namespace dlwrapper {

struct dl_wrap {
uint64_t st_dev;
uint64_t st_ino;
uint64_t refcount;
void* real_handle;

struct hash {
size_t operator()(const dl_wrap* wrap) const {
return std::hash<uint64_t>()(wrap->st_dev) ^
std::hash<uint64_t>()(wrap->st_ino);
}
};

struct equal {
bool operator()(const dl_wrap* a,
const dl_wrap* b) const {
return a->st_dev == b->st_dev && a->st_ino == b->st_ino;
}
};
};

static Mutex dlhandles_mutex;
static std::unordered_set<dl_wrap*, dl_wrap::hash, dl_wrap::equal>
dlhandles;
static thread_local std::string dlerror_storage;

char* wrapped_dlerror() {
return &dlerror_storage[0];
}

void* wrapped_dlopen(const char* filename, int flags) {
CHECK_NOT_NULL(filename); // This deviates from the 'real' dlopen().
Mutex::ScopedLock lock(dlhandles_mutex);

uv_fs_t req;
OnScopeLeave cleanup([&]() { uv_fs_req_cleanup(&req); });
int rc = uv_fs_stat(nullptr, &req, filename, nullptr);

if (rc != 0) {
dlerror_storage = uv_strerror(rc);
return nullptr;
}

dl_wrap search = {
req.statbuf.st_dev,
req.statbuf.st_ino,
0, nullptr
};

auto it = dlhandles.find(&search);
if (it != dlhandles.end()) {
(*it)->refcount++;
return *it;
}

void* real_handle = dlopen(filename, flags);
if (real_handle == nullptr) {
dlerror_storage = dlerror();
return nullptr;
}
dl_wrap* wrap = new dl_wrap();
wrap->st_dev = req.statbuf.st_dev;
wrap->st_ino = req.statbuf.st_ino;
wrap->refcount = 1;
wrap->real_handle = real_handle;
dlhandles.insert(wrap);
return wrap;
}

int wrapped_dlclose(void* handle) {
Mutex::ScopedLock lock(dlhandles_mutex);
dl_wrap* wrap = static_cast<dl_wrap*>(handle);
int ret = 0;
CHECK_GE(wrap->refcount, 1);
if (--wrap->refcount == 0) {
ret = dlclose(wrap->real_handle);
if (ret != 0) dlerror_storage = dlerror();
dlhandles.erase(wrap);
delete wrap;
}
return ret;
}

void* wrapped_dlsym(void* handle, const char* symbol) {
if (handle == RTLD_DEFAULT || handle == RTLD_NEXT)
return dlsym(handle, symbol);
dl_wrap* wrap = static_cast<dl_wrap*>(handle);
return dlsym(wrap->real_handle, symbol);
}

#define dlopen node::dlwrapper::wrapped_dlopen
#define dlerror node::dlwrapper::wrapped_dlerror
#define dlclose node::dlwrapper::wrapped_dlclose
#define dlsym node::dlwrapper::wrapped_dlsym

} // namespace dlwrapper
} // namespace node

#endif // _AIX

#ifdef __linux__
static bool libc_may_be_musl() {
static std::atomic_bool retval; // Cache the return value.
static std::atomic_bool has_cached_retval { false };
if (has_cached_retval) return retval;
retval = dlsym(RTLD_DEFAULT, "gnu_get_libc_version") == nullptr;
has_cached_retval = true;
return retval;
}
#else // __linux__
static bool libc_may_be_musl() { return false; }
#endif // __linux__

namespace node {

using v8::Context;
Expand All @@ -110,7 +235,6 @@ using v8::Value;
// Globals per process
static node_module* modlist_internal;
static node_module* modlist_linked;
static node_module* modlist_addon;
static uv_once_t init_modpending_once = UV_ONCE_INIT;
static uv_key_t thread_local_modpending;

Expand All @@ -136,6 +260,57 @@ extern "C" void node_module_register(void* m) {

namespace binding {

static struct global_handle_map_t {
public:
void set(void* handle, node_module* mod) {
CHECK_NE(handle, nullptr);
Mutex::ScopedLock lock(mutex_);

map_[handle].module = mod;
// We need to store this flag internally to avoid a chicken-and-egg problem
// during cleanup. By the time we actually use the flag's value,
// the shared object has been unloaded, and its memory would be gone,
// making it impossible to access fields of `mod` --
// unless `mod` *is* dynamically allocated, but we cannot know that
// without checking the flag.
map_[handle].wants_delete_module = mod->nm_flags & NM_F_DELETEME;
map_[handle].refcount++;
}

node_module* get_and_increase_refcount(void* handle) {
CHECK_NE(handle, nullptr);
Mutex::ScopedLock lock(mutex_);

auto it = map_.find(handle);
if (it == map_.end()) return nullptr;
it->second.refcount++;
return it->second.module;
}

void erase(void* handle) {
CHECK_NE(handle, nullptr);
Mutex::ScopedLock lock(mutex_);

auto it = map_.find(handle);
if (it == map_.end()) return;
CHECK_GE(it->second.refcount, 1);
if (--it->second.refcount == 0) {
if (it->second.wants_delete_module)
delete it->second.module;
map_.erase(handle);
}
}

private:
Mutex mutex_;
struct Entry {
unsigned int refcount;
bool wants_delete_module;
node_module* module;
};
std::unordered_map<void*, Entry> map_;
} global_handle_map;

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

Expand All @@ -149,7 +324,21 @@ bool DLib::Open() {

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

if (libc_may_be_musl()) {
// musl libc implements dlclose() as a no-op which returns 0.
// As a consequence, trying to re-load a previously closed addon at a later
// point will not call its static constructors, which Node.js uses.
// Therefore, when we may be using musl libc, we assume that the shared
// object exists indefinitely and keep it in our handle map.
return;
}

int err = dlclose(handle_);
if (err == 0) {
if (has_entry_in_global_handle_map_)
global_handle_map.erase(handle_);
}
handle_ = nullptr;
}

Expand All @@ -170,6 +359,8 @@ bool DLib::Open() {

void DLib::Close() {
if (handle_ == nullptr) return;
if (has_entry_in_global_handle_map_)
global_handle_map.erase(handle_);
uv_dlclose(&lib_);
handle_ = nullptr;
}
Expand All @@ -181,6 +372,16 @@ void* DLib::GetSymbolAddress(const char* name) {
}
#endif // !__POSIX__

void DLib::SaveInGlobalHandleMap(node_module* mp) {
has_entry_in_global_handle_map_ = true;
global_handle_map.set(handle_, mp);
}

node_module* DLib::GetSavedModuleFromGlobalHandleMap() {
has_entry_in_global_handle_map_ = true;
return global_handle_map.get_and_increase_refcount(handle_);
}

using InitializerCallback = void (*)(Local<Object> exports,
Local<Value> module,
Local<Context> context);
Expand Down Expand Up @@ -235,12 +436,15 @@ void DLOpen(const FunctionCallbackInfo<Value>& args) {

node::Utf8Value filename(env->isolate(), args[1]); // Cast
env->TryLoadAddon(*filename, flags, [&](DLib* dlib) {
static Mutex dlib_load_mutex;
Mutex::ScopedLock lock(dlib_load_mutex);

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
// module per object is supported.
node_module* const mp =
node_module* mp =
static_cast<node_module*>(uv_key_get(&thread_local_modpending));
uv_key_set(&thread_local_modpending, nullptr);

Expand All @@ -257,17 +461,24 @@ void DLOpen(const FunctionCallbackInfo<Value>& args) {
return false;
}

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

// -1 is used for N-API modules
Expand Down Expand Up @@ -300,10 +511,8 @@ void DLOpen(const FunctionCallbackInfo<Value>& args) {
}
CHECK_EQ(mp->nm_flags & NM_F_BUILTIN, 0);

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

// Do not keep the lock while running userland addon loading code.
Mutex::ScopedUnlock unlock(lock);
if (mp->nm_context_register_func != nullptr) {
mp->nm_context_register_func(exports, module, context, mp->nm_priv);
} else if (mp->nm_register_func != nullptr) {
Expand Down
4 changes: 4 additions & 0 deletions src/node_binding.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ enum {
NM_F_BUILTIN = 1 << 0, // Unused.
NM_F_LINKED = 1 << 1,
NM_F_INTERNAL = 1 << 2,
NM_F_DELETEME = 1 << 3,
};

#define NODE_MODULE_CONTEXT_AWARE_CPP(modname, regfunc, priv, flags) \
Expand Down Expand Up @@ -62,6 +63,8 @@ class DLib {
bool Open();
void Close();
void* GetSymbolAddress(const char* name);
void SaveInGlobalHandleMap(node_module* mp);
node_module* GetSavedModuleFromGlobalHandleMap();

const std::string filename_;
const int flags_;
Expand All @@ -70,6 +73,7 @@ class DLib {
#ifndef __POSIX__
uv_lib_t lib_;
#endif
bool has_entry_in_global_handle_map_ = false;

private:
DISALLOW_COPY_AND_ASSIGN(DLib);
Expand Down
Loading

0 comments on commit 07b2ee2

Please sign in to comment.