child_process: fix data loss with readable event

This commit prevents child process stdio streams from being
automatically flushed on child process exit/close if a 'readable'
event handler has been attached at the time of exit.

Without this, child process stdio data can be lost if the process
exits quickly and a `read()` (e.g. from a 'readable' handler)
hasn't had the chance to get called yet.

Fixes: #5034
PR-URL: #5036
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>

Author: mscdex

lib/internal/child_process.js

@@ -217,7 +217,7 @@ util.inherits(ChildProcess, EventEmitter);
 function flushStdio(subprocess) {
   if (subprocess.stdio == null) return;
   subprocess.stdio.forEach(function(stream, fd, stdio) {
-    if (!stream || !stream.readable)
+    if (!stream || !stream.readable || stream._readableState.readableListening)
       return;
     stream.resume();
   });

test/parallel/test-child-process-flush-stdio.js

@@ -8,10 +8,22 @@ const p = cp.spawn('echo');
 p.on('close', common.mustCall(function(code, signal) {
   assert.strictEqual(code, 0);
   assert.strictEqual(signal, null);
+  spawnWithReadable();
 }));
 
 p.stdout.read();
 
-setTimeout(function() {
-  p.kill();
-}, 100);
+function spawnWithReadable() {
+  const buffer = [];
+  const p = cp.spawn('echo', ['123']);
+  p.on('close', common.mustCall(function(code, signal) {
+    assert.strictEqual(code, 0);
+    assert.strictEqual(signal, null);
+    assert.strictEqual(Buffer.concat(buffer).toString().trim(), '123');
+  }));
+  p.stdout.on('readable', function() {
+    let buf;
+    while (buf = this.read())
+      buffer.push(buf);
+  });
+}