From 27f6e5d38291088990c0c581573d7524f0c10bb9 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Thu, 19 Mar 2020 22:38:30 +0100 Subject: [PATCH] stream: complete pipeline with stdio stdio (stderr & stdout) should for compatibility reasons not be closed/end():ed. However, this causes pipeline with a stdio destination to never finish. This commit fixes this issue at a performance cost. Refs: https://github.com/nodejs/node/issues/7606 Fixes: https://github.com/nodejs/node/issues/32363 PR-URL: https://github.com/nodejs/node/pull/32373 Reviewed-By: James M Snell Reviewed-By: Anna Henningsen Reviewed-By: Matteo Collina --- lib/internal/streams/pipeline.js | 7 +++++ test/parallel/test-stream-pipeline-process.js | 29 +++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 test/parallel/test-stream-pipeline-process.js diff --git a/lib/internal/streams/pipeline.js b/lib/internal/streams/pipeline.js index fdba5ebc737206..cdd5bcb791f451 100644 --- a/lib/internal/streams/pipeline.js +++ b/lib/internal/streams/pipeline.js @@ -267,6 +267,13 @@ function pipeline(...streams) { } else if (isStream(stream)) { if (isReadable(ret)) { ret.pipe(stream); + + // Compat. Before node v10.12.0 stdio used to throw an error so + // pipe() did/does not end() stdio destinations. + // Now they allow it but "secretly" don't close the underlying fd. + if (stream === process.stdout || stream === process.stderr) { + ret.on('end', () => stream.end()); + } } else { ret = makeAsyncIterable(ret); diff --git a/test/parallel/test-stream-pipeline-process.js b/test/parallel/test-stream-pipeline-process.js new file mode 100644 index 00000000000000..825b4454918ddc --- /dev/null +++ b/test/parallel/test-stream-pipeline-process.js @@ -0,0 +1,29 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const os = require('os'); + +if (process.argv[2] === 'child') { + const { pipeline } = require('stream'); + pipeline( + process.stdin, + process.stdout, + common.mustCall((err) => { + assert.ifError(err); + }) + ); +} else { + const cp = require('child_process'); + cp.exec([ + 'echo', + 'hello', + '|', + `"${process.execPath}"`, + `"${__filename}"`, + 'child' + ].join(' '), common.mustCall((err, stdout) => { + assert.ifError(err); + assert.strictEqual(stdout.split(os.EOL).shift().trim(), 'hello'); + })); +}