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: #7606 Fixes: #32363 PR-URL: #32373 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
nodejs · Apr 11, 2020 · 1c47bba · 1c47bba
1 parent 9df274a
commit 1c47bba
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 0 deletions.
diff --git a/lib/internal/streams/pipeline.js b/lib/internal/streams/pipeline.js
@@ -255,6 +255,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
@@ -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');
+  }));
+}