fix(ext/node): pass stream fd to child process stdio instead of inheriting#33525
Conversation
…iting When a Stream object (like another child's stdout) was passed as a stdio option to `spawn()`, Deno incorrectly converted it to "inherit", causing the child to inherit the parent Deno process's stdio instead of sharing the stream's underlying OS pipe. Now extracts the fd from the stream's handle and passes it directly, so the Rust side can dup() it for the child. For streams without a handle fd, falls back to creating a pipe with JS-level piping.
bartlomieju
left a comment
bartlomieju
left a comment
There was a problem hiding this comment.
test-child-process-stdio-merge-stdouts-into-cat.js
test-child-process-stdio-reuse-readable-stdio.js
should be enabled in tests/node_compat/config.jsonc
On Windows, PipeWrap.fd always returns -1, so the fd-sharing path is
not used and we fall back to JS-level pipe(). The default pipe()
behavior calls end() on the destination when the source ends, which
closes a shared stream (e.g. p3.stdin) when the first child exits,
preventing other children from writing to it. Use { end: false } for
stdout/stderr pipe() calls so multiple children can share a destination.
fibibot
left a comment
fibibot
left a comment
There was a problem hiding this comment.
LGTM. The shape is right and the test scenarios both work out:
-
test-child-process-stdio-reuse-readable-stdio.js(spawn('head', ['-n1'], { stdio: [p1.stdout, 'pipe', 'inherit'] })):p1.stdout._handle.fdis the parent's read end of cat's stdout pipe.toDenoStdioreturns that fd; Rustdup()s it into p2's stdin. p2 (head) and the parent share the OS pipe. Test reads p2.stdout for 'hello\n', then after p2 exits, parent reads p1.stdout for 'world\n'. The dup-shared pipe works because consumers compete (head wins while it's running, parent reads after head exits). -
test-child-process-stdio-merge-stdouts-into-cat.js(spawn('cat', { stdio: ['pipe', p3.stdin, 'inherit'] })):p3.stdin._handle.fdis the parent's write end of cat's stdin pipe. The fd is duped to p1's and p2's stdouts, so both children write to p3's stdin pipe; p3 reads them as input. The{ end: false }on the pipe-creation fallback path correctly handles the multi-producer case (any one child exiting must not close the destination for the others). -
The
streamHandleFdextraction (stream._handle?.fd >= 0) correctly falls through to the "piped" + JS-level pipe fallback for streams without a real fd;stdin.pipe(stdinSocket)andstdoutSocket.pipe(stdout, { end: false })cover that case. -
For the fd-direct case
this.stdin/stdout/stderris intentionally left unset — Node also doesn't expose the parent-side handle when the stream is duped, since the parent is supposed to keep using the original stream object.
CI green.
2 participants
Summary
When a
Streamobject (e.g. another child'sstdout) was passed as astdio option to
child_process.spawn(), Deno incorrectly converted it to"inherit", causing the child to inherit the parent Deno process's stdiofd instead of sharing the stream's underlying OS pipe.
For example, this pattern used to fail:
headwould inherit Deno's stdin instead of reading fromcat's stdoutpipe. Now
toDenoStdio()extracts the fd from the stream's_handleandpasses it directly, so the Rust side
dup()s it for the child process.For streams without a handle fd, falls back to creating a pipe with
JS-level piping.
Test plan
./x test-compat test-child-process-stdio-reuse-readable-stdio.js— was failing, now passes./x test-compat test-child-process-stdio-merge-stdouts-into-cat.js— still passes