From 5660ae267064820f15ebb7221fab37b9881bd0f9 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
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
---
 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());
+  }
+}