addons: integrate workers with native addons

Native addons need to use flags to indicate that they are capable
of being loaded by worker threads.

Native addons are unloaded if all Environments referring to it
have been cleaned up, except if it also loaded by the main Environment.

src/node.cc

@@ -2658,11 +2658,28 @@ struct node_module* get_linked_module(const char* name) {
   return mp;
 }
 
-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;
@@ -2698,95 +2715,167 @@ 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);
 
-  // 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;
+    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) {
+      if (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<String> 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 = modpending;
+    modpending = nullptr;
+
+    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 != NODE_MODULE_VERSION) {
-    char errmsg[1024];
-    if (mp->nm_version == -1) {
-      snprintf(errmsg,
-               sizeof(errmsg),
-               "The module '%s'"
-               "\nwas compiled against the ABI-stable Node.js API (N-API)."
-               "\nThis feature is experimental and must be enabled on the "
-               "\ncommand-line by adding --napi-modules.",
-               *filename);
-    } else {
-      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);
+    if (mp == nullptr) {
+      env->ThrowError("Module did not self-register.");
+      return;
+    }
+    if (mp->nm_version != NODE_MODULE_VERSION) {
+      char errmsg[1024];
+      if (mp->nm_version == -1) {
+        snprintf(errmsg,
+                 sizeof(errmsg),
+                 "The module '%s'"
+                 "\nwas compiled against the ABI-stable Node.js API (N-API)."
+                 "\nThis feature is experimental and must be enabled on the "
+                 "\ncommand-line by adding --napi-modules.",
+                 *filename);
+      } else {
+        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;
+      }
     }
 
-    // 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_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());
@@ -2796,7 +2885,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

@@ -423,6 +423,7 @@ typedef void (*addon_context_register_func)(
 
 #define NM_F_BUILTIN 0x01
 #define NM_F_LINKED  0x02
+#define NM_F_WORKER_ENABLED  0x400
 
 struct node_module {
   int nm_version;
@@ -513,6 +514,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

@@ -861,6 +861,8 @@ void napi_module_register(napi_module* mod) {
   }
 #endif  // EXTERNAL_NAPI
 
+  static_assert(NAPI_F_WORKER_ENABLED == NM_F_WORKER_ENABLED,
+                "Worker-enabled flags match for N-API and Node");
   node::node_module* nm = new node::node_module {
     module_version,
     mod->nm_flags,

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)
+
 EXTERN_C_START
 
 NAPI_EXTERN void napi_module_register(napi_module* mod);

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

@@ -17,7 +17,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)