stream: fix readable behavior for highWaterMark === 0

Avoid trying to emit 'readable' due to the fact that
state.length is always >= state.highWaterMark if highWaterMark is 0.
Therefore upon .read(0) call (through .on('readable')) stream assumed
that it has enough data to emit 'readable' even though
state.length === 0 instead of issuing _read(). Which led to the TTY
not recognizing that someone is waiting for the input.

Fixes: https://github.com/nodejs/node/issues/20503
Refs: https://github.com/nodejs/node/pull/18372

PR-URL: https://github.com/nodejs/node/pull/21690
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

2 files changed, 29 insertions, 0 deletions

diff --git a/test/pseudo-tty/test-readable-tty-keepalive.js b/test/pseudo-tty/test-readable-tty-keepalive.js
new file mode 100644
index 0000000000..4cf0b9fb29
--- /dev/null
+++ b/test/pseudo-tty/test-readable-tty-keepalive.js
@@ -0,0 +1,29 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+
+// This test ensures that Node.js will not ignore tty 'readable' subscribers
+// when it's the only tty subscriber and the only thing keeping event loop alive
+// https://github.com/nodejs/node/issues/20503
+
+process.stdin.setEncoding('utf8');
+
+const expectedInput = ['foo', 'bar', null];
+
+process.stdin.on('readable', common.mustCall(function() {
+  const data = process.stdin.read();
+  assert.strictEqual(data, expectedInput.shift());
+}, 3)); // first 2 data, then end
+
+process.stdin.on('end', common.mustCall());
+
+setTimeout(() => {
+  process.stdin.push('foo');
+  process.nextTick(() => {
+    process.stdin.push('bar');
+    process.nextTick(() => {
+      process.stdin.push(null);
+    });
+  });
+}, 1);
diff --git a/test/pseudo-tty/test-readable-tty-keepalive.out b/test/pseudo-tty/test-readable-tty-keepalive.out
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/test/pseudo-tty/test-readable-tty-keepalive.out