From d5cf51dad589dffed2f15cc3a49d19ae13968b8f Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 10 Jun 2019 16:48:15 +0200 Subject: [PATCH] =?UTF-8?q?worker:=20only=20unref=20port=20for=20stdin=20i?= =?UTF-8?q?f=20we=20ref=E2=80=99ed=20it=20before?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We set the `kStartedReading` flag from `_read()` for Worker stdio, and then `ref()` the port. However, the `.on('end')` handler is also attached when `._read()` is not called, e.g. when `process.stdin` inside a Worker is prematurely ended because stdin was not enabled by the parent thread. In that case, we should not call `.unref()` for stdin if we did not also call `.ref()` for it before. Fixes: https://github.com/nodejs/node/issues/28144 PR-URL: https://github.com/nodejs/node/pull/28153 Reviewed-By: Colin Ihrig Reviewed-By: Rich Trott Reviewed-By: Ruben Bridgewater --- lib/internal/worker/io.js | 6 ++++-- ...test-worker-no-stdin-stdout-interaction.js | 20 +++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-worker-no-stdin-stdout-interaction.js diff --git a/lib/internal/worker/io.js b/lib/internal/worker/io.js index 63c8a8d9763220..ba2150e530b5be 100644 --- a/lib/internal/worker/io.js +++ b/lib/internal/worker/io.js @@ -169,8 +169,10 @@ class ReadableWorkerStdio extends Readable { this[kIncrementsPortRef] = true; this[kStartedReading] = false; this.on('end', () => { - if (this[kIncrementsPortRef] && --this[kPort][kWaitingStreams] === 0) - this[kPort].unref(); + if (this[kStartedReading] && this[kIncrementsPortRef]) { + if (--this[kPort][kWaitingStreams] === 0) + this[kPort].unref(); + } }); } diff --git a/test/parallel/test-worker-no-stdin-stdout-interaction.js b/test/parallel/test-worker-no-stdin-stdout-interaction.js new file mode 100644 index 00000000000000..6febbd07b566fa --- /dev/null +++ b/test/parallel/test-worker-no-stdin-stdout-interaction.js @@ -0,0 +1,20 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const { Worker, isMainThread } = require('worker_threads'); + +// Regression test for https://github.com/nodejs/node/issues/28144. + +if (isMainThread) { + const w = new Worker(__filename); + w.on('exit', common.mustCall((status) => { + assert.strictEqual(status, 0); + })); + w.stdout.on('data', common.mustCall(10)); +} else { + process.stdin.on('data', () => {}); + + for (let i = 0; i < 10; ++i) { + process.stdout.write(`processing(${i})\n`, common.mustCall()); + } +}