child_process: only stop readable side of stream passed to proc

Closing the underlying resource completely has the unwanted side effect
that the stream can no longer be used at all, including passing it
to other child processes.

What we want to avoid is accidentally reading from the stream;
accordingly, it should be sufficient to stop its readable side
manually, and otherwise leave the underlying resource intact.

Fixes: https://github.com/nodejs/node/issues/27097
Refs: https://github.com/nodejs/node/pull/21209

PR-URL: https://github.com/nodejs/node/pull/27373
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>

1 files changed, 17 insertions, 6 deletions

diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js
index 8e6a303a2e2..03c0ed9159f 100644
--- a/lib/internal/child_process.js
+++ b/lib/internal/child_process.js
@@ -63,6 +63,7 @@ let freeParser;
 let HTTPParser;
 
 const MAX_HANDLE_RETRANSMISSIONS = 3;
+const kIsUsedAsStdio = Symbol('kIsUsedAsStdio');
 
 // This object contain function to convert TCP objects to native handle objects
 // and back again.
@@ -278,8 +279,14 @@ function flushStdio(subprocess) {
 
   for (var i = 0; i < stdio.length; i++) {
     const stream = stdio[i];
-    if (!stream || !stream.readable || stream._readableState.readableListening)
+    // TODO(addaleax): This doesn't necessarily account for all the ways in
+    // which data can be read from a stream, e.g. being consumed on the
+    // native layer directly as a StreamBase.
+    if (!stream || !stream.readable ||
+        stream._readableState.readableListening ||
+        stream[kIsUsedAsStdio]) {
       continue;
+    }
     stream.resume();
   }
 }
@@ -384,12 +391,16 @@ ChildProcess.prototype.spawn = function(options) {
       continue;
     }
 
-    // The stream is already cloned and piped, thus close it.
+    // The stream is already cloned and piped, thus stop its readable side,
+    // otherwise we might attempt to read from the stream when at the same time
+    // the child process does.
     if (stream.type === 'wrap') {
-      stream.handle.close();
-      if (stream._stdio && stream._stdio instanceof EventEmitter) {
-        stream._stdio.emit('close');
-      }
+      stream.handle.reading = false;
+      stream.handle.readStop();
+      stream._stdio.pause();
+      stream._stdio.readableFlowing = false;
+      stream._stdio._readableState.reading = false;
+      stream._stdio[kIsUsedAsStdio] = true;
       continue;
     }