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]>
nodejs · Mar 1, 2016 · 29951cf · 29951cf
1 parent 66d1115
commit 29951cf
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 4 deletions.
diff --git a/lib/internal/child_process.js b/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();
   });

diff --git a/test/parallel/test-child-process-flush-stdio.js b/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);
+  });
+}