From 9e6ad2d8ffc4e524a74edd8e804e19c2bc322166 Mon Sep 17 00:00:00 2001 From: Brian White Date: Tue, 2 Feb 2016 00:57:24 -0500 Subject: [PATCH] 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: https://github.com/nodejs/node/issues/5034 PR-URL: https://github.com/nodejs/node/pull/5036 Reviewed-By: Evan Lucas Reviewed-By: Colin Ihrig Reviewed-By: Ben Noordhuis Reviewed-By: James M Snell --- lib/internal/child_process.js | 2 +- .../parallel/test-child-process-flush-stdio.js | 18 +++++++++++++++--- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js index 1617665b521eea..1bc1c86951b5cd 100644 --- 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 index 5fd7eb3bc99922..dacc1226f30d03 100644 --- 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); + }); +}