Skip to content

Commit

Permalink
stream: complete pipeline with stdio
Browse files Browse the repository at this point in the history
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: #7606

Fixes: #32363

PR-URL: #32373
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
  • Loading branch information
ronag authored and BethGriggs committed Apr 7, 2020
1 parent 46f060c commit 27f6e5d
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 0 deletions.
7 changes: 7 additions & 0 deletions lib/internal/streams/pipeline.js
Expand Up @@ -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);

Expand Down
29 changes: 29 additions & 0 deletions 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');
}));
}

0 comments on commit 27f6e5d

Please sign in to comment.