child_process: emit close on parent stdin when child closes fd 0

lib/internal/child_process.js

@@ -71,6 +71,7 @@ const {
   UV_ENFILE,
   UV_ENOENT,
   UV_ENOSYS,
+  UV_EOF,
   UV_ESRCH,
 } = internalBinding('uv');
 
@@ -337,6 +338,47 @@ function createSocket(pipe, readable) {
 }
 
 
+// `child.stdin` is constructed with `readable: false`, which prevents
+// `net.Socket` from starting the libuv read loop on the underlying pipe.
+// Without it the parent never observes when the child closes its end of
+// the stdin pipe, so `child.stdin` only emits 'close' once the child
+// process exits (and sometimes not at all, depending on teardown order).
+// This function restores the pre-#18701 behavior by attaching a minimal
+// EOF watcher to the pipe handle: when the kernel signals that the peer
+// (the child's fd 0) has closed, we destroy the writable side so users
+// observe a 'close' event on `child.stdin`. Refs: #25131.
+function watchStdinForPeerClose(socket, pipe) {
+  if (pipe === null || socket === null) return;
+  pipe.onread = function() {
+    const nread = streamBaseState[kReadBytesOrError];
+    if (nread === UV_EOF) {
+      // The child closed its end of the stdin pipe. End the writable
+      // side gracefully (draining any pending writes) and then destroy,
+      // so net.Socket fires 'close' on `child.stdin`.
+      if (!socket.destroyed) {
+        socket.destroySoon();
+      }
+    } else if (nread > 0) {
+      // The child is unexpectedly writing to its fd 0. Stop reading so
+      // we don't keep allocating buffers we never consume; teardown
+      // will fall back to the `onexit` handler.
+      pipe.reading = false;
+      pipe.readStop();
+    }
+    // Other read errors (negative nread other than UV_EOF) are
+    // intentionally ignored; net.Socket's normal teardown remains in
+    // charge of the rest of the lifecycle.
+  };
+  const err = pipe.readStart();
+  if (err) {
+    // If readStart fails, fall back to the previous behavior. The
+    // `onexit` handler will still destroy the socket once the child
+    // process exits.
+    pipe.onread = FunctionPrototype;
+  }
+}
+
+
 function getHandleWrapType(stream) {
   if (stream instanceof Pipe) return 'pipe';
   if (stream instanceof TTY) return 'tty';
@@ -479,6 +521,10 @@ ChildProcess.prototype.spawn = function spawn(options) {
         stream.socket.on('close', () => {
           maybeClose(this);
         });
+      } else if (i === 0 && this.pid !== 0) {
+        // Restore detection of the child closing its end of the stdin
+        // pipe so `child.stdin` emits 'close' eagerly. Refs: #25131.
+        watchStdinForPeerClose(stream.socket, stream.handle);
       }
     }
   }

test/parallel/test-child-process-stdin-close-on-exit.js

@@ -0,0 +1,73 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+'use strict';
+
+// Regression test for https://github.com/nodejs/node/issues/25131.
+//
+// Since #18701 (v8.12) the parent's `child.stdin` socket no longer emitted a
+// 'close' event when the child closed its end of the stdin pipe. The
+// 'close' event should be observable so consumers can detect that the
+// writable side of the pipe is gone without having to attempt a write and
+// wait for EPIPE. This test exercises both:
+//   1. The well-behaved case where the child simply exits without anyone
+//      touching stdin — `child.stdin` must still emit 'close'.
+//   2. The exact scenario from #25131 where a long-running child closes
+//      its own fd 0 — the parent should observe 'close' eagerly, before
+//      the child process exits.
+
+const common = require('../common');
+const { spawn } = require('child_process');
+
+// Case 1: child exits without anyone touching stdin. `child.stdin` must
+// emit 'close'.
+{
+  const child = spawn(process.execPath, ['-e', 'setTimeout(() => {}, 50)']);
+  child.stdin.on('close', common.mustCall());
+  child.on('exit', common.mustCall());
+}
+
+// Case 2: child explicitly closes its own stdin file descriptor while
+// still running. The parent should observe 'close' on `child.stdin`
+// eagerly, without waiting for the child process to exit. Skipped on
+// Windows because the named-pipe semantics there do not propagate the
+// peer close until the child process exits.
+if (!common.isWindows) {
+  const child = spawn(
+    process.execPath,
+    ['-e', 'require("fs").closeSync(0); setTimeout(() => {}, 2000)'],
+    { stdio: ['pipe', 'inherit', 'inherit'] },
+  );
+  // The 'close' must fire well before the child's 2s timer expires.
+  // common.platformTimeout keeps this comfortable on slow CI machines
+  // while still catching the regression (which would only fire 'close'
+  // after the child finally exits at ~2000ms).
+  const eagerWindow = setTimeout(() => {
+    throw new Error('child.stdin close did not fire eagerly on peer-close');
+  }, common.platformTimeout(1500));
+  child.stdin.on('close', common.mustCall(() => {
+    clearTimeout(eagerWindow);
+    // Kill the still-running child so the test does not have to wait
+    // for its 2s timer.
+    child.kill();
+  }));
+  child.on('exit', common.mustCall());
+}