Bun.write(file, file): wait for a non-blocking fd instead of rejecting with EAGAIN - #38498
Bun.write(file, file): wait for a non-blocking fd instead of rejecting with EAGAIN#38498robobun wants to merge 4 commits into
Conversation
…with EAGAIN CopyFile copies on a pool thread with blocking-style loops, but the fds it is handed can be in O_NONBLOCK mode: process.stdout puts the shared stdio open file description there (a Worker's startup does so too), and a child inherits the state. splice/sendfile and the read/write fallback then return EAGAIN, which the loops reported as the copy's failure, so the documented `await Bun.write(Bun.stdout, Bun.stdin)` rejected whenever stdin had no data yet or stdout was full. Add bun_sys::block_until_readable / block_until_writable (poll(2); select(2) on macOS, where poll does not see a named pipe's peer closing) and make the copy loops wait and retry on EAGAIN. The splice arm waits for both fds since a pipe-to-pipe splice is non-blocking as a whole if either pipe is.
|
Warning Review limit reached
Next review available in: 7 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
Comment |
|
Status: closed in favour of #37128. The bug is real (reproduced on the released 1.4.0 and on main: |
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it adds new cross-platform syscall wrappers (select$DARWIN_EXTSN$NOCANCEL FFI on macOS that couldn't be tested locally) and changes the blocking behaviour of the Bun.write(file, file) copy path, a human look would still be worthwhile — particularly on whether blocking the pool thread in poll/select is the shape you want here versus the "should use non-blocking I/O" TODO the PR removes.
Checked: the select_one bitset math (word + 1 words covers nfds = fd + 1); posix::poll already retries EINTR; read_write_loop_capped's write-EAGAIN arm keeps slice intact so no bytes are dropped on retry; is_retry() matches only EAGAIN. The tests await observable conditions (child stderr JSON reports, first-piece echo) rather than sleeping.
Extended reasoning...
Overview
The PR fixes Bun.write(Bun.stdout, Bun.stdin) (and the other file-to-file copy variants) rejecting with EAGAIN when either fd's open file description is O_NONBLOCK. It adds bun_sys::block_until_readable/block_until_writable (poll on Linux/FreeBSD/Android, select$DARWIN_EXTSN$NOCANCEL on macOS to avoid the XNU FIFO-EOF poll bug), a new Tag::select, and wires the two EAGAIN sites in copy_file.rs (do_copy_file_range and read_write_loop_capped) to wait and retry. A new 8-case test file exercises splice/sendfile/read/write against non-blocking sources and destinations.
Security risks
None. No user-controlled input reaches the new code beyond fds already validated by the existing paths; the select bitset is sized from an fd the process already owns.
Level of scrutiny
High. This is production runtime I/O in Bun.write, adds a new public helper pair to bun_sys, introduces macOS-specific FFI (select$DARWIN_EXTSN$NOCANCEL with a hand-built variable-length fd_set) that the author could not run locally, and makes a design call — block the pool thread in poll/select — where the existing code carried a "TODO: this should use non-blocking I/O" comment that the PR deletes. The description justifies the choice well (the loop already blocks in the kernel for blocking fds, and O_NONBLOCK on the shared description must not be flipped), but it's the kind of architectural decision a maintainer should confirm.
Other factors
The tests are thorough (both copy loops via BUN_CONFIG_DISABLE_COPY_FILE_RANGE, both directions of backpressure, Worker case, FIFO source) and structured to await real signals rather than sleep. The comment-cop feedback was addressed in d3fa7c2. CI hasn't reported yet, and the darwin lanes are the only coverage of the macOS select arm. The PR also notes intentional overlap with #37823/#37852 and deliberately leaves copy_file_using_read_write_loop for #36692/#33715 — a maintainer should confirm that scoping.
|
Updated 7:05 AM PT - Aug 14th, 2026
❌ @robobun, your commit d3fa7c2 has some failures in 🧪 To try this PR locally: bunx bun-pr 38498That installs a local version of the PR into your bun-38498 --bun |
Problem
await Bun.write(Bun.stdout, Bun.stdin)(the documented way to copy stdin to stdout) rejects withEAGAIN,syscall: "splice", as soon as fd 1 is in O_NONBLOCK mode and stdin has no data yet.process.stdoutputs the inherited stdio open file description into that mode the first time it is used, and a Worker does that while starting up, so inside aworker_threadsWorker the idiom fails every time the data is not already sitting in the pipe. Reproduces on 1.4.0 and on main, Linux.Bun.write(Bun.stdout, Bun.stdin)orBun.write(Bun.stdout, Bun.file(path))into a non-blocking stdout whose reader is slower than the copy rejects withEAGAINfromspliceorsendfileonce the pipe is full, and the read/write fallback (what macOS and FreeBSD always use for an fd destination, and Linux withBUN_CONFIG_DISABLE_COPY_FILE_RANGE=1) rejects withEAGAINfromreadorwrite. A copy that started fine fails the same way as soon as its source is momentarily empty.Bun.writeisCopyFile(src/runtime/webcore/blob/copy_file.rs), which copies on a pool thread with loops written for blocking fds.do_copy_file_range(thesplice/sendfile/copy_file_rangeloop, errno match at line 427) andread_write_loop_capped(the?onreadandwriteat lines 995 and 1002) treat EAGAIN like any other errno and fail the copy. O_NONBLOCK is a property of the open file description, so the flag set byprocess.stdoutis visible to every user of fd 1, and is inherited by child processes as well.Fix
bun_sys::block_until_readable(fd)/block_until_writable(fd): block until a read-side or write-side syscall on the fd would make progress.poll(2)(via the existing EINTR-retryingposix::poll) everywhere except macOS; on macOSselect(2), because XNU's poll is built on the kqueue vnode filter, which for a named pipe never fires for the peer closing, so a poll-based wait on a non-blocking FIFO would turn today's EAGAIN into a hang at EOF.select$DARWIN_EXTSN$NOCANCELis the variant without the FD_SETSIZE limit. This is the same helper (same import, sameTag::select) that Bun.file(fifo).bytes(): wait for a named pipe's EOF with select(2) on macOS #37823 and Bun.write: reject with EPIPE when a FIFO's reader goes away mid-write #37852 add for their macOS-only FIFO cases; it is defined in the same spot so whichever lands second gets a textual conflict rather than a duplicate definition.do_copy_file_range: on EAGAIN, wait for the source to be readable and the destination to be writable, then retry. Both, because a pipe-to-pipespliceis non-blocking as a whole when either pipe is, so the errno does not say which side would have blocked (in the repro it is the blocking, empty stdin that produces the EAGAIN). On a regular file either wait returns at once.read_write_loop_capped: EAGAIN fromreadwaits for the source and re-reads; EAGAIN fromwritewaits for the destination and retries the same slice, so nothing read is lost.spliceon an empty blocking stdin sits in the kernel until data arrives). Waiting inpoll/selecton the same thread gives a non-blocking description exactly the behaviour a blocking one already has, without touching the flag, which is shared withprocess.stdoutand must stay set for it. Both waits return on HUP/ERR too, and the retried syscall then reports the real outcome (0 for EOF, EPIPE for a vanished reader), so error semantics are unchanged. The interaction withworker.terminate()is the same as for a blockedsplicetoday and is handled separately by Run pool jobs that own their memory without a VM borrow so worker.terminate() does not wait on a blocked Bun.write(file, file) #38312. Moving the wait to the io thread instead (what theTODO: this should use non-blocking I/Onotes in this function ask for) would be a rewrite of the job into a state machine; this PR does not attempt it, and only drops the one copy of that note that sat directly above the arm which now handles the non-blocking case, the ones on the fallback arms still stand.CopyFileopened itself from a path (copy_file_using_read_write_loop, shared withfs.copyFile). Its destination is always blocking; a non-blocking source only reaches it through the cases Bun.write(file, Bun.stdin): splice a pipe source into any destination on Linux #36692 is adding (FIFO source with a path destination) or on kernels wheresendfilerejects the source, and it is being reshaped by Bun.write(file, Bun.stdin): splice a pipe source into any destination on Linux #36692 and Bun.write: fix file-to-file copy resolving 0 bytes on Windows and macOS overwrite #33715, so it is left for those to pick the helpers up.test/js/bun/io/bun-write-nonblocking-stdio.test.ts(new, POSIX): the reported case (stdin late, stdout made non-blocking byprocess.stdout), the same inside a Worker, and for both the kernel-copy loop and the read/write loop: a pipe source and a regular-file source into a stdout the child has filled up, and a source that is non-blocking by construction (a FIFO opened with O_NONBLOCK handed to the child as stdin). The source-side tests feed the input in two pieces and send the second only once the first has come back out, so the copy is guaranteed to meet an empty source mid-way. Against the released 1.4.0 all 8 fail, 60 of 60 runs, each with its own errno site:EAGAIN splice(4),EAGAIN sendfile,EAGAIN write(2),EAGAIN read. With this change on the debug (ASAN) build: 8 of 8 pass, 15 of 15 runs, about 6 s per run for the file.bun-write.test.jsbecause that file is onedescribe.concurrentof subprocess tests that already runs close to its timeouts under ASAN (see test: stop the Bun.write copy_file_range fallback test from starving its concurrent siblings #37792); adding eight more there pushed both old and new tests over 5 s in this environment, while on their own they take 1.2 to 1.5 s (3.5 s for the Worker one, which has an explicit timeout).bun-write.test.jsbehaves the same before and after (the one failure here is the pre-existingcopyFileRange is not available > on large filestimeout under ASAN).cargo check/cargo clippyofbun_sysandbun_runtimenatively and foraarch64-apple-darwin,x86_64-apple-darwin,x86_64-unknown-freebsd,aarch64-linux-android,x86_64-pc-windows-msvc(bun_sys);cargo fmt;test/internal/source-lints. There is no macOS machine here; the macOSselectarm is exercised by the new tests on the darwin lanes (the same code passed Bun.file(fifo).bytes(): wait for a named pipe's EOF with select(2) on macOS #37823's and Bun.write: reject with EPIPE when a FIFO's reader goes away mid-write #37852's darwin lanes).Background
dup,fork/spawnand stdio inheritance make several fds share one. O_NONBLOCK is stored on the description, so setting it through any one fd changes the behaviour of every fd sharing it, in this process and in children. That is howprocess.stdout(which dups fd 1 and sets O_NONBLOCK for its own writer) ends up changing whatBun.write(Bun.stdout, ...)sees on fd 1.poll/selectand retry.splice(2)between two pipes uses the flags of both pipes, so it behaves non-blocking towards both ends if either one is non-blocking.CopyFileis the off-thread half ofBun.write(fileA, fileB): it opens what it needs, pickscopy_file_range,spliceorsendfileon Linux (read/write loop otherwise or as fallback), runs the whole copy on a work-pool thread, and resolves the promise with the byte count when done.poll(2)is implemented on kqueue. For a FIFO created withmkfifo, XNU's vnode filter reports only "bytes/space available", not "the other end went away", whileselect(2)goes through the FIFO's underlying socket, which reports both.pipe(2)pipes (what shell pipelines and subprocess stdio use) report both under either call.