Bun.write: keep stdio writes in call order when stdout/stderr is a regular file - #35949
Bun.write: keep stdio writes in call order when stdout/stderr is a regular file#35949robobun wants to merge 1 commit into
Conversation
…egular file Five un-awaited Bun.write(Bun.stdout, ...) calls with stdout redirected to a file landed as C0 C4 C3 C2 C1. The fast-path eligibility check in write_file_internal excluded fd-backed blobs whose fstat mode was S_IFREG, which is the one case Bun.stdout/Bun.stderr hit under shell redirection. Those writes were instead scheduled onto the WorkPool, whose global run queue is a Treiber stack: the first write runs immediately and the rest are drained in reverse push order. The exclusion dates to the io_uring removal in #7470 and was inverted relative to its own comment (which intended to keep pipes off the main thread). Removing it sends every non-S3 fd blob with offset 0 through write_{string,bytes}_to_file_fast, the same path pipes and ttys already take, so small stdio writes land in call order regardless of what fd 1/2 points at. A blocking pipe still falls through to the async path via needs_async on EAGAIN.
|
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 (2)
Comment |
|
Updated 11:53 AM PT - Jul 26th, 2026
❌ @robobun, your commit 5a03f8c has 1 failures in
Add 🧪 To try this PR locally: bunx bun-pr 35949That installs a local version of the PR into your bun-35949 --bun |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
CI is red on lanes this diff does not touch:
|
|
Closing: #37128 reworks the stdio sinks and lists this PR as superseded ( If something here turns out not to be covered once #37128 lands, this can be reopened. |
2 participants
What's wrong
Un-awaited
Bun.write(Bun.stdout, ...)calls land out of order when fd 1 is a regular file:Mixed with
console.log, all console lines come out first and theBun.writebatch after them, reversed.await-ing each write or usingBun.stdout.writer()stays in order; only the fire-and-forget queue on a file destination drains as a stack. Log/report writers that fire-and-forget to a redirected stdout produce a correctly-sized but reordered file with exit 0.Cause
write_file_internal's synchronous fast path is gated on the destination blob'smode. The condition (added in #7470) excludes fd-backed blobs whose fstat mode isS_IFREG, which is exactly whatBun.stdout/Bun.stderrbecome under> out.txt. Those writes are instead wrapped in aWriteFileTaskand scheduled onto theWorkPool, whose globalrun_queueis a Treiber stack: the first write is picked up immediately by a worker, the remaining four pile onto the stack while that worker is inwrite(), and the worker then drains them LIFO. HenceC0 C4 C3 C2 C1.The guard is inverted relative to its own comment, which says it is meant to keep pipes off the main thread. Pipes, ttys and unknown-mode fds have always taken the synchronous path (and so have always been ordered); only regular-file fds were routed to the thread pool.
Fix
Drop the
modeexclusion so every non-S3 fd blob at offset 0 takeswrite_{string,bytes}_to_file_fast, the same synchronous path pipes and ttys already use. Small writes to a redirected stdout/stderr now complete on the JS thread in call order, with an already-settled promise, matching the pipe case. A pipe that would block still falls through to the async path vianeeds_asynconEAGAIN, which is the existing behaviour.Verification
New tests in
test/js/bun/io/bun-write.test.jsspawn a child that issues fiveBun.write(Bun.{stdout,stderr}, ...)calls with the stream redirected to a file and to a pipe, and assert both that the output isC0..C4and that none of the returned promises were still pending whenBun.write()returned. Againstmain'ssrc/the file cases fail withpending=5and reversed output; with this change all four pass.Windows is skipped:
Bun.write()there always goes throughuv_fs_writeon the libuv thread pool, which can also race writes to a file-backed stdio fd, but via a different mechanism (worker concurrency rather than a LIFO stack) that the POSIX fast path does not touch.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