diff --git a/src/node.cc b/src/node.cc index 6250acfd5d540f..6cb4dc6d11b8b8 100644 --- a/src/node.cc +++ b/src/node.cc @@ -1135,7 +1135,17 @@ node_module* get_linked_module(const char* name) { return FindModule(modlist_linked, name, NM_F_LINKED); } -class DLib { +namespace { + +Mutex dlib_mutex; + +class DLib; + +std::unordered_map> dlopen_cache; +std::unordered_map> + handle_to_dlib; + +class DLib : public std::enable_shared_from_this { public: #ifdef __POSIX__ static const int kDefaultFlags = RTLD_LAZY; @@ -1146,17 +1156,27 @@ class DLib { inline DLib(const char* filename, int flags) : filename_(filename), flags_(flags), handle_(nullptr) {} + inline DLib() {} + inline ~DLib() { + Close(); + } + inline bool Open(); inline void Close(); inline void* GetSymbolAddress(const char* name); + inline void AddEnvironment(Environment* env); - const std::string filename_; - const int flags_; + std::string filename_; + int flags_; std::string errmsg_; - void* handle_; + void* handle_ = nullptr; + std::unordered_set users_; + node_module* own_info = nullptr; + #ifndef __POSIX__ uv_lib_t lib_; #endif + private: DISALLOW_COPY_AND_ASSIGN(DLib); }; @@ -1225,18 +1245,49 @@ void InitModpendingOnce() { CHECK_EQ(0, uv_key_create(&thread_local_modpending)); } +void DLib::AddEnvironment(Environment* env) { + if (users_.count(env) > 0) return; + users_.insert(env); + if (env->is_main_thread()) return; + struct cleanup_hook_data { + std::shared_ptr info; + Environment* env; + }; + env->AddCleanupHook([](void* arg) { + Mutex::ScopedLock lock(dlib_mutex); + std::unique_ptr cbdata( + static_cast(arg)); + std::shared_ptr info = cbdata->info; + info->users_.erase(cbdata->env); + + if (info->users_.empty()) { + // No more Environments left using this DLib -> remove filename from + // caches. + std::vector filenames; + + for (const auto& entry : dlopen_cache) { + if (entry.second == info) + filenames.push_back(entry.first); + } + for (const std::string& filename : filenames) + dlopen_cache.erase(filename); + + handle_to_dlib.erase(info->handle_); + } + }, static_cast(new cleanup_hook_data { shared_from_this(), env })); +} + +} // anonymous namespace + // DLOpen is process.dlopen(module, filename, flags). // Used to load 'module.node' dynamically shared objects. -// -// FIXME(bnoordhuis) Not multi-context ready. TBD how to resolve the conflict -// when two contexts try to load the same shared object. Maybe have a shadow -// cache that's a plain C list or hash table that's shared across contexts? static void DLOpen(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - auto context = env->context(); - uv_once(&init_modpending_once, InitModpendingOnce); - CHECK_NULL(uv_key_get(&thread_local_modpending)); + + Local context = env->context(); + node_module* mp; + std::shared_ptr dlib; if (args.Length() < 2) { env->ThrowError("process.dlopen needs at least 2 arguments."); @@ -1257,83 +1308,128 @@ static void DLOpen(const FunctionCallbackInfo& args) { return; // Exception pending. } - node::Utf8Value filename(env->isolate(), args[1]); // Cast - DLib dlib(*filename, flags); - bool is_opened = dlib.Open(); + // Use a do { ... } while(false) loop to be able to break out early + // if a cached entry was found. + do { + CHECK_NULL(uv_key_get(&thread_local_modpending)); + Mutex::ScopedLock lock(dlib_mutex); + + if (args.Length() < 2) { + env->ThrowError("process.dlopen needs at least 2 arguments."); + return; + } + + int32_t flags = DLib::kDefaultFlags; + if (args.Length() > 2 && !args[2]->Int32Value(context).To(&flags)) { + return env->ThrowTypeError("flag argument must be an integer."); + } + + node::Utf8Value filename(env->isolate(), args[1]); // Cast + auto it = dlopen_cache.find(*filename); + + if (it != dlopen_cache.end()) { + dlib = it->second; + mp = dlib->own_info; + dlib->AddEnvironment(env); + break; + } + + dlib = std::make_shared(); + dlib->filename_ = *filename; + dlib->flags_ = flags; + 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 = static_cast( - uv_key_get(&thread_local_modpending)); - uv_key_set(&thread_local_modpending, nullptr); + if (is_opened && handle_to_dlib.count(dlib->handle_) > 0) { + dlib = handle_to_dlib[dlib->handle_]; + mp = dlib->own_info; + dlib->AddEnvironment(env); + break; + } - if (!is_opened) { - Local errmsg = OneByteString(env->isolate(), dlib.errmsg_.c_str()); - dlib.Close(); + // 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. + mp = static_cast(uv_key_get(&thread_local_modpending)); + uv_key_set(&thread_local_modpending, nullptr); + + if (!is_opened) { + Local errmsg = + OneByteString(env->isolate(), dlib->errmsg_.c_str()); #ifdef _WIN32 - // Windows needs to add the filename into the error message - errmsg = String::Concat( - env->isolate(), errmsg, args[1]->ToString(context).ToLocalChecked()); + // 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; - } + env->isolate()->ThrowException(Exception::Error(errmsg)); + return; + } - if (mp == nullptr) { - if (auto callback = GetInitializerCallback(&dlib)) { - callback(exports, module, context); - } else if (auto napi_callback = GetNapiInitializerCallback(&dlib)) { - napi_module_register_by_symbol(exports, module, context, napi_callback); - } else { - dlib.Close(); - env->ThrowError("Module did not self-register."); + if (mp == nullptr) { + if (auto callback = GetInitializerCallback(dlib.get())) { + callback(exports, module, context); + } else if (auto napi_callback = GetNapiInitializerCallback(dlib.get())) { + napi_module_register_by_symbol(exports, module, context, napi_callback); + } else { + dlib->Close(); + env->ThrowError("Module did not self-register."); + } + return; } - return; - } - // -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)) { - callback(exports, module, context); + // -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.get())) { + callback(exports, module, context); + return; + } + char errmsg[1024]; + snprintf(errmsg, + sizeof(errmsg), + "The module '%s'" + "\nwas compiled against a different Node.js version using" + "\nNODE_MODULE_VERSION %d. This version of Node.js requires" + "\nNODE_MODULE_VERSION %d. Please try re-compiling or " + "re-installing\nthe module (for instance, using `npm rebuild` " + "or `npm install`).", + *filename, mp->nm_version, NODE_MODULE_VERSION); + + // NOTE: `mp` is allocated inside of the shared library's memory, + // calling `dlclose` will deallocate it + env->ThrowError(errmsg); return; } - char errmsg[1024]; - snprintf(errmsg, - sizeof(errmsg), - "The module '%s'" - "\nwas compiled against a different Node.js version using" - "\nNODE_MODULE_VERSION %d. This version of Node.js requires" - "\nNODE_MODULE_VERSION %d. Please try re-compiling or " - "re-installing\nthe module (for instance, using `npm rebuild` " - "or `npm install`).", - *filename, mp->nm_version, NODE_MODULE_VERSION); - - // NOTE: `mp` is allocated inside of the shared library's memory, calling - // `dlclose` will deallocate it - dlib.Close(); - env->ThrowError(errmsg); - return; - } - if (mp->nm_flags & NM_F_BUILTIN) { - dlib.Close(); - env->ThrowError("Built-in module self-registered."); - return; - } - mp->nm_dso_handle = dlib.handle_; - mp->nm_link = modlist_addon; - modlist_addon = mp; + if (mp->nm_flags & NM_F_BUILTIN) { + env->ThrowError("Built-in module self-registered."); + return; + } + + if (mp->nm_context_register_func == nullptr && + mp->nm_register_func == nullptr) { + env->ThrowError("Module has no declared entry point."); + return; + } + + dlib->own_info = mp; + handle_to_dlib[dlib->handle_] = dlib; + dlopen_cache[*filename] = dlib; + + dlib->AddEnvironment(env); + + mp->nm_dso_handle = dlib->handle_; + mp->nm_link = modlist_addon; + modlist_addon = mp; + } while (false); 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) { mp->nm_register_func(exports, module, mp->nm_priv); } else { - dlib.Close(); env->ThrowError("Module has no declared entry point."); return; } diff --git a/test/addons/dlopen-ping-pong/test.js b/test/addons/dlopen-ping-pong/test.js index c533593496090a..2b5c65dae9f8f6 100644 --- a/test/addons/dlopen-ping-pong/test.js +++ b/test/addons/dlopen-ping-pong/test.js @@ -14,7 +14,5 @@ process.dlopen(module, bindingPath, module.exports.load(`${path.dirname(bindingPath)}/ping.so`); assert.strictEqual(module.exports.ping(), 'pong'); -// Check that after the addon is loaded with -// process.dlopen() a require() call fails. -const re = /^Error: Module did not self-register\.$/; -assert.throws(() => require(`./build/${common.buildType}/binding`), re); +// This second `require()` call should not throw an error. +require(`./build/${common.buildType}/binding`); diff --git a/test/addons/hello-world/test-worker.js b/test/addons/hello-world/test-worker.js new file mode 100644 index 00000000000000..f989c738c873c7 --- /dev/null +++ b/test/addons/hello-world/test-worker.js @@ -0,0 +1,14 @@ +// Flags: --experimental-worker +'use strict'; +const common = require('../../common'); +const assert = require('assert'); +const path = require('path'); +const { Worker } = require('worker_threads'); +const binding = path.resolve(__dirname, `./build/${common.buildType}/binding`); + +const w = new Worker(` +require('worker_threads').parentPort.postMessage( + require(${JSON.stringify(binding)}).hello());`, { eval: true }); +w.on('message', common.mustCall((message) => { + assert.strictEqual(message, 'world'); +})); diff --git a/test/addons/worker-addon/binding.cc b/test/addons/worker-addon/binding.cc new file mode 100644 index 00000000000000..cb87a43757dfc5 --- /dev/null +++ b/test/addons/worker-addon/binding.cc @@ -0,0 +1,45 @@ +#include +#include +#include +#include +#include + +using v8::Context; +using v8::HandleScope; +using v8::Isolate; +using v8::Local; +using v8::Object; +using v8::Value; + +size_t count = 0; + +struct statically_allocated { + statically_allocated() { + assert(count == 0); + printf("ctor "); + } + ~statically_allocated() { + assert(count == 0); + printf("dtor"); + } +} var; + +void Dummy(void*) { + assert(0); +} + +void Cleanup(void* str) { + printf("%s ", static_cast(str)); +} + +void Initialize(Local exports, + Local module, + Local context) { + node::AddEnvironmentCleanupHook( + context->GetIsolate(), Cleanup, + const_cast(static_cast("cleanup"))); + node::AddEnvironmentCleanupHook(context->GetIsolate(), Dummy, nullptr); + node::RemoveEnvironmentCleanupHook(context->GetIsolate(), Dummy, nullptr); +} + +NODE_MODULE(NODE_GYP_MODULE_NAME, Initialize) diff --git a/test/addons/worker-addon/binding.gyp b/test/addons/worker-addon/binding.gyp new file mode 100644 index 00000000000000..7ede63d94a0d77 --- /dev/null +++ b/test/addons/worker-addon/binding.gyp @@ -0,0 +1,9 @@ +{ + 'targets': [ + { + 'target_name': 'binding', + 'defines': [ 'V8_DEPRECATION_WARNINGS=1' ], + 'sources': [ 'binding.cc' ] + } + ] +} diff --git a/test/addons/worker-addon/test.js b/test/addons/worker-addon/test.js new file mode 100644 index 00000000000000..bdaac8659f2459 --- /dev/null +++ b/test/addons/worker-addon/test.js @@ -0,0 +1,17 @@ +// Flags: --experimental-worker +'use strict'; +const common = require('../../common'); +const assert = require('assert'); +const child_process = require('child_process'); +const path = require('path'); +const { Worker } = require('worker_threads'); +const binding = path.resolve(__dirname, `./build/${common.buildType}/binding`); + +if (process.argv[2] === 'child') { + new Worker(`require(${JSON.stringify(binding)});`, { eval: true }); +} else { + const proc = child_process.spawnSync(process.execPath, [__filename, 'child']); + assert.strictEqual(proc.stderr.toString(), ''); + assert.strictEqual(proc.stdout.toString(), 'ctor cleanup dtor'); + assert.strictEqual(proc.status, 0); +}