From 697cd01d8c77399664d4bc65bd8b828c0f09cc02 Mon Sep 17 00:00:00 2001 From: Mhayk Whandson Date: Mon, 20 Jul 2026 14:16:13 +0100 Subject: [PATCH] child_process: fix stdin close event when child closes fd 0 Fixes #25131 When a child process closes its stdin file descriptor (fd 0), the parent process should receive a 'close' event on the stdin socket. This was working in v8.11.4 but broke in PR #18701. The root cause: PR #18701 explicitly set `readable: true` when creating child stdio sockets, which prevented net.Socket from calling `read(0)`. Without `read(0)`, libuv doesn't listen for EOF/HUP events from the child closing fd 0, so the 'close' event is never emitted. Solution: For stdin specifically (index 0), create the socket without explicitly setting the `readable` option. This allows net.Socket to call `read(0)` in its constructor. After the socket is created, manually set `readable = false` and `writable = true` to maintain the correct semantic properties of stdin. For all other stdio sockets (stdout, stderr, and custom pipes), the original behavior is preserved. Reviewed-By: Mhayk Whandson --- lib/internal/child_process.js | 86 +++++++++++-------- .../test-child-process-stdin-close.js | 29 +++++++ 2 files changed, 78 insertions(+), 37 deletions(-) create mode 100644 test/parallel/test-child-process-stdin-close.js diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js index a12b2954db81de..1e30e2f06f3941 100644 --- a/lib/internal/child_process.js +++ b/lib/internal/child_process.js @@ -176,7 +176,7 @@ const handleConversion = { // waiting for the NODE_HANDLE_ACK of the current passing handle. assert(!target._pendingMessage); target._pendingMessage = - { callback, message, handle, options, retransmissions: 0 }; + { callback, message, handle, options, retransmissions: 0 }; } else { handle.close(); } @@ -332,8 +332,20 @@ function flushStdio(subprocess) { } -function createSocket(pipe, readable) { - return net.Socket({ handle: pipe, readable }); +function createSocket(pipe, readable, stdin = false) { + // For stdin specifically, we must call read(0) to start the read loop, + // allowing libuv to detect EOF/HUP events when the child closes fd 0. + // See: https://github.com/nodejs/node/issues/25131 + if (stdin) { + // stdin: create without specifying readable to trigger read(0), + // then manually set readable=false and writable=true + const socket = net.Socket({ handle: pipe }); + socket.readable = false; + socket.writable = true; + return socket; + } + // For all other sockets, use the standard behavior + return net.Socket({ handle: pipe, readable, writable: !readable }); } @@ -368,7 +380,7 @@ ChildProcess.prototype.spawn = function spawn(options) { validateOneOf(options.serialization, 'options.serialization', - [undefined, 'json', 'advanced']); + [undefined, 'json', 'advanced']); const serialization = options.serialization || 'json'; if (ipc !== undefined) { @@ -380,7 +392,7 @@ ChildProcess.prototype.spawn = function spawn(options) { ArrayPrototypePush(options.envPairs, `NODE_CHANNEL_FD=${ipcFd}`); ArrayPrototypePush(options.envPairs, - `NODE_CHANNEL_SERIALIZATION_MODE=${serialization}`); + `NODE_CHANNEL_SERIALIZATION_MODE=${serialization}`); } validateString(options.file, 'options.file'); @@ -418,10 +430,10 @@ ChildProcess.prototype.spawn = function spawn(options) { // Run-time errors should emit an error, not throw an exception. if (err === UV_EACCES || - err === UV_EAGAIN || - err === UV_EMFILE || - err === UV_ENFILE || - err === UV_ENOENT) { + err === UV_EAGAIN || + err === UV_EMFILE || + err === UV_ENFILE || + err === UV_ENOENT) { if (childProcessSpawn.hasSubscribers) { childProcessSpawn.error.publish({ process: this, @@ -489,7 +501,7 @@ ChildProcess.prototype.spawn = function spawn(options) { if (stream.handle) { stream.socket = createSocket(this.pid !== 0 ? - stream.handle : null, i > 0); + stream.handle : null, i > 0, i === 0); if (i > 0 && this.pid !== 0) { this._closesNeeded++; @@ -511,7 +523,7 @@ ChildProcess.prototype.spawn = function spawn(options) { for (i = 0; i < stdio.length; i++) ArrayPrototypePush(this.stdio, - stdio[i].socket === undefined ? null : stdio[i].socket); + stdio[i].socket === undefined ? null : stdio[i].socket); // Add .send() method and start listening for IPC data if (ipc !== undefined) setupChannel(this, ipc, serialization); @@ -557,7 +569,7 @@ ChildProcess.prototype.kill = function kill(sig) { return false; }; -ChildProcess.prototype[SymbolDispose] = function() { +ChildProcess.prototype[SymbolDispose] = function () { if (!this.killed) { this.kill(); } @@ -635,7 +647,7 @@ function setupChannel(target, channel, serializationMode) { let pendingHandle = null; initMessageChannel(channel); channel.pendingHandle = null; - channel.onread = function(arrayBuffer) { + channel.onread = function (arrayBuffer) { const recvHandle = channel.pendingHandle; channel.pendingHandle = null; if (arrayBuffer) { @@ -674,19 +686,19 @@ function setupChannel(target, channel, serializationMode) { channel.sockets = { got: {}, send: {} }; // Handlers will go through this - target.on('internalMessage', function(message, handle) { + target.on('internalMessage', function (message, handle) { // Once acknowledged - continue sending handles. if (message.cmd === 'NODE_HANDLE_ACK' || - message.cmd === 'NODE_HANDLE_NACK') { + message.cmd === 'NODE_HANDLE_NACK') { if (target._pendingMessage) { if (message.cmd === 'NODE_HANDLE_ACK') { closePendingHandle(target); } else if (target._pendingMessage.retransmissions++ === - MAX_HANDLE_RETRANSMISSIONS) { + MAX_HANDLE_RETRANSMISSIONS) { closePendingHandle(target); process.emitWarning('Handle did not reach the receiving process ' + - 'correctly', 'SentHandleNotReceivedWarning'); + 'correctly', 'SentHandleNotReceivedWarning'); } } @@ -696,9 +708,9 @@ function setupChannel(target, channel, serializationMode) { if (target._pendingMessage) { target._send(target._pendingMessage.message, - target._pendingMessage.handle, - target._pendingMessage.options, - target._pendingMessage.callback); + target._pendingMessage.handle, + target._pendingMessage.options, + target._pendingMessage.callback); } for (let i = 0; i < queue.length; i++) { @@ -740,7 +752,7 @@ function setupChannel(target, channel, serializationMode) { }); }); - target.on('newListener', function() { + target.on('newListener', function () { process.nextTick(() => { if (!target.channel || !target.listenerCount('message')) @@ -758,7 +770,7 @@ function setupChannel(target, channel, serializationMode) { }); }); - target.send = function(message, handle, options, callback) { + target.send = function (message, handle, options, callback) { if (typeof handle === 'function') { callback = handle; handle = undefined; @@ -784,7 +796,7 @@ function setupChannel(target, channel, serializationMode) { return false; }; - target._send = function(message, handle, options, callback) { + target._send = function (message, handle, options, callback) { assert(this.connected || this.channel); if (message === undefined) @@ -795,9 +807,9 @@ function setupChannel(target, channel, serializationMode) { // will result in error message that is weakly consumable. // So perform a final check on message prior to sending. if (typeof message !== 'string' && - typeof message !== 'object' && - typeof message !== 'number' && - typeof message !== 'boolean') { + typeof message !== 'object' && + typeof message !== 'number' && + typeof message !== 'boolean') { throw new ERR_INVALID_ARG_TYPE( 'message', ['string', 'object', 'number', 'boolean'], message); } @@ -858,8 +870,8 @@ function setupChannel(target, channel, serializationMode) { handle.setSimultaneousAccepts(true); } } else if (this._handleQueue && - !(message && (message.cmd === 'NODE_HANDLE_ACK' || - message.cmd === 'NODE_HANDLE_NACK'))) { + !(message && (message.cmd === 'NODE_HANDLE_ACK' || + message.cmd === 'NODE_HANDLE_NACK'))) { // Queue request anyway to avoid out-of-order messages. ArrayPrototypePush(this._handleQueue, { callback: callback, @@ -922,7 +934,7 @@ function setupChannel(target, channel, serializationMode) { // null and connected is false target.connected = true; - target.disconnect = function() { + target.disconnect = function () { if (!this.connected) { this.emit('error', new ERR_IPC_DISCONNECTED()); return; @@ -938,7 +950,7 @@ function setupChannel(target, channel, serializationMode) { this._disconnect(); }; - target._disconnect = function() { + target._disconnect = function () { assert(this.channel); // This marks the fact that the channel is actually disconnected. @@ -996,11 +1008,11 @@ function setupChannel(target, channel, serializationMode) { const INTERNAL_PREFIX = 'NODE_'; function isInternal(message) { return (message !== null && - typeof message === 'object' && - typeof message.cmd === 'string' && - message.cmd.length > INTERNAL_PREFIX.length && - StringPrototypeSlice(message.cmd, 0, INTERNAL_PREFIX.length) === - INTERNAL_PREFIX); + typeof message === 'object' && + typeof message.cmd === 'string' && + message.cmd.length > INTERNAL_PREFIX.length && + StringPrototypeSlice(message.cmd, 0, INTERNAL_PREFIX.length) === + INTERNAL_PREFIX); } const nop = FunctionPrototype; @@ -1038,7 +1050,7 @@ function getValidStdio(stdio, sync) { if (stdio === 'ignore') { ArrayPrototypePush(acc, { type: 'ignore' }); } else if (stdio === 'pipe' || stdio === 'overlapped' || - (typeof stdio === 'number' && stdio < 0)) { + (typeof stdio === 'number' && stdio < 0)) { const a = { type: stdio === 'overlapped' ? 'overlapped' : 'pipe', readable: i === 0, @@ -1078,7 +1090,7 @@ function getValidStdio(stdio, sync) { fd: typeof stdio === 'number' ? stdio : stdio.fd, }); } else if (getHandleWrapType(stdio) || getHandleWrapType(stdio.handle) || - getHandleWrapType(stdio._handle)) { + getHandleWrapType(stdio._handle)) { const handle = getHandleWrapType(stdio) ? stdio : getHandleWrapType(stdio.handle) ? stdio.handle : stdio._handle; diff --git a/test/parallel/test-child-process-stdin-close.js b/test/parallel/test-child-process-stdin-close.js new file mode 100644 index 00000000000000..1094388fc06d11 --- /dev/null +++ b/test/parallel/test-child-process-stdin-close.js @@ -0,0 +1,29 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const { spawn } = require('child_process'); + +// Test that stdin close event is emitted when child process closes its stdin +const cp = spawn( + 'node', [ + '-e', + 'fs.closeSync(0); setTimeout(() => {}, 2000)' + ], + {stdio: ['pipe', 'inherit', 'inherit']} +); + +let closeEventEmitted = false; + +cp.stdin.on('close', common.mustCall(() => { + closeEventEmitted = true; +})); + +setTimeout(() => { + assert(closeEventEmitted, 'stdin close event was not emitted'); + cp.kill(); +}, 1000); + +cp.on('exit', common.mustCall((code, signal) => { + assert(closeEventEmitted, 'stdin close event must be emitted before child exit'); +}));