Skip to content

Commit

Permalink
worker: perform initial port.unref() before preload modules
Browse files Browse the repository at this point in the history
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: #31777

PR-URL: #33455
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
  • Loading branch information
addaleax authored and codebytere committed Jun 18, 2020
1 parent 7d1d00f commit 9c30080
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lib/internal/main/worker_thread.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ if (process.env.NODE_CHANNEL_FD) {

port.on('message', (message) => {
if (message.type === LOAD_SCRIPT) {
port.unref();
const {
argv,
cwdCounter,
Expand Down Expand Up @@ -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');
Expand Down
20 changes: 20 additions & 0 deletions test/parallel/test-worker-stdio-from-preload-module.js
Original file line number Diff line number Diff line change
@@ -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');
}));
}

0 comments on commit 9c30080

Please sign in to comment.