Skip to content

Commit 697cd01

Browse files
committed
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 <hi@mhayk.com>
1 parent fe3b3b8 commit 697cd01

2 files changed

Lines changed: 78 additions & 37 deletions

File tree

‎lib/internal/child_process.js‎

Lines changed: 49 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ const handleConversion = {
176176
// waiting for the NODE_HANDLE_ACK of the current passing handle.
177177
assert(!target._pendingMessage);
178178
target._pendingMessage =
179-
{ callback, message, handle, options, retransmissions: 0 };
179+
{ callback, message, handle, options, retransmissions: 0 };
180180
} else {
181181
handle.close();
182182
}
@@ -332,8 +332,20 @@ function flushStdio(subprocess) {
332332
}
333333

334334

335-
function createSocket(pipe, readable) {
336-
return net.Socket({ handle: pipe, readable });
335+
function createSocket(pipe, readable, stdin = false) {
336+
// For stdin specifically, we must call read(0) to start the read loop,
337+
// allowing libuv to detect EOF/HUP events when the child closes fd 0.
338+
// See: https://github.com/nodejs/node/issues/25131
339+
if (stdin) {
340+
// stdin: create without specifying readable to trigger read(0),
341+
// then manually set readable=false and writable=true
342+
const socket = net.Socket({ handle: pipe });
343+
socket.readable = false;
344+
socket.writable = true;
345+
return socket;
346+
}
347+
// For all other sockets, use the standard behavior
348+
return net.Socket({ handle: pipe, readable, writable: !readable });
337349
}
338350

339351

@@ -368,7 +380,7 @@ ChildProcess.prototype.spawn = function spawn(options) {
368380

369381

370382
validateOneOf(options.serialization, 'options.serialization',
371-
[undefined, 'json', 'advanced']);
383+
[undefined, 'json', 'advanced']);
372384
const serialization = options.serialization || 'json';
373385

374386
if (ipc !== undefined) {
@@ -380,7 +392,7 @@ ChildProcess.prototype.spawn = function spawn(options) {
380392

381393
ArrayPrototypePush(options.envPairs, `NODE_CHANNEL_FD=${ipcFd}`);
382394
ArrayPrototypePush(options.envPairs,
383-
`NODE_CHANNEL_SERIALIZATION_MODE=${serialization}`);
395+
`NODE_CHANNEL_SERIALIZATION_MODE=${serialization}`);
384396
}
385397

386398
validateString(options.file, 'options.file');
@@ -418,10 +430,10 @@ ChildProcess.prototype.spawn = function spawn(options) {
418430

419431
// Run-time errors should emit an error, not throw an exception.
420432
if (err === UV_EACCES ||
421-
err === UV_EAGAIN ||
422-
err === UV_EMFILE ||
423-
err === UV_ENFILE ||
424-
err === UV_ENOENT) {
433+
err === UV_EAGAIN ||
434+
err === UV_EMFILE ||
435+
err === UV_ENFILE ||
436+
err === UV_ENOENT) {
425437
if (childProcessSpawn.hasSubscribers) {
426438
childProcessSpawn.error.publish({
427439
process: this,
@@ -489,7 +501,7 @@ ChildProcess.prototype.spawn = function spawn(options) {
489501

490502
if (stream.handle) {
491503
stream.socket = createSocket(this.pid !== 0 ?
492-
stream.handle : null, i > 0);
504+
stream.handle : null, i > 0, i === 0);
493505

494506
if (i > 0 && this.pid !== 0) {
495507
this._closesNeeded++;
@@ -511,7 +523,7 @@ ChildProcess.prototype.spawn = function spawn(options) {
511523

512524
for (i = 0; i < stdio.length; i++)
513525
ArrayPrototypePush(this.stdio,
514-
stdio[i].socket === undefined ? null : stdio[i].socket);
526+
stdio[i].socket === undefined ? null : stdio[i].socket);
515527

516528
// Add .send() method and start listening for IPC data
517529
if (ipc !== undefined) setupChannel(this, ipc, serialization);
@@ -557,7 +569,7 @@ ChildProcess.prototype.kill = function kill(sig) {
557569
return false;
558570
};
559571

560-
ChildProcess.prototype[SymbolDispose] = function() {
572+
ChildProcess.prototype[SymbolDispose] = function () {
561573
if (!this.killed) {
562574
this.kill();
563575
}
@@ -635,7 +647,7 @@ function setupChannel(target, channel, serializationMode) {
635647
let pendingHandle = null;
636648
initMessageChannel(channel);
637649
channel.pendingHandle = null;
638-
channel.onread = function(arrayBuffer) {
650+
channel.onread = function (arrayBuffer) {
639651
const recvHandle = channel.pendingHandle;
640652
channel.pendingHandle = null;
641653
if (arrayBuffer) {
@@ -674,19 +686,19 @@ function setupChannel(target, channel, serializationMode) {
674686
channel.sockets = { got: {}, send: {} };
675687

676688
// Handlers will go through this
677-
target.on('internalMessage', function(message, handle) {
689+
target.on('internalMessage', function (message, handle) {
678690
// Once acknowledged - continue sending handles.
679691
if (message.cmd === 'NODE_HANDLE_ACK' ||
680-
message.cmd === 'NODE_HANDLE_NACK') {
692+
message.cmd === 'NODE_HANDLE_NACK') {
681693

682694
if (target._pendingMessage) {
683695
if (message.cmd === 'NODE_HANDLE_ACK') {
684696
closePendingHandle(target);
685697
} else if (target._pendingMessage.retransmissions++ ===
686-
MAX_HANDLE_RETRANSMISSIONS) {
698+
MAX_HANDLE_RETRANSMISSIONS) {
687699
closePendingHandle(target);
688700
process.emitWarning('Handle did not reach the receiving process ' +
689-
'correctly', 'SentHandleNotReceivedWarning');
701+
'correctly', 'SentHandleNotReceivedWarning');
690702
}
691703
}
692704

@@ -696,9 +708,9 @@ function setupChannel(target, channel, serializationMode) {
696708

697709
if (target._pendingMessage) {
698710
target._send(target._pendingMessage.message,
699-
target._pendingMessage.handle,
700-
target._pendingMessage.options,
701-
target._pendingMessage.callback);
711+
target._pendingMessage.handle,
712+
target._pendingMessage.options,
713+
target._pendingMessage.callback);
702714
}
703715

704716
for (let i = 0; i < queue.length; i++) {
@@ -740,7 +752,7 @@ function setupChannel(target, channel, serializationMode) {
740752
});
741753
});
742754

743-
target.on('newListener', function() {
755+
target.on('newListener', function () {
744756

745757
process.nextTick(() => {
746758
if (!target.channel || !target.listenerCount('message'))
@@ -758,7 +770,7 @@ function setupChannel(target, channel, serializationMode) {
758770
});
759771
});
760772

761-
target.send = function(message, handle, options, callback) {
773+
target.send = function (message, handle, options, callback) {
762774
if (typeof handle === 'function') {
763775
callback = handle;
764776
handle = undefined;
@@ -784,7 +796,7 @@ function setupChannel(target, channel, serializationMode) {
784796
return false;
785797
};
786798

787-
target._send = function(message, handle, options, callback) {
799+
target._send = function (message, handle, options, callback) {
788800
assert(this.connected || this.channel);
789801

790802
if (message === undefined)
@@ -795,9 +807,9 @@ function setupChannel(target, channel, serializationMode) {
795807
// will result in error message that is weakly consumable.
796808
// So perform a final check on message prior to sending.
797809
if (typeof message !== 'string' &&
798-
typeof message !== 'object' &&
799-
typeof message !== 'number' &&
800-
typeof message !== 'boolean') {
810+
typeof message !== 'object' &&
811+
typeof message !== 'number' &&
812+
typeof message !== 'boolean') {
801813
throw new ERR_INVALID_ARG_TYPE(
802814
'message', ['string', 'object', 'number', 'boolean'], message);
803815
}
@@ -858,8 +870,8 @@ function setupChannel(target, channel, serializationMode) {
858870
handle.setSimultaneousAccepts(true);
859871
}
860872
} else if (this._handleQueue &&
861-
!(message && (message.cmd === 'NODE_HANDLE_ACK' ||
862-
message.cmd === 'NODE_HANDLE_NACK'))) {
873+
!(message && (message.cmd === 'NODE_HANDLE_ACK' ||
874+
message.cmd === 'NODE_HANDLE_NACK'))) {
863875
// Queue request anyway to avoid out-of-order messages.
864876
ArrayPrototypePush(this._handleQueue, {
865877
callback: callback,
@@ -922,7 +934,7 @@ function setupChannel(target, channel, serializationMode) {
922934
// null and connected is false
923935
target.connected = true;
924936

925-
target.disconnect = function() {
937+
target.disconnect = function () {
926938
if (!this.connected) {
927939
this.emit('error', new ERR_IPC_DISCONNECTED());
928940
return;
@@ -938,7 +950,7 @@ function setupChannel(target, channel, serializationMode) {
938950
this._disconnect();
939951
};
940952

941-
target._disconnect = function() {
953+
target._disconnect = function () {
942954
assert(this.channel);
943955

944956
// This marks the fact that the channel is actually disconnected.
@@ -996,11 +1008,11 @@ function setupChannel(target, channel, serializationMode) {
9961008
const INTERNAL_PREFIX = 'NODE_';
9971009
function isInternal(message) {
9981010
return (message !== null &&
999-
typeof message === 'object' &&
1000-
typeof message.cmd === 'string' &&
1001-
message.cmd.length > INTERNAL_PREFIX.length &&
1002-
StringPrototypeSlice(message.cmd, 0, INTERNAL_PREFIX.length) ===
1003-
INTERNAL_PREFIX);
1011+
typeof message === 'object' &&
1012+
typeof message.cmd === 'string' &&
1013+
message.cmd.length > INTERNAL_PREFIX.length &&
1014+
StringPrototypeSlice(message.cmd, 0, INTERNAL_PREFIX.length) ===
1015+
INTERNAL_PREFIX);
10041016
}
10051017

10061018
const nop = FunctionPrototype;
@@ -1038,7 +1050,7 @@ function getValidStdio(stdio, sync) {
10381050
if (stdio === 'ignore') {
10391051
ArrayPrototypePush(acc, { type: 'ignore' });
10401052
} else if (stdio === 'pipe' || stdio === 'overlapped' ||
1041-
(typeof stdio === 'number' && stdio < 0)) {
1053+
(typeof stdio === 'number' && stdio < 0)) {
10421054
const a = {
10431055
type: stdio === 'overlapped' ? 'overlapped' : 'pipe',
10441056
readable: i === 0,
@@ -1078,7 +1090,7 @@ function getValidStdio(stdio, sync) {
10781090
fd: typeof stdio === 'number' ? stdio : stdio.fd,
10791091
});
10801092
} else if (getHandleWrapType(stdio) || getHandleWrapType(stdio.handle) ||
1081-
getHandleWrapType(stdio._handle)) {
1093+
getHandleWrapType(stdio._handle)) {
10821094
const handle = getHandleWrapType(stdio) ?
10831095
stdio :
10841096
getHandleWrapType(stdio.handle) ? stdio.handle : stdio._handle;
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
const { spawn } = require('child_process');
6+
7+
// Test that stdin close event is emitted when child process closes its stdin
8+
const cp = spawn(
9+
'node', [
10+
'-e',
11+
'fs.closeSync(0); setTimeout(() => {}, 2000)'
12+
],
13+
{stdio: ['pipe', 'inherit', 'inherit']}
14+
);
15+
16+
let closeEventEmitted = false;
17+
18+
cp.stdin.on('close', common.mustCall(() => {
19+
closeEventEmitted = true;
20+
}));
21+
22+
setTimeout(() => {
23+
assert(closeEventEmitted, 'stdin close event was not emitted');
24+
cp.kill();
25+
}, 1000);
26+
27+
cp.on('exit', common.mustCall((code, signal) => {
28+
assert(closeEventEmitted, 'stdin close event must be emitted before child exit');
29+
}));

0 commit comments

Comments
 (0)