Bun.write: deliver the whole payload to a FIFO instead of a torn prefix by robobun · Pull Request #36025 · oven-sh/bun

robobun · 2026-07-26T23:17:27Z

Repro

// linux; mkfifo /tmp/f && (cat /tmp/f > /tmp/got &) then:
await Bun.write("/tmp/f", "x".repeat(200000) + "END");

With a live reader attached and a payload larger than the kernel pipe buffer:

source	before	reader received
string / Uint8Array	rejects `ENXIO` ("no reader")	torn prefix (pipe-buffer bytes)
Blob / Response	never settles	torn prefix
`Bun.file(fifo).writer()` (control)	ok	full payload

Cause

Sync fast path (write_string_to_file_fast / write_bytes_to_file_fast): opened O_WRONLY|O_NONBLOCK, wrote until EAGAIN, then set needs_async, which dropped the CloseOnDrop guard and closed the fd. Closing the only writer gave the reader EOF at the partial length; the async fallback then reopened with O_TRUNC|O_NONBLOCK and got ENXIO because the reader had already gone.

Async WriteFile path: could_block was hardwired false for path targets (the fstat was deliberately skipped), and do_write matched EAGAIN in a loop whose sys::write sat outside the loop body, so continue re-matched the same stale Err(EAGAIN) forever without ever reaching wait_for_writable().

Fix

write_file.rs: run_with_fd now fstats the opened fd and sets could_block = !is_regular_file(mode), so a FIFO/socket/chardev registers for POLLOUT on EAGAIN exactly like FileSink already does. do_write issues sys::write inside the loop so the non-pollable branch actually retries.
Blob.rs: when the fast path opens a path and fstat shows it is not a regular file, it writes nothing and hands the open fd to the async path via a new WriteFileOptions.handoff_fd. WriteFile adopts that fd (get_fd already short-circuits on a preset opened_fd), so the FIFO is opened exactly once and the reader never sees an intermediate EOF. Scope guards close the fd on any early return between the fast path and WriteFile adopting it.

Regular-file writes are unchanged apart from one fstat on the freshly opened fd.

Verification

$ bun bd test test/js/bun/io/bun-write.test.js -t "larger than the pipe buffer"
(pass) Bun.write(fifo, string) larger than the pipe buffer > delivers every byte to the reader
(pass) Bun.write(fifo, Uint8Array) larger than the pipe buffer > delivers every byte to the reader
(pass) Bun.write(fifo, Blob) larger than the pipe buffer > delivers every byte to the reader
(pass) Bun.write(fifo, Response) larger than the pipe buffer > delivers every byte to the reader
4 pass, 0 fail

All four time out on main. Bun.write(fifo, ...) with no reader still rejects ENXIO immediately, and the existing filesink.test.ts suite is unchanged (50 pass).

no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/io/bun-write.test.js

Bun.write(fifoPath, data) with a live reader and a payload larger than the kernel pipe buffer delivered only the first partial write and then either rejected ENXIO or never settled. Sync fast path (string/Uint8Array): opened O_WRONLY|O_NONBLOCK, wrote until EAGAIN, then closed the fd and fell through to the async path. Closing the only writer gave the reader EOF at the partial length, and the async reopen (O_TRUNC|O_NONBLOCK) found no reader and surfaced ENXIO for a write that had already shipped bytes. Async path (Blob/Response, and anything over 256 KiB): could_block was hardwired false for path targets, and do_write matched EAGAIN in a loop whose sys::write sat outside the loop body, so it spun forever on the same stale result without ever registering for POLLOUT. Fix: * write_file.rs: fstat the opened fd in run_with_fd and set could_block from the real mode, so a FIFO/socket/chardev takes wait_for_writable() on EAGAIN. Move sys::write inside the do_write loop so the non-pollable branch actually re-issues the syscall. * Blob.rs: after the fast path opens a non-regular file, do not write and do not close. Hand the open fd to the async WriteFile via a new WriteFileOptions.handoff_fd so the fifo is opened exactly once and the reader never sees an intermediate EOF. Guards close the fd on any early-return between the fast path and WriteFile adopting it. Bun.file(fifo).writer() already handled this correctly; Bun.write now matches it.

robobun · 2026-07-26T23:17:37Z

^{Updated 8:30 PM PT - Jul 26th, 2026}

✅ @robobun, your commit d8994254aef4ae8a6e0c34e610c49a8afc92f28c passed in Build #82982! 🎉

🧪 To try this PR locally:

bunx bun-pr 36025

That installs a local version of the PR into your bun-36025 executable, so you can run:

bun-36025 --bun

coderabbitai · 2026-07-26T23:17:37Z

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 2 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 028d2ad3-3566-4f6e-964e-06e6079fbe2a

📥 Commits

Reviewing files that changed from the base of the PR and between 43d60f6 and d899425.

📒 Files selected for processing (4)

src/runtime/webcore/Blob.rs
src/runtime/webcore/S3Client.rs
src/runtime/webcore/blob/write_file.rs
test/js/bun/io/bun-write.test.js

_{Comment @coderabbitai help to get the list of available commands.}

claude

Beyond the inline findings, I traced the handoff-fd ownership chain — the two nested scopeguards close it on every early return, handoff_fd.take() disarms them before adoption, and is_allowed_to_close() is true for path targets so WriteFile::do_close releases the adopted fd on finish; no leak or double-close found. The O_CLOEXEC concern was also checked and ruled out (the fast path opens with the same flags it did before, so no behavior change there).

Extended reasoning...

Bugs were found (the empty-payload → EINVAL regression is real and worth fixing), so deferring to human review. This note just records what else was examined so it isn't re-traced: fd ownership across the fast-path → write_file_internal scopeguard → WriteFileOptions.handoff_fd → write_file_with_source_destination scopeguard → (*file_copier).opened_fd chain is balanced on all paths, and the CLOEXEC candidate was refuted.

…ONBLOCK * Blob.rs: only hand the fd to the async path when there are bytes to write; a 0-byte write cannot EAGAIN and stays on the fast path. Rename _needs_async -> needs_async in write_bytes_to_file_fast now that it is written on every branch. * bun-write.test.js: open the read side synchronously with O_RDONLY|O_NONBLOCK and drain via Bun.file(fd).stream(). The previous fs.createReadStream(fifo) reader parked a thread-pool worker in a blocking open() per test, and under describe.concurrent that let the write side's ENXIO retry spin past the test timeout on the asan lane.

robobun · 2026-07-27T00:04:12Z

Same fix landed independently in #36028 (branch farm/c3bec125/bun-write-fifo); closing that one in favour of this PR.

One difference worth a look: this PR's do_write keeps the loop { match ... } and moves sys::write inside it, so the !could_block => continue arm now busy-spins on real write() syscalls. The run_with_fd fstat makes that arm unreachable for path-opened FIFOs, but a caller-supplied fd whose store has seekable == None (e.g. Bun.file(rawPipeFd)) still falls through to could_block = false and would spin. #36028 (and #35953) drop the loop entirely and set could_block = true on the first EAGAIN before wait_for_writable(), which covers that case too.

claude

Additional findings (outside current diff — PR may have been updated during review):

🔴 src/runtime/webcore/Blob.rs:5565-5576 — The new fstat gate is behind if NEEDS_OPEN, so Bun.write(Bun.file(fifoFd), payload) — the <false> monomorphization selected at 5112/5145 for a lazy mode==0 fd store — still enters the write loop, delivers a partial prefix, hits EAGAIN, and sets needs_async without recording bytes written. The async fallback then adopts the user fd via get_fd, and because this PR's run_with_fd now fstats and sets could_block=true and do_write now actually retries, it rewrites the full payload from total_written=0 — reader receives [pipe-buf prefix] ++ [full payload] while the promise resolves payload.len. Drop the NEEDS_OPEN conjunct (fstat the user fd too and skip straight to needs_async for non-regular files before writing anything; no handoff_fd needed since get_fd already adopts an Fd pathlike) here and at the mirror in write_string_to_file_fast (~5466).
Extended reasoning...

What the bug is

The PR gates the new fast-path fstat check on NEEDS_OPEN (Blob.rs:5466, 5568), so it only fires for path-based destinations. The fd-based sibling — Bun.write(Bun.file(fifoFd), smallStringOrBytes) — still falls into the write loop, writes until the pipe buffer fills, hits EAGAIN, sets needs_async = true after already delivering a partial prefix to the reader, and returns without recording how much was written. The async fallback then rewrites the full payload from byte 0.

Reachability of the <false> fast path

fast_path_ok (Blob.rs:5082-5087) is true for a PathOrBlob::Blob when NOT(store is File with mode != 0 && kind == File). Bun.file(fd) constructs its store lazily with mode = 0 / seekable = None (populated only by resolve_file_stat, which hasn't run yet), so the inner condition is false and fast_path_ok = true. pathlike is Fd, so write_{string,bytes}_to_file_fast::<false> is selected at 5112/5145. The one requirement for EAGAIN to fire is that the user's fd is O_NONBLOCK — which is exactly how you open a FIFO writer without blocking (fs.openSync(fifo, O_WRONLY | O_NONBLOCK)), and a payload in [pipe_buf, 256KB).

Step-by-step
1. User: const fd = fs.openSync(fifo, O_WRONLY | O_NONBLOCK); await Bun.write(Bun.file(fd), "x".repeat(200000)).
2. fast_path_ok = true (mode==0) → write_bytes_to_file_fast::<false> at Blob.rs:5145.
3. NEEDS_OPEN = false, so the new fstat gate at 5568 is skipped.
4. Write loop at 5585-5598: writes ~64 KiB (pipe buffer), then write() returns EAGAIN → *needs_async = true; return ZERO (line 5596-5598). The local written count is discarded.
5. Back in write_file_internal: falls through with the original data JSValue → WriteFile::create with total_written = 0.
6. get_fd (Blob.rs:7209-7213): pathlike is Fd → sets opened_fd = user_fd directly → run_with_fd.
7. run_with_fd (write_file.rs:453-476): file.pathlike.is_fd() = true, file.seekable = None (never resolved) → falls through to this PR's new fstat at line 471-473 → could_block = !is_regular_file(FIFO) = true.
8. do_write_loop_posix starts at total_written = 0 and — with this PR's fixed do_write (write inside the loop, line 336-344) + could_block = true — drives POLLOUT to completion, writing all 200000 bytes.
9. Reader receives ~64 KiB prefix + 200000 bytes = ~264 KiB. Promise resolves written == 200000 as if correct.
Pre-PR vs post-PR

Pre-PR the same input hung: step 4 delivered the same prefix, but step 7's old code left could_block = false (seekable = None fell through to the "We do not call fstat()" comment → false), and the old do_write had sys::write outside the loop, so continue re-matched the stale Err(EAGAIN) forever without ever reaching wait_for_writable(). Loud worker-thread spin, promise never settles.

Post-PR: silent duplicated-prefix corruption with a success resolve. So this PR's own write_file.rs changes convert the fd-sibling from a diagnosable hang into silent data corruption on the exact sibling of the bug being fixed.

Why this matters

REVIEW.md is explicit: "Fix the whole class in the same PR — sync/async twins, fast/slow paths". The <false> monomorphization is the fd twin of the <true> path case this PR fixes; leaving it unfixed while simultaneously making the async fallback work (which it didn't before) trades a hang users would notice for output corruption they won't.

Fix

Drop the NEEDS_OPEN conjunct on the fstat gate (or add a matching else branch for !NEEDS_OPEN) so <false> also fstats and sets needs_async before writing anything. No handoff_fd is needed on this path — get_fd already adopts a user fd via pathlike.is_fd(), and the fd is user-owned so it must not be closed. Apply the same at both sites: write_bytes_to_file_fast (~5568) and write_string_to_file_fast (~5466).

…egular fd Bun.write(Bun.file(nonblockFifoFd), payload) still entered the fast-path write loop, delivered a pipe-buffer prefix, hit EAGAIN, and fell through to WriteFile with total_written=0. With run_with_fd now setting could_block from fstat, WriteFile would then drive the full payload, leaving the reader with prefix+payload. Drop the NEEDS_OPEN conjunct on the fstat gate so a user-supplied fd is checked too; when we did not open it ourselves we skip handoff_fd (WriteFile picks the fd up via pathlike.is_fd()). Also settle the test's drained reader on the failure path and cover the fd destination alongside the path destination.

robobun · 2026-07-27T00:39:34Z

Re the fd-target finding above: reproduced (fast-path prefix + full async replay = 265539 bytes to the reader with a written==200003 resolve) and fixed in d899425. The fstat gate now runs for both monomorphizations; when NEEDS_OPEN is false the handoff fd is left unset and get_fd adopts the user fd via pathlike.is_fd() as before. New (O_NONBLOCK fd dest) test variants cover this alongside the path-destination ones.

claude

No new findings on d899425 — the earlier empty-payload gate, _needs_async naming, and test-cleanup nits are all addressed, and the fd-target double-write is fixed by running the fstat gate in both fast-path monomorphizations. Deferring to a human for the fd-ownership handoff (scopeguarded Cell<Option<Fd>> → unsafe { (*file_copier).opened_fd = fd } across the JS-thread/work-pool boundary) and for robobun's own note that #36028 handles the do_write EAGAIN loop differently (set could_block=true on first EAGAIN vs. this PR's retry-in-loop).

What was reviewed:

get_fd short-circuit on preset opened_fd (Blob.rs:7198) — confirmed the handoff fd is adopted without reopening.
is_allowed_to_close() for the handoff case — pathlike.is_path() is true, so WriteFile closes the fd it adopted; the fast path's CloseOnDrop is declared after the gate returns, so no double-close.
run_with_fd's could_block block — the fd-pathlike/seekable==None case now falls through to the new fstat, so the busy-spin robobun raised is covered.
Scopeguards in write_file_internal and write_file_with_source_destination — every early-return between fast-path open and WriteFile adoption closes the fd.

Extended reasoning...

Overview

The PR fixes Bun.write(fifo, ...) delivering only a torn prefix when the payload exceeds the kernel pipe buffer. It touches four files: the two sync fast paths in Blob.rs (write_string_to_file_fast / write_bytes_to_file_fast) gain an fstat gate that routes non-regular fds to the async path before any bytes go out; a new WriteFileOptions.handoff_fd carries the already-opened fd to WriteFile so the FIFO isn't closed-and-reopened (which gave the reader a premature EOF and the writer ENXIO); write_file.rs's run_with_fd now fstats to set could_block for path-opened fds and do_write moves sys::write inside its retry loop; S3Client.rs just fills the new struct field; the test adds 8 FIFO cases (4 source types × path/fd dest).

Security risks

None identified. The fstat is on an fd we just opened; no user-controlled sizes feed allocations; the unsafe write is to a freshly heap::into_raw'd struct with sole ownership on the JS thread before it is handed to the work pool.

Level of scrutiny

High. This is fd-lifecycle code with ownership crossing three functions via a Cell<Option<Fd>> guarded by two independent scopeguards and an unsafe field write, then executed on a work-pool thread. It also reverses a documented performance decision (the deleted "We do not call fstat() because that is very expensive" comment) — one extra fstat per Bun.write(path, bytes) is almost certainly fine, but a maintainer should sign off on that. And the author flagged a competing PR (#36028) that takes a different approach to the do_write loop; that's a design choice a human should pick.

Other factors

Two prior review rounds from me were addressed (empty-payload gate on the fstat handoff; _needs_async rename; test failure-path cleanup). The fd-target prefix-replay bug the author self-reported was fixed in d899425 by running the fstat gate for NEEDS_OPEN=false too, with new test coverage. The bug-hunting pass on the current head found nothing. Remaining reasons to defer: the handoff plumbing is intricate enough that a second pair of eyes on the scopeguard ordering is warranted, and the #36028 design comparison is an open question the author raised themselves.

robobun · 2026-08-12T12:04:07Z

Still reproduces on current main (9a543cc) and on the 1.4.0 release: with a reader attached that starts draining after 2s, Bun.write(fifoPath, 1 MiB string) is still pending after 6s and the process burns 6.0s of user CPU for 6.0s of wall clock, i.e. a pool thread is spinning on the stale EAGAIN in WriteFile::do_write and never re-issues the write even after the reader drains the FIFO.

The write_file.rs hunks here (fstat in run_with_fd, write(2) inside the retry loop) still merge cleanly onto main. The branch is currently conflicting only in src/runtime/webcore/Blob.rs (the fast-path fd handoff, after #32024 and friends moved that code) and in test/js/bun/io/bun-write.test.js, so this needs a rebase rather than a new PR. Not opening a separate one for the same fix.

robobun assigned Jarred-Sumner Jul 26, 2026

github-actions Bot added the claude label Jul 26, 2026

claude Bot reviewed Jul 26, 2026

View reviewed changes

Comment thread src/runtime/webcore/Blob.rs Outdated

Comment thread src/runtime/webcore/Blob.rs Outdated