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: #5037 Reviewed-By: James M Snell <[email protected]>
nodejs · Feb 11, 2016 · 6c468df · 6c468df
1 parent 018e4e0
commit 6c468df
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 2 deletions.
diff --git a/lib/child_process.js b/lib/child_process.js
@@ -1094,8 +1094,11 @@ util.inherits(ChildProcess, EventEmitter);
 function flushStdio(subprocess) {
   if (subprocess.stdio == null) return;
   subprocess.stdio.forEach(function(stream, fd, stdio) {
-    if (!stream || !stream.readable || stream._consuming ||
-        stream._readableState.flowing)
+    if (!stream ||
+        !stream.readable ||
+        stream._consuming ||
+        stream._readableState.flowing ||
+        stream._readableState.readableListening)
       return;
     stream.resume();
   });

diff --git a/test/simple/test-child-process-flush-stdio.js b/test/simple/test-child-process-flush-stdio.js
@@ -0,0 +1,17 @@
+'use strict';
+var cp = require('child_process');
+var common = require('../common');
+var assert = require('assert');
+
+var buffer = [];
+var 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() {
+  var buf;
+  while (buf = this.read())
+    buffer.push(buf);
+});