worker: make terminate() resolve for unref’ed Workers

Once `worker.terminate()` is called, the Worker instance will be destroyed as soon as possible anyway, so in order to make the Promise returned by `worker.terminate()` resolve always, it should be okay to just call `.ref()` on it and keep the main event loop alive temporarily. PR-URL: nodejs#29484 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
addaleax · Sep 9, 2019 · 2833a0d · 2833a0d
1 parent d7c5ffc
commit 2833a0d
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 0 deletions.
diff --git a/lib/internal/worker.js b/lib/internal/worker.js
@@ -226,6 +226,8 @@ class Worker extends EventEmitter {
   terminate(callback) {
     debug(`[${threadId}] terminates Worker with ID ${this.threadId}`);
 
+    this.ref();
+
     if (typeof callback === 'function') {
       process.emitWarning(
         'Passing a callback to worker.terminate() is deprecated. ' +

diff --git a/test/parallel/test-worker-terminate-unrefed.js b/test/parallel/test-worker-terminate-unrefed.js
@@ -0,0 +1,16 @@
+'use strict';
+const common = require('../common');
+const { once } = require('events');
+const { Worker } = require('worker_threads');
+
+// Test that calling worker.terminate() on an unref()’ed Worker instance
+// still resolves the returned Promise.
+
+async function test() {
+  const worker = new Worker('setTimeout(() => {}, 1000000);', { eval: true });
+  await once(worker, 'online');
+  worker.unref();
+  await worker.terminate();
+}
+
+test().then(common.mustCall());