From 9c3008005d955e5e612d38c52df222e60f0d81cf Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 18 May 2020 08:55:17 +0200 Subject: [PATCH] worker: perform initial port.unref() before preload modules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The refcount of the internal communication port is relevant for stdio, but the `port.unref()` call effectively resets any `.ref()` calls happening during stdio operations happening before it. Therefore, do the `.unref()` call before loading preload modules, which may cause stdio operations. Fixes: https://github.com/nodejs/node/issues/31777 PR-URL: https://github.com/nodejs/node/pull/33455 Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Benjamin Gruenbaum Reviewed-By: Juan José Arboleda --- lib/internal/main/worker_thread.js | 2 +- .../test-worker-stdio-from-preload-module.js | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-worker-stdio-from-preload-module.js diff --git a/lib/internal/main/worker_thread.js b/lib/internal/main/worker_thread.js index ecf5d00c440a26..6e3935d1382965 100644 --- a/lib/internal/main/worker_thread.js +++ b/lib/internal/main/worker_thread.js @@ -92,6 +92,7 @@ if (process.env.NODE_CHANNEL_FD) { port.on('message', (message) => { if (message.type === LOAD_SCRIPT) { + port.unref(); const { argv, cwdCounter, @@ -145,7 +146,6 @@ port.on('message', (message) => { debug(`[${threadId}] starts worker script ${filename} ` + `(eval = ${eval}) at cwd = ${process.cwd()}`); - port.unref(); port.postMessage({ type: UP_AND_RUNNING }); if (doEval) { const { evalScript } = require('internal/process/execution'); diff --git a/test/parallel/test-worker-stdio-from-preload-module.js b/test/parallel/test-worker-stdio-from-preload-module.js new file mode 100644 index 00000000000000..e4178c58d46b21 --- /dev/null +++ b/test/parallel/test-worker-stdio-from-preload-module.js @@ -0,0 +1,20 @@ +'use strict'; +const common = require('../common'); +const fixtures = require('../common/fixtures'); +const { Worker } = require('worker_threads'); +const assert = require('assert'); + +// Regression test for https://github.com/nodejs/node/issues/31777: +// stdio operations coming from preload modules should count towards the +// ref count of the internal communication port on the Worker side. + +for (let i = 0; i < 10; i++) { + const w = new Worker('console.log("B");', { + execArgv: ['--require', fixtures.path('printA.js')], + eval: true, + stdout: true + }); + w.on('exit', common.mustCall(() => { + assert.strictEqual(w.stdout.read().toString(), 'A\nB\n'); + })); +}