worker: improve integration with native addons

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: https://github.com/nodejs/node/pull/23319

PR-URL: https://github.com/nodejs/node/pull/26175
Fixes: https://github.com/nodejs/node/issues/21481
Fixes: https://github.com/nodejs/node/issues/21783
Fixes: https://github.com/nodejs/node/issues/25662
Fixes: https://github.com/nodejs/node/issues/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>

6 files changed, 88 insertions, 10 deletions

diff --git a/test/addons/dlopen-ping-pong/test-worker.js b/test/addons/dlopen-ping-pong/test-worker.js
new file mode 100644
index 0000000000..feba6aa5eb
--- /dev/null
+++ b/test/addons/dlopen-ping-pong/test-worker.js
@@ -0,0 +1,20 @@
+'use strict';
+const common = require('../../common');
+
+if (common.isWindows)
+  common.skip('dlopen global symbol loading is not supported on this os.');
+
+const assert = require('assert');
+const { Worker } = require('worker_threads');
+
+// Check that modules that are not declared as context-aware cannot be re-loaded
+// from workers.
+
+const bindingPath = require.resolve(`./build/${common.buildType}/binding`);
+require(bindingPath);
+
+new Worker(`require(${JSON.stringify(bindingPath)})`, { eval: true })
+  .on('error', common.mustCall((err) => {
+    assert.strictEqual(err.constructor, Error);
+    assert.strictEqual(err.message, 'Module did not self-register.');
+  }));
diff --git a/test/addons/worker-addon/binding.cc b/test/addons/worker-addon/binding.cc
index faf3ba8647..01c857c43e 100644
--- a/test/addons/worker-addon/binding.cc
+++ b/test/addons/worker-addon/binding.cc
@@ -21,7 +21,7 @@ struct statically_allocated {
   }
   ~statically_allocated() {
     assert(count == 0);
-    printf("dtor");
+    printf("dtor ");
   }
 } var;
 
diff --git a/test/addons/worker-addon/test.js b/test/addons/worker-addon/test.js
index 7fb7c1a87a..ef158e98b1 100644
--- a/test/addons/worker-addon/test.js
+++ b/test/addons/worker-addon/test.js
@@ -1,3 +1,4 @@
+// Flags: --experimental-report
 'use strict';
 const common = require('../../common');
 const assert = require('assert');
@@ -6,21 +7,62 @@ const path = require('path');
 const { Worker } = require('worker_threads');
 const binding = path.resolve(__dirname, `./build/${common.buildType}/binding`);
 
-if (process.argv[2] === 'worker') {
-  new Worker(`require(${JSON.stringify(binding)});`, { eval: true });
-  return;
-} else if (process.argv[2] === 'main-thread') {
-  process.env.addExtraItemToEventLoop = 'yes';
-  require(binding);
-  return;
+switch (process.argv[2]) {
+  case 'both':
+    require(binding);
+    // fallthrough
+  case 'worker-twice':
+  case 'worker':
+    const worker = new Worker(`require(${JSON.stringify(binding)});`, {
+      eval: true
+    });
+    if (process.argv[2] === 'worker-twice') {
+      worker.on('exit', common.mustCall(() => {
+        new Worker(`require(${JSON.stringify(binding)});`, {
+          eval: true
+        });
+      }));
+    }
+    return;
+  case 'main-thread':
+    process.env.addExtraItemToEventLoop = 'yes';
+    require(binding);
+    return;
 }
 
-for (const test of ['worker', 'main-thread']) {
+// Use process.report to figure out if we might be running under musl libc.
+const glibc = JSON.parse(process.report.getReport()).header.glibcVersionRuntime;
+assert(typeof glibc === 'string' || glibc === undefined, glibc);
+
+const libcMayBeMusl = common.isLinux && glibc === undefined;
+
+for (const { test, expected } of [
+  { test: 'worker', expected: [ 'ctor cleanup dtor ' ] },
+  { test: 'main-thread', expected: [ 'ctor cleanup dtor ' ] },
+  // We always only have 1 instance of the shared object in memory, so
+  // 1 ctor and 1 dtor call. If we attach the module to 2 Environments,
+  // we expect 2 cleanup calls, otherwise one.
+  { test: 'both', expected: [ 'ctor cleanup cleanup dtor ' ] },
+  {
+    test: 'worker-twice',
+    // In this case, we load and unload an addon, then load and unload again.
+    // musl doesn't support unloading, so the output may be missing
+    // a dtor + ctor pair.
+    expected: [
+      'ctor cleanup dtor ctor cleanup dtor '
+    ].concat(libcMayBeMusl ? [
+      'ctor cleanup cleanup dtor ',
+    ] : [])
+  },
+]) {
+  console.log('spawning test', test);
   const proc = child_process.spawnSync(process.execPath, [
     __filename,
     test
   ]);
+  process.stderr.write(proc.stderr.toString());
   assert.strictEqual(proc.stderr.toString(), '');
-  assert.strictEqual(proc.stdout.toString(), 'ctor cleanup dtor');
+  assert(expected.includes(proc.stdout.toString()),
+         `${proc.stdout.toString()} is not included in ${expected}`);
   assert.strictEqual(proc.status, 0);
 }
diff --git a/test/node-api/1_hello_world/test.js b/test/node-api/1_hello_world/test.js
index d1d67d34f6..dd28e26a56 100644
--- a/test/node-api/1_hello_world/test.js
+++ b/test/node-api/1_hello_world/test.js
@@ -1,6 +1,8 @@
 'use strict';
 const common = require('../../common');
 const assert = require('assert');
+const { Worker } = require('worker_threads');
+
 const bindingPath = require.resolve(`./build/${common.buildType}/binding`);
 const binding = require(bindingPath);
 assert.strictEqual(binding.hello(), 'world');
@@ -11,3 +13,10 @@ delete require.cache[bindingPath];
 const rebinding = require(bindingPath);
 assert.strictEqual(rebinding.hello(), 'world');
 assert.notStrictEqual(binding.hello, rebinding.hello);
+
+// Test that workers can load addons declared using NAPI_MODULE_INIT().
+new Worker(`
+const { parentPort } = require('worker_threads');
+const msg = require(${JSON.stringify(bindingPath)}).hello();
+parentPort.postMessage(msg)`, { eval: true })
+  .on('message', common.mustCall((msg) => assert.strictEqual(msg, 'world')));
diff --git a/test/node-api/test_worker_terminate/test.js b/test/node-api/test_worker_terminate/test.js
index 2bfaab8e8e..7c7224c073 100644
--- a/test/node-api/test_worker_terminate/test.js
+++ b/test/node-api/test_worker_terminate/test.js
@@ -4,6 +4,11 @@ const assert = require('assert');
 const { Worker, isMainThread, workerData } = require('worker_threads');
 
 if (isMainThread) {
+  // Load the addon in the main thread first.
+  // This checks that N-API addons can be loaded from multiple contexts
+  // when they are not loaded through NAPI_MODULE().
+  require(`./build/${common.buildType}/test_worker_terminate`);
+
   const counter = new Int32Array(new SharedArrayBuffer(4));
   const worker = new Worker(__filename, { workerData: { counter } });
   worker.on('exit', common.mustCall(() => {
diff --git a/test/node-api/test_worker_terminate/test_worker_terminate.c b/test/node-api/test_worker_terminate/test_worker_terminate.c
index a88d936293..517cae4203 100644
--- a/test/node-api/test_worker_terminate/test_worker_terminate.c
+++ b/test/node-api/test_worker_terminate/test_worker_terminate.c
@@ -36,4 +36,6 @@ napi_value Init(napi_env env, napi_value exports) {
   return exports;
 }
 
+// Do not start using NAPI_MODULE_INIT() here, so that we can test
+// compatibility of Workers with NAPI_MODULE().
 NAPI_MODULE(NODE_GYP_MODULE_NAME, Init)