Conversation
When process.stdin starts reading, Node/libuv put fd 0 into nonblocking mode (uv_pipe_open/uv_tty_init both call uv__nonblock(fd, 1)). Bun left fd 0 untouched and relied on preadv2(RWF_NOWAIT) internally, so a user fs.readSync(0, buf) after attaching a readline/stdin listener would block on an empty pipe (or, on a raw TTY, return one keystroke at a time) instead of throwing EAGAIN like Node. FileReader::open_file_blob now mirrors libuv: for a tty stdio fd it reopens the device with the original access mode and dup2s the fresh nonblocking file description back onto the stdio fd; for a pipe/socket stdio fd it sets O_NONBLOCK directly. Regular-file stdin is left alone. Fixes #5305.
WalkthroughChangesThe change updates Unix stdin handling to preserve access modes and enable nonblocking behavior for pollable stdin, adds startup snapshots and exit-time restoration of Stdin nonblocking behavior
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Updated 5:55 PM PT - Jul 24th, 2026
@autofix-ci[bot], your commit 3c1f8ed is building: |
|
Reproduced with the snippet from #5305: with stdin as a pipe and no data available,
|
|
Found 1 issue this PR may fix:
🤖 Generated with Claude Code |
There was a problem hiding this comment.
I didn't find any bugs, but this changes fd-level stdin semantics on Unix (dup2'ing a fresh file description onto fd 0 for ttys, and flipping O_NONBLOCK on the shared file description for pipe/socket stdio) and shifts the stdio-pipe reader from read_blocking_pipe onto the NonblockingPipe path — worth a human look given the blast radius.
What was reviewed:
- Confirmed
dup2does not carry FD_CLOEXEC to fd 0, so children spawned afterprocess.stdinstarts still inherit stdin. - Verified the tty reopen preserves the fd's original access mode (O_RDWR stays writable) and the parent shell's file description is untouched (the new one is dup2'd in, not fcntl'd).
- The new
set_nonblockingblock only fires whenpollableis true, so regular-file stdin is left blocking (covered by the second test).
Extended reasoning...
Overview
The PR modifies Lazy::open_file_blob in src/runtime/webcore/FileReader.rs to mirror libuv's behavior when process.stdin starts reading: for tty stdio it now reopens with the fd's existing access mode (previously hardcoded O_RDONLY) and dup2s the fresh nonblocking file description back onto the stdio fd; for pipe/socket stdio it sets O_NONBLOCK directly via fcntl. It also adds sys::O::ACCMODE for Unix and a new three-case test file.
Security risks
None identified. The change only touches fd-flag manipulation on the process's own stdio fds; there's no untrusted input parsing, and the added syscalls (fcntl F_GETFL, dup2, fcntl F_SETFL) fail closed (errors are ignored and fall back to the previous behavior).
Level of scrutiny
This warrants maintainer review. The mechanics are small and well-reasoned against libuv (uv_tty_init, uv_pipe_open), but the change alters process-global fd state in a way every stdin consumer sees:
- Previously fd 0 was left completely untouched for ttys (the reader used a separate fd); now fd 0 is replaced via
dup2. Any code holding assumptions about fd 0's original file description now sees a different one. - For pipe stdio,
O_NONBLOCKis set on the shared file description, which is visible to any other fd (in this or a parent process) referencing the same description. This matches Node/libuv, but it's a real behavioral change from what Bun did before. - The PR notes the stdio-pipe reader now routes through
FileType::NonblockingPipe/read_pipeinstead ofread_blocking_pipe, dropping the per-readpoll(). That's a code-path swap in a hot area someone with I/O-subsystem context should sanity-check.
Other factors
I checked a few specific hazards and ruled them out: open_as_nonblocking_tty opens with O_CLOEXEC, but dup2 does not propagate FD_CLOEXEC to the target, so fd 0 remains inheritable by children. The accmode change means an O_RDWR stdin (e.g., an interactive tty) stays writable after the swap, which the previous O_RDONLY hardcode would have broken had the dup2 been added without it. The set_nonblocking block is gated on pollable && !is_nonblocking, so regular files and the already-handled tty-reopen success path are skipped. The 30s Bun.sleep race in the first test is justified with a comment (the failure mode is a blocked read(), no observable signal). The tty/dup2 path itself is not covered by an automated test, which is typical for tty behavior but leaves that branch verified only by reasoning.
Address review: - Scope the new dup2/set_nonblocking to fd 0 only so a Bun.stdout.stream() reader does not flip the write side's blocking-tty contract on fd 1/2. - Snapshot F_GETFL for each stdio fd at startup and restore the O_NONBLOCK bit in bun_restore_stdio(), mirroring Node's ResetStdio(), so a sibling process in the same subshell does not inherit a nonblocking pipe after Bun exits. Install the SIGINT/SIGTERM restore handler whenever a stdio fd was valid at startup, not only when one was a tty. - Add a pty-backed test for the tty dup2 path (O_RDWR preserved, readSync sees EAGAIN) and a subshell test for the exit-time restore.
…ore test The accmode preservation only matters for the fd that is dup2'd back (stdin); reopening fd 1/2 with their own accmode would produce a write-only fd for '> /dev/tty' and break the reader. Keep the previous O_RDONLY reopen for stdout/stderr. Use harness libcPathForDlopen() and fs.constants.O_NONBLOCK in the exit-restore probe so it runs on musl; skip on FreeBSD where the helper has no mapping yet.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/runtime/webcore/FileReader.rs`:
- Around line 184-185: Propagate descriptor setup failures in both affected
sites: in src/runtime/webcore/FileReader.rs lines 184-185, close the reopened
temporary fd and return the error from dup2; in
src/runtime/webcore/FileReader.rs lines 281-284, close fd and return the
set_nonblocking error. Ensure both syscall failures surface as catchable errors
rather than continuing with potentially blocking stdin.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 63aafc3e-1909-41e1-8f78-ce10dc96b63a
📒 Files selected for processing (4)
src/jsc/bindings/c-bindings.cppsrc/runtime/webcore/FileReader.rssrc/sys/lib.rstest/js/node/process/stdin/process-stdin-nonblock.test.ts
There was a problem hiding this comment.
No bugs found this run, but this touches process-global state (stdio fd mutation, exit-time restore, and — notably — widens the onExitSignal SIGINT/SIGTERM handler from tty-only to effectively every invocation via anyTTYs || anyFlags), so it's worth a human look.
What was reviewed:
- tty path: accmode now scoped to stdin (fd 1/2 stays
O_RDONLY),dup2best-effort fallback keeps the pre-PR reader path. - pipe path:
set_nonblockingfailure keepsis_nonblocking = false→ routes throughread_blocking_pipeas before. bun_restore_stdio: only touches the O_NONBLOCK bit; async-signal-safe (fcntl+ EINTR loop); ordered before the termios restore/pipeline-producer suppression.- test: probe now uses
libcPathForDlopen()+fs.constants.O_NONBLOCK, gated to Linux/macOS.
Extended reasoning...
Overview
This PR makes process.stdin put fd 0 into nonblocking mode on Unix, matching Node/libuv (uv_tty_init / uv_pipe_open). It touches four places: FileReader::open_file_blob (reopen tty stdin with preserved accmode + dup2 back onto fd 0; fcntl(F_SETFL, O_NONBLOCK) for pipe/socket stdin), bun_initialize_process (snapshot F_GETFL for each stdio fd; install SIGINT/SIGTERM restore handler whenever any stdio fd was valid, not just when one was a tty), bun_restore_stdio (restore the startup O_NONBLOCK bit at exit), and sys::O::ACCMODE (adds the Unix constant). A new 5-case test covers pipe EAGAIN, regular-file passthrough, tty accmode preservation + EAGAIN, exit-time restore via a subshell sibling probe, and data delivery over the now-nonblocking pipe.
Security risks
None identified. The changes are stdio fd flag manipulation and signal-handler installation on the process's own fds; no untrusted input is parsed.
Level of scrutiny
High. This mutates process-global state that runs for every Bun invocation:
- Signal handler scope:
anyFlagsis true whenever any of fd 0/1/2 succeedsF_GETFL, which is essentially always. Before,onExitSignalwas only installed when a stdio was a tty; now it's installed unconditionally in practice. The handler is transparent (bun_restore_stdio; SIG_DFL; raise) and Node'sPlatformInitdoes the same, but it's a change in default disposition for non-interactive processes that a maintainer should sign off on — e.g., interaction withbun install/bun runsubprocess signal forwarding, or userprocess.on('SIGINT')install/remove sequences that save/restore the previous action. - fd 0 mutation:
dup2swaps fd 0's file description (tty) orF_SETFLmutates the shared one (pipe). The pipe case is visible to any concurrent code touching fd 0 and to sibling processes on the same pipe until exit-time restore runs. Crash / SIGKILL leaves the sibling with nonblocking stdin (same limitation as Node). - Read path change: pipe stdin now takes
NonblockingPipe(read_pipe) instead ofread_blocking_pipe, dropping the per-readpoll()— a real code-path swap for a hot input source.
Other factors
Both prior inline findings from the earlier review pass are addressed (accmode scoped to stdin; probe fixture uses libcPathForDlopen() + fs.constants.O_NONBLOCK and is gated to Linux/macOS). CodeRabbit's error-propagation concern was withdrawn after the best-effort-fallback rationale was explained. Test coverage is thorough for the observable contract, and the exit-restore test uses a real subshell chain to prove the sibling sees blocking stdin again. Given the process-init / signal-handler surface, deferring to a human reviewer.
Fixes #5305.
Repro
With stdin as a pipe and no data available:
EAGAIN: resource temporarily unavailable, readCause
When
process.stdinstarts reading, Node/libuv put fd 0 into nonblocking mode:uv_pipe_opencallsuv__nonblock(fd, 1)directly, anduv_tty_initreopens the tty viattyname_r,dup2s the fresh file description back onto the stdio fd, then setsO_NONBLOCKon it. After that, a directfs.readSync(0, buf)observes a nonblocking fd and surfaces the kernel'sEAGAIN.Bun's
FileReader::open_file_blobleft fd 0 untouched. For a tty it opened a separate nonblocking fd and used that for its own reads; for a pipe it used fd 0 as-is and relied onpreadv2(RWF_NOWAIT)to avoid blocking internally. Either way, fd 0's file description stayed blocking, so any user syscall on fd 0 (fs.readSync,fs.readvSync) blocked.Fix
open_file_blobnow mirrors libuv for stdin on Unix:O_RDWRstdin stays writable) anddup2the new nonblocking file description back onto fd 0. The reader keeps using the new fd;Fd::closealready skips fd 0.fcntl(F_SETFL, O_NONBLOCK)on fd 0 directly, matchinguv_pipe_open.pollableis false).Bun.stdout.stream()reader cannot disturb the write side's blocking-tty contract.Setting
O_NONBLOCKon a pipe's file description is visible to any sibling in the same subshell that shares fd 0. Node bounds that by restoring the startup flag bit inResetStdio()on the way out, sobun_initialize_processnow snapshotsF_GETFLfor each stdio fd andbun_restore_stdio()restores theO_NONBLOCKbit at exit (and on SIGINT/SIGTERM, for which the handler is now installed whenever a stdio fd was valid at startup rather than only when one was a tty).As a side effect the stdin pipe reader now takes the
NonblockingPipepath (read_pipe) instead ofread_blocking_pipe, dropping the per-readpoll()readiness check.Verification
test/js/node/process/stdin/process-stdin-nonblock.test.ts:fs.readSync(0)throwsEAGAINonceprocess.stdinis reading (hangs on main)fs.readSync(0)still returns dataBun.spawn({ terminal })): fd 0 is nonblocking,fs.writeSync(0, ...)still succeeds (access mode preserved),fs.readSync(0)throwsEAGAIN(hangs on main)process.stdinstill delivers data over the now-nonblocking pipeno test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/node/process/stdin/process-stdin-nonblock.test.ts