refactor(ext/node): rewrite process stdio to match Node.js pattern#33230
Open
bartlomieju wants to merge 7 commits intomainfrom
Open
refactor(ext/node): rewrite process stdio to match Node.js pattern#33230bartlomieju wants to merge 7 commits intomainfrom
bartlomieju wants to merge 7 commits intomainfrom
Conversation
Rewrites process.stdin/stdout/stderr creation to exactly match Node.js's
`lib/internal/bootstrap/switches/is_main_thread.js` pattern:
- TTY fds -> tty.WriteStream / tty.ReadStream (via libuv TTY handle)
- PIPE/TCP fds -> net.Socket({ fd }) (via libuv Pipe/TCP handle)
- FILE fds -> SyncWriteStream (stdout/stderr) / fs.ReadStream (stdin)
- UNKNOWN -> dummy streams
Previously, process.stdout/stderr were always a generic Writable wrapping
Deno's io.writeSync(), regardless of fd type. Now they are proper Node.js
stream types with real handles, matching Node.js behavior.
Also fixes:
- child_process: skip resume() on parent's stdout/stderr in
waitForChildStreamsToClose to prevent EBADF on write-only Sockets
- Tests: isTTY is now `undefined` (not `false`) for non-TTY streams,
matching Node.js behavior
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
3 tasks
isTTY is `true` for TTY streams and `undefined` for non-TTY, not `false`. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
cc402b7 to
592a3f6
Compare
Match Node.js bootstrap ordering: set up the IPC channel before
creating stdio streams so that process.channel.fd is available.
When a stdio fd is already claimed by IPC, reuse the channel handle
via net.Socket({ handle: process.channel }) instead of opening the
fd again (which fails with EEXIST).
Also fixes unused `process` import lint error in tty_test.ts.
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
createStdin always returns a stream, so the conditional check was unnecessary (leftover from old initStdin warmup path). Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
NativePipe was missing the setBlocking method that net.Socket calls on Windows for stdout/stderr (fd 1/2). This caused a panic when process.stdout was a pipe on Windows because the Socket constructor tried to call this._handle.setBlocking(true). Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
The native write_cb was silently dropping the write status (including EPIPE). Now it calls an onwrite JS callback on the native handle with the actual status, and Pipe.writeBuffer uses this callback to fire req.oncomplete with the real result instead of eagerly reporting success in a microtask. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- uv_stream_set_blocking now correctly dispatches to uv_pipe_t for UV_NAMED_PIPE handles instead of incorrectly casting to uv_tcp_t - Remove Windows-only guard for stdout/stderr setBlocking(true) in net.Socket constructor, matching Node.js which does it on all platforms Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Rewrites
process.stdin/stdout/stderrcreation to exactly match Node.js'slib/internal/bootstrap/switches/is_main_thread.jspattern. This is afollow-up / alternative approach to #33154.
Key changes:
process.stdout/stderrare now created based onguessHandleType(fd):tty.WriteStream(fd)(libuv TTY handle vianet.Socket)net.Socket({ fd })(libuv Pipe/TCP handle)SyncWriteStream(fd)(synchronousfs.writeSync)process.stdinsimilarly uses the correct Node.js stream type per fd typeDeno.stdout.writeSync()in a genericWritableregardless of fd typeand warmup stream replacement logic
Deno.stdin/stdout/stderrcontinue to work via RID-based ops; a__setNodeStreams()hook is added for future delegationBug fixes included:
child_process:#_waitForChildStreamsToClosenow skipsresume()onparent process stdout/stderr to prevent EBADF when they are write-only
net.SocketinstancesisTTYis nowundefinedfor non-TTY streams (matching Node.js) insteadof
falseFixes #33131: The old code used
io.stdout.isTerminal()(Rust'sis_terminal()) to decide whether to create atty.WriteStream. On WindowsGit Bash, Rust's
is_terminal()returnstruebutuv_tty_initfails withEBADF because Git Bash uses named pipes, not a real Windows console. This PR
switches to
guessHandleType(fd)(which calls libuv'suv_guess_handle),matching Node.js. On Git Bash,
uv_guess_handlecorrectly returns PIPE, soa
net.Socketis created instead of attempting TTY initialization.Net deletions: ~200 lines removed.
Closes #33131
Test plan
./x test-node process_test— all pass (82 passed, 0 failed)./x test-node child_process_test— all pass (47 passed, 0 failed)./x test-spec pipe_open_fd— 5/5 pass./x test-spec net_socket_fd— 4/4 pass./x test-spec stdio— 3/3 pass./x test-spec tty— 3/3 pass./x test-spec process— all pass (only pre-existingdenortfailures)./x test-spec node— 246 tests, only pre-existingdenort/compile failures./x test-unit process_test— pass./x test-unit stdio_test— passprocess.stdout.destroy()+ subsequent write still works (indestructible)net.Socket, file redirect createsSyncWriteStream🤖 Generated with Claude Code