addons: integrate workers with native addons

src/node.cc

@@ -2668,11 +2668,28 @@ node_module* get_linked_module(const char* name) {
   return FindModule(modlist_linked, name, NM_F_LINKED);
 }
 
-struct DLib {
+namespace {
+
+Mutex dlib_mutex;
+
+struct DLib;
+
+std::unordered_map<std::string, std::shared_ptr<DLib>> dlopen_cache;
+std::unordered_map<decltype(uv_lib_t().handle), std::shared_ptr<DLib>>
+    handle_to_dlib;
+
+struct DLib : public std::enable_shared_from_this<DLib> {
   std::string filename_;
   std::string errmsg_;
-  void* handle_;
+  void* handle_ = nullptr;
   int flags_;
+  std::unordered_set<Environment*> users_;
+  node_module* own_info = nullptr;
+
+  DLib() {}
+  ~DLib() {
+    Close();
+  }
 
 #ifdef __POSIX__
   static const int kDefaultFlags = RTLD_LAZY;
@@ -2708,90 +2725,160 @@ struct DLib {
     uv_dlclose(&lib_);
   }
 #endif  // !__POSIX__
+
+  DLib(const DLib& other) = delete;
+  DLib(DLib&& other) = delete;
+  DLib& operator=(const DLib& other) = delete;
+  DLib& operator=(DLib&& other) = delete;
+
+  void 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<DLib> info;
+      Environment* env;
+    };
+    env->AddCleanupHook([](void* arg) {
+      Mutex::ScopedLock lock(dlib_mutex);
+      cleanup_hook_data* cbdata = static_cast<cleanup_hook_data*>(arg);
+      std::shared_ptr<DLib> info = cbdata->info;
+      info->users_.erase(cbdata->env);
+      delete cbdata;
+      if (info->users_.empty()) {
+        std::vector<std::string> 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<void*>(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<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
+  node_module* mp;
+  std::shared_ptr<DLib> dlib;
+  Local<Object> module = args[0]->ToObject(env->isolate());
 
-  CHECK_EQ(modpending, nullptr);
+  do {
+    Mutex::ScopedLock lock(dlib_mutex);
+    CHECK_EQ(modpending, nullptr);
 
-  if (args.Length() < 2) {
-    env->ThrowError("process.dlopen needs at least 2 arguments.");
-    return;
-  }
+    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(env->context()).To(&flags)) {
-    return env->ThrowTypeError("flag argument must be an integer.");
-  }
+    int32_t flags = DLib::kDefaultFlags;
+    if (args.Length() > 2 && !args[2]->Int32Value(env->context()).To(&flags)) {
+      return env->ThrowTypeError("flag argument must be an integer.");
+    }
 
-  Local<Object> module = args[0]->ToObject(env->isolate());  // Cast
-  node::Utf8Value filename(env->isolate(), args[1]);  // Cast
-  DLib dlib;
-  dlib.filename_ = *filename;
-  dlib.flags_ = flags;
-  bool is_opened = dlib.Open();
+    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>();
+    dlib->filename_ = *filename;
+    dlib->flags_ = flags;
+    bool is_opened = dlib->Open();
+
+    if (is_opened && handle_to_dlib.count(dlib->handle_) > 0) {
+      dlib = handle_to_dlib[dlib->handle_];
+      mp = dlib->own_info;
+      dlib->AddEnvironment(env);
+      break;
+    }
 
-  // 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 = modpending;
-  modpending = nullptr;
+    // 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 = modpending;
+    modpending = nullptr;
 
-  if (!is_opened) {
-    Local<String> errmsg = OneByteString(env->isolate(), dlib.errmsg_.c_str());
-    dlib.Close();
+    if (!is_opened) {
+      Local<String> errmsg =
+          OneByteString(env->isolate(), dlib->errmsg_.c_str());
 #ifdef _WIN32
-    // Windows needs to add the filename into the error message
-    errmsg = String::Concat(errmsg, args[1]->ToString(env->isolate()));
+      // Windows needs to add the filename into the error message
+      errmsg = String::Concat(errmsg, args[1]->ToString(env->isolate()));
 #endif  // _WIN32
-    env->isolate()->ThrowException(Exception::Error(errmsg));
-    return;
-  }
+      env->isolate()->ThrowException(Exception::Error(errmsg));
+      return;
+    }
 
-  if (mp == nullptr) {
-    dlib.Close();
-    env->ThrowError("Module did not self-register.");
-    return;
-  }
-  if (mp->nm_version == -1) {
-    if (env->EmitNapiWarning()) {
-      ProcessEmitWarning(env, "N-API is an experimental feature and could "
-                         "change at any time.");
+    if (mp == nullptr) {
+      env->ThrowError("Module did not self-register.");
+      return;
     }
-  } else if (mp->nm_version != NODE_MODULE_VERSION) {
-    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;
-  }
+    if (mp->nm_version == -1) {
+      if (env->EmitNapiWarning()) {
+        ProcessEmitWarning(env, "N-API is an experimental feature and could "
+                           "change at any time.");
+      }
+    } else if (mp->nm_version != NODE_MODULE_VERSION) {
+      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;
+    }
+
+    if (mp->nm_flags & NM_F_BUILTIN) {
+      env->ThrowError("Built-in module self-registered.");
+      return;
+    }
+
+    if (!env->is_main_thread() && !(mp->nm_flags & NM_F_WORKER_ENABLED)) {
+      env->ThrowError("Native modules need to explicitly indicate multi-"
+                      "isolate/multi-thread support by using "
+                      "`NODE_MODULE_WORKER_ENABLED` or "
+                      "`NAPI_MODULE_WORKER_ENABLED` to register themselves.");
+      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;
+    mp->nm_dso_handle = dlib->handle_;
+    mp->nm_link = modlist_addon;
+    modlist_addon = mp;
+  } while (false);
 
   Local<String> exports_string = env->exports_string();
   Local<Object> exports = module->Get(exports_string)->ToObject(env->isolate());
@@ -2801,7 +2888,6 @@ static 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();
     env->ThrowError("Module has no declared entry point.");
     return;
   }

src/node.h

@@ -445,6 +445,7 @@ typedef void (*addon_context_register_func)(
 #define NM_F_BUILTIN   0x01
 #define NM_F_LINKED    0x02
 #define NM_F_INTERNAL  0x04
+#define NM_F_WORKER_ENABLED  0x400
 
 struct node_module {
   int nm_version;
@@ -532,6 +533,10 @@ extern "C" NODE_EXTERN void node_module_register(void* mod);
 #define NODE_MODULE_CONTEXT_AWARE_BUILTIN(modname, regfunc)           \
   NODE_MODULE_CONTEXT_AWARE_X(modname, regfunc, NULL, NM_F_BUILTIN)   \
 
+#define NODE_MODULE_WORKER_ENABLED(modname, regfunc)                  \
+  NODE_MODULE_CONTEXT_AWARE_X(modname, regfunc, NULL,                 \
+                              NM_F_WORKER_ENABLED)
+
 /*
  * For backward compatibility in add-on modules.
  */

src/node_api.cc

@@ -856,6 +856,8 @@ void napi_module_register_cb(v8::Local<v8::Object> exports,
 
 // Registers a NAPI module.
 void napi_module_register(napi_module* mod) {
+  static_assert(NAPI_F_WORKER_ENABLED == NM_F_WORKER_ENABLED,
+                "Worker-enabled flags match for N-API and Node");
   int module_version = -1;
 #ifdef EXTERNAL_NAPI
   module_version = NODE_MODULE_VERSION;

src/node_api.h

@@ -80,6 +80,8 @@ typedef struct {
 #define EXTERN_C_END
 #endif
 
+#define NAPI_F_WORKER_ENABLED 0x400
+
 #define NAPI_MODULE_X(modname, regfunc, priv, flags)                  \
   EXTERN_C_START                                                      \
     static napi_module _module =                                      \
@@ -100,6 +102,9 @@ typedef struct {
 #define NAPI_MODULE(modname, regfunc) \
   NAPI_MODULE_X(modname, regfunc, NULL, 0)
 
+#define NAPI_MODULE_WORKER_ENABLED(modname, regfunc) \
+  NAPI_MODULE_X(modname, regfunc, NULL, NAPI_F_WORKER_ENABLED)
+
 #define NAPI_AUTO_LENGTH SIZE_MAX
 
 EXTERN_C_START

test/addons/dlopen-ping-pong/test.js

@@ -14,7 +14,4 @@ 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);
+assert.doesNotThrow(() => require(`./build/${common.buildType}/binding`));

test/addons/hello-world/test-worker.js

@@ -0,0 +1,15 @@
+'use strict';
+const common = require('../../common');
+const assert = require('assert');
+const path = require('path');
+const { Worker } = require('worker');
+const binding = path.resolve(__dirname, `./build/${common.buildType}/binding`);
+
+const w = new Worker(`require(${JSON.stringify(binding)});`, { eval: true });
+w.on('error', common.mustCall((err) => {
+  assert(String(err).includes(
+    'Error: Native modules need to explicitly indicate ' +
+    'multi-isolate/multi-thread support by using ' +
+    '`NODE_MODULE_WORKER_ENABLED` or `NAPI_MODULE_WORKER_ENABLED` to ' +
+    'register themselves.'));
+}));

test/addons/worker-addon/binding.cc

@@ -0,0 +1,43 @@
+#include <assert.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <node.h>
+#include <v8.h>
+
+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<const char*>(str));
+}
+
+void Init(Local<Object> exports, Local<Value> module, Local<Context> context) {
+  node::AddEnvironmentCleanupHook(
+      context->GetIsolate(), Cleanup,
+      const_cast<void*>(static_cast<const void*>("cleanup")));
+  node::AddEnvironmentCleanupHook(context->GetIsolate(), Dummy, nullptr);
+  node::RemoveEnvironmentCleanupHook(context->GetIsolate(), Dummy, nullptr);
+}
+
+NODE_MODULE_WORKER_ENABLED(binding, Init)

test/addons/worker-addon/binding.gyp

@@ -0,0 +1,9 @@
+{
+  'targets': [
+    {
+      'target_name': 'binding',
+      'defines': [ 'V8_DEPRECATION_WARNINGS=1' ],
+      'sources': [ 'binding.cc' ]
+    }
+  ]
+}