fs: back createWriteStream(path) with a FileSink that adopts the fd - #35993
fs: back createWriteStream(path) with a FileSink that adopts the fd#35993robobun wants to merge 32 commits into
Conversation
fs.createWriteStream(path) dispatched every write() to the thread pool via fs.write: each chunk cost one write(2) to the file plus one 8-byte eventfd wake back to the JS thread, with no coalescing. 20000 x 81-byte writes were ~40000 write syscalls and ~12x slower than Node, which batches buffered chunks through _writev. Back the default path-based WriteStream with Bun.file(fd).writer() once fs.open completes. _write and _writev write into the FileSink's in-process buffer and complete synchronously, so the Writable machinery never needs to buffer and there is no thread-pool round-trip per chunk. A _final flushes the sink before 'finish'. flags/mode are honoured because the fd still comes from fs.open, and the fd is closed via the existing close() path after the sink drains. decodeStrings is disabled on this path so UTF-8 strings reach the sink without a Buffer.from round-trip; other encodings are decoded in _write. The FileSink buffer is not enabled when options.fs, options.fd, options.start or a monkey-patched open() is present; those keep the fs.write path. The stdio fast path is unchanged.
|
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 (4)
WalkthroughChangesWriteStream file sink and writev support
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Updated 9:27 AM PT - Jul 27th, 2026
✅ @robobun, your commit 0fe7392801f6250841d2a3f7dd3c4d501aa8a903 passed in 🧪 To try this PR locally: bunx bun-pr 35993That installs a local version of the PR into your bun-35993 --bun |
|
Found 3 issues this PR may fix:
🤖 Generated with Claude Code |
|
Checked each:
|
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
|
Re #31764: covered in the "Related" section above. Same symptom, different layer: #31764 re-enables |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline findings, I also checked: the _writev string path with decodeStrings:false — chunks are converted to Buffer before size is accumulated, so bytesWritten counts bytes; the fallback when Bun.file(fd).writer() throws in streamConstruct — the added Buffer.from(data, encoding) in _write's else branch covers the now-possible string input; and the /proc/self/io syscw test against concurrent tests in the same file — ruled out as a flake source.
Extended reasoning...
This PR reroutes the default createWriteStream(path) through a FileSink, which is a high-traffic Node compat surface with subtle interactions (decodeStrings, autoClose, fd ownership, close ordering). The inline findings — particularly the 2× fd consumption / autoClose:false dup leak on POSIX and the fdClosed race in the syscw test — are substantive enough to warrant a human look before landing. The note above records what else was examined so a follow-up pass doesn't re-derive it.
… batching
Rework of the previous FileSink-backed approach. Bun.file(fd).writer()
dup()s the fd on POSIX, so every WriteStream would hold two fds (and
leak the dup under {autoClose:false}), and FileSink buffers meant data
was not on disk when 'drain' fired, which several upstream Node tests
rely on.
Instead, keep the existing fs.open/close path and write on the JS thread
with a captured writeSync: _write writes its chunk and defers completion
via nextTick so the Writable buffer fills, then _writev concatenates the
batch into a single writeSync. The result is ~2 write(2) per drain cycle
instead of one thread-pool dispatch plus eventfd wake per chunk, data is
on disk before each completion callback, and the stream holds a single
fd.
The sync path is used for the default createWriteStream(path)/fd case
with no options.fs and no options.start. It falls back to the per-chunk
fs.write path at first write when fs.write/fs.writev have been
monkey-patched, so error-injection tests that replace fs.write still see
their patch.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
robobun
changed the title
…p the thread-pool write Also reset the retries counter on a non-zero write, matching the async writeAll/writevAll twins.
|
CI status: Linux green; the two Windows failures on build #83232 (test-file-write-stream2.js and |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/js/internal/fs/streams.ts`:
- Around line 577-595: Update writeAllSync’s deferred completion path to recheck
stream.destroyed before invoking the callback, matching the destroyed-stream and
kIoDone handling used by writeAll and writevAll. Report
$ERR_STREAM_DESTROYED("write") instead of success when destruction occurs after
the synchronous write but before process.nextTick executes.
🪄 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: 27e405ce-08a1-42e5-9c90-f99e111c8b3d
📒 Files selected for processing (2)
src/js/internal/fs/streams.tstest/js/node/fs/fs.test.ts
Match the async writeAll/writevAll twins: if destroy() runs between the synchronous write and the nextTick completion, report ERR_STREAM_DESTROYED to the write callback instead of success.
createWriteStream(fifoPath) or createWriteStream('/dev/stdout') has
fd == null at constructor time but opens a pipe/char-device; writeSync
on the JS thread would block the event loop once the kernel pipe buffer
fills. fstat the fd in _construct's open callback and clear kSyncWrite
unless it is a regular file.
Also run the single-fd test in a subprocess so a GC finalizer closing an
fd from an earlier test cannot skew the /proc/self/fd delta, and add a
FIFO test pinning the event loop stays responsive while a blocking pipe
write is pending.
…cted there [skip size check]
process.exit(0) while a thread-pool fs.write is blocked in write(2) raced VM teardown and panicked with 'enqueueTaskConcurrent: VM has terminated' on the asan lane. Wait for 'alive', then drain the FIFO so the child's pending write completes and it exits on its own. [skip size check]
Drop the writeSync-on-JS-thread approach: isFile() cannot tell an NFS / FUSE / sshfs-backed regular file from a local one, and write(2) on those blocks the event loop for the full network latency. libuv thread-pools all file I/O for this reason. Keep the coalescing win without the regression: stop shadowing the prototype _writev (the own-property undefined set in #16422 was what disabled batching), and have _writev drain the buffered batch as a single thread-pool fs.write on the concatenated buffer so the default path does not depend on fs.writev's IOV_MAX handling. A custom options.fs that supplies writev still gets writevAll. Also fix writevAll passing this.pos instead of its pos parameter on partial-write retries. [skip size check]
[skip size check]
so sink.write() and flush()/AutoFlusher for a regular file also probe pwritev2(RWF_NOWAIT) on Linux, matching writev(). force_sync still issues blocking sys::write. [skip size check]
There was a problem hiding this comment.
Additional findings (outside current diff — PR may have been updated during review):
-
🔴
src/io/PipeWriter.rs:2648-2653— 🔴 075d741's fix for the borrowed-pipe close now hasend()callself.on_close_source()for a borrowedSource::Pipe, butWindowsStreamingWriter::on_close_source(line 2062) doesself.source = None, which drops theBox<uv::Pipe>while theuv_pipe_tis still linked in libuv's loop handle queue (neveruv_close'd) — the exact hazard the comment at PipeWriter.rs:1392-1394 documents ("would Drop the prior Box WITHOUT uv_close, leaving libuv with a dangling handle → UAF on next loop tick"). Reachable viaBun.file(pipeFd).writer().end()on Windows: Blob.rs:1874 setsowns_fd=falseandw.start(fd, true)→Source::open→Source::Pipefor a named-pipe fd. Pre-PR the!owns_fdguard early-returned andDrop→close_without_reporting()→close()uv_close'd via the Pipe arm at :1282-1289. Either fireParent::on_closehere without taking the source (soDropstill uv_close's it), or dup borrowed pipe fds upfront on Windows the way POSIX now dups pollable fds so the sink owns its uv handle.Extended reasoning...
What the bug is
075d741 addressed the earlier review's "b47349c makes
end()uv_close the caller's borrowed pipe handle" comment by havingWindowsStreamingWriter::end()skipclose()for a borrowed non-File source and callself.on_close_source()directly:if !self.owns_fd && !matches!(self.source, Some(Source::File(_) | Source::SyncFile(_))) { // uv_close on a borrowed pipe/tty would close the caller's // handle; fire on_close without touching it. self.on_close_source(); return; }
But
WindowsStreamingWriter::on_close_source(PipeWriter.rs:2062-2069) is:fn on_close_source(&mut self) { self.source = None; ... unsafe { Parent::on_close(self.parent) }; }
For
Source::Pipe(Box<uv::Pipe>),self.source = Nonedrops theBoxand frees theuv_pipe_tallocation. That handle was registered with the loop's handle queue byuv_pipe_init/uv_pipe_openinSource::open_pipeand has never beenuv_close'd, so libuv is left with a dangling handle-queue link and a staleloop->handlesentry. There is noDropimpl onuv::PipeorSourcethat callsuv_close(grep confirms none). This is precisely the hazard the codebase's own comment at PipeWriter.rs:1392-1394 (set_pipe) documents:The assignment below would Drop the prior Box WITHOUT uv_close, leaving libuv with a dangling handle → UAF on next loop tick.
(
Source::TtyisNonNull<uv_tty_t>, a non-owning backref, so dropping it leaks the handle rather than freeing it — the load-bearing UAF is thePipearm.)The specific code path
On Windows,
Bun.file(pipeFd).writer()wherepipeFdis a pipe/named-pipe handle:Blob::get_writer's#[cfg(windows)]branch (Blob.rs:1872-1880, pre-existing, not touched by this PR) doesw.owns_fd = !matches!(pathlike, Fd(_))→false, thenw.start(fd, true).start(fd, true)→Source::open(loop_, fd)→uv_guess_handle(fd)→NamedPipe→Source::open_pipe→uv_pipe_init(loop, pipe)+uv_pipe_open(pipe, fd). The handle is now linked into the loop'shandle_queue;self.source = Some(Source::Pipe(Box<Pipe>)).sink.end()→end_from_js→writer.flush()returnsWrote(0)(no pending data) →writer.end().WindowsStreamingWriter::end():is_done = true;!has_pending_data()→!owns_fd && source is Pipe→ new branch →self.on_close_source().on_close_source:self.source = None→Box<uv::Pipe>deallocated.uv_closewas never called.- Next
uv_runiteration (oruv_walk/uv_loop_close) walksloop->handle_queueand dereferences the freeduv_pipe_t.
The same shape is reachable via this PR's new
createWriteStream(namedPipePath)path whenfs.openreturns a named-pipe fd:FileSink::setup()setsw.owns_fd = false(FileSink.rs:668) andw.start(fd, pollable)createsSource::Pipe.Why existing code doesn't prevent it
on_close_sourceis normally reached only as the terminal step ofclose()— after theSource::Pipearm at :1282-1289 hasinto_raw'd the Box and handed it touv_close(on_pipe_close)(which reclaims and frees it in the close callback). Callingon_close_sourcedirectly with a liveSource::Pipebypasses that step. There is noDrop for uv::Pipethat callsuv_close. And onceself.source = None, the laterWindowsStreamingWriter::Drop→close_without_reporting()seesget_fd() == INVALIDand skips, so nothing ever uv_close's the handle.Why this is a regression
Pre-PR,
end()hadif !self.owns_fd { return; }— the source stayed alive, and when theWindowsStreamingWriterwas later dropped (FileSink::deinit→ dropwriter→Dropat :2660 →close_without_reporting()→close()), theSource::Pipearm at :1282-1289 properlyuv_close'd the handle viainto_raw+on_pipe_close. That closed the caller's fd (a correctness bug this PR set out to fix) and leaked the keep-alive ref — but it was memory-safe. 075d741 trades that for a use-after-free.Step-by-step proof
// Windows const [r, w] = createSocketPair(); // or any named-pipe handle const sink = Bun.file(w).writer(); // owns_fd=false, Source::Pipe(Box<Pipe>) init'd on the loop await sink.end(); // end() → on_close_source() → Box freed, no uv_close setImmediate(() => {}); // next uv_run tick walks handle_queue → deref freed uv_pipe_t
Under ASAN this is a heap-use-after-free in
uv_run; in release it corrupts the loop's intrusive handle list.How to fix
The uv handle's memory must stay alive until a
uv_closecallback fires. Two options:- Don't drop the source here. Add a variant that fires
Parent::on_closewithoutself.source = None(or setclosed_without_reporting = trueand leave the source in place). ThenDrop→close_without_reporting()→close()willuv_closethe handle when the writer is torn down. This restores pre-PR memory safety but re-closes the caller's fd at Drop time — which per the comment at :1376-1381 is unavoidable onceuv_pipe_openhas adopted it. - Dup borrowed pipe fds upfront on Windows, mirroring what
FileSink::setupnow does on POSIX for pollable fds (bun_sys::dup_with_flags→owns_fd = true). Then the sink owns its own handle,end()reaches the ordinaryclose()path, and the caller's fd is never touched.
…dows end() for borrowed Pipe; test error codes - PosixStreamingWriter::writev advances a start index instead of Vec::remove(0). - Default JsSinkType::writev_bytes treats Writable::Backpressure(n) as progress (adds n, continues, returns Backpressure(total) at the end) so an HTTP response controller.writev does not drop tail chunks once the socket backs up. - Windows end() for a borrowed Pipe/Tty returns without dropping the source (Drop -> close() uv_close's it); File/SyncFile still reach close() which honours owns_fd via detach_borrowed_fd. - writev argument-validation test asserts ERR_INVALID_ARG_TYPE. [skip size check]
|
Re the Windows borrowed-Pipe |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/io/PipeWriter.rs`:
- Around line 1008-1017: Update the iovec construction in PipeWriter’s
buffer-writing path to use clippy-approved pointer conversions instead of direct
as casts, preserving the existing buffer addresses and lengths. In the unsafe
block around the write operation near the referenced lines, use the appropriate
raw-pointer conversion that changes constness and add a safety comment
documenting the invariants that make the operation valid.
In `@test/js/bun/util/filesink.test.ts`:
- Around line 182-190: Update the “writev rejects non-ArrayBufferView entries”
test to be async and ensure the writer is always torn down with await sink.end()
in a finally block, preserving both invalid-input assertions while preventing
cleanup errors or pending resources from escaping.
🪄 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: 3160a29c-5ecb-4811-b5da-9d547dfbfd1f
📒 Files selected for processing (10)
packages/bun-types/s3.d.tssrc/codegen/generate-jssink.tssrc/io/PipeWriter.rssrc/js/internal/fs/streams.tssrc/jsc/bindings/headers.hsrc/runtime/webcore/FileSink.rssrc/runtime/webcore/Sink.rssrc/sys/lib.rstest/js/bun/util/filesink.test.tstest/js/node/fs/fs.test.ts
…fd after start(); clippy casts on_auto_flush no longer short-circuits on done.get() so a regular-file Pending after end_from_js (RWF_NOWAIT EAGAIN) is drained by the deferred microtask, and once drained with done set it ends the writer so on_close fires and the keep-alive ref is released. FileSink::setup now writes the writer's close_fd/owns_fd flag after w.start() returns, so start()'s internal close() of the previous handle still uses the previous handle's flag. [skip size check]
…; add ArrayBufferSink.writev type
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/runtime/webcore/Sink.rs (1)
547-578: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winDo not keep
thislive acrossget_length/get_index. Both calls can enter JS (including proxy/getter paths), so a callback can close or mutate the sink beforewritev_bytesruns. Collect and validate the chunks first, then reacquirethisand recheck pending error immediately beforewritev_bytes.🤖 Prompt for 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. In `@src/runtime/webcore/Sink.rs` around lines 547 - 578, The writev flow currently retains the sink across JS-entering get_length and get_index calls. In the method containing this.sink and writev_bytes, collect and validate all chunks before reacquiring this; then call get_this again and recheck get_pending_error immediately before writev_bytes, so callbacks can close or mutate the sink safely.Source: Coding guidelines
🤖 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/FileSink.rs`:
- Around line 702-718: The fallback error paths in FileSink.rs (lines 702-718)
and Blob.rs (lines 1897-1909) manually close the file descriptor while leaving
the writer ownership flag set. Update the corresponding writer-start error
handling to clear close_fd on Unix or owns_fd on Windows after the explicit
close, ensuring teardown cannot close the descriptor again.
In `@test/js/bun/util/filesink.test.ts`:
- Around line 532-563: Guard the named-pipe test beginning with
“Bun.file(fd).writer() on a named pipe” so it is skipped when the platform is
not Windows, using the file’s existing platform-guard convention. Also await a
promise resolved by the server connection’s first data event before asserting
received equals “hello”, and reject it on connection or server errors so the
assertion does not depend on the GC polling loop’s timing.
---
Outside diff comments:
In `@src/runtime/webcore/Sink.rs`:
- Around line 547-578: The writev flow currently retains the sink across
JS-entering get_length and get_index calls. In the method containing this.sink
and writev_bytes, collect and validate all chunks before reacquiring this; then
call get_this again and recheck get_pending_error immediately before
writev_bytes, so callbacks can close or mutate the sink safely.
🪄 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: 36c1aa7d-34d3-4add-9079-3039f23fcdb9
📒 Files selected for processing (6)
packages/bun-types/bun.d.tssrc/io/PipeWriter.rssrc/runtime/webcore/Blob.rssrc/runtime/webcore/FileSink.rssrc/runtime/webcore/Sink.rstest/js/bun/util/filesink.test.ts
…et Drop close on POSIX; test fixes
There was a problem hiding this comment.
No new findings this pass, but this touches fd ownership and writer teardown across POSIX/Windows plus new public writev API on all sinks — worth a human look before merge.
What was reviewed:
- fd ownership:
open_for_writingborrowed-fd path no longer closes on error;close_fd/owns_fdthreading throughsetup()error and success arms; POSIX pollable-fd dup and Windows pipe/ttyDuplicateHandlesoend()reacheson_closeand releases the keep-alive ref. js_writev: accessor getters run beforeget_this/slice extraction;MarkedArgumentBufferroots each item; detached buffers seen as zero-length (test covers this).PosixStreamingWriter::writeviovec advance on partial write stays in bounds;FileType::Piperouted towritev_bufferedso blocking pipes never stall the JS thread.- streams.ts:
kFileSinkgated on!fastPath && fd == null && start === undefined && autoClose !== falseandfs.write === writeat open time;close()ends the sink then falls through to the existingfs.close/flushpath.
Extended reasoning...
Overview
13 files: native fd-ownership semantics for Bun.file(fd).writer() on POSIX (adopt instead of dup for non-pollable fds; dup pollable fds and Windows pipe/tty so owns_fd stays true), a new close_fd flag on PosixStreamingWriter threaded through close()/close_without_reporting(), a new writev() method on all six sink prototypes (codegen + Rust host-fn + .d.ts), PosixStreamingWriter::writev/WindowsStreamingWriter::writev implementations, writev_nonblocking in bun_sys, and a kFileSink fast path in createWriteStream(path) that routes _write/_writev through the sink instead of the thread-pool fs.write.
Security risks
None identified. No auth/crypto/permissions surface. Untrusted-input handling in js_writev (array length, per-item type) is validated before allocation with a MAX_CHUNKS cap; slices are extracted only after all accessor JS has run and rooted, so a getter that detaches a buffer is seen as zero-length rather than dereferenced stale.
Level of scrutiny
High. This changes fd lifetime semantics on a hot Node-compat path, adds cross-platform writer teardown branches that went through three revisions on this PR (UAF → keep-alive leak → dup-before-open), and introduces new public API surface on every sink. The try_write refactor also changes the FileType::File arm from sys::write to sys::write_nonblocking — a real behaviour change for regular-file sinks on Linux (RWF_NOWAIT), which the PR relies on for the on_auto_flush retry loop.
Other factors
The PR has been through ~10 iterations with every prior finding (Windows borrowed-pipe keep-alive leak, on_auto_flush done short-circuit, setup() error-path double-close, js_writev re-entrancy ordering, FileType::Pipe blocking writev) addressed and marked resolved. Test coverage is solid (syscall-count assertion, fd-count subprocess, /dev/full error path, monkey-patch fallback, Windows named-pipe liveCount, detach-via-getter). But the scope — native memory/fd ownership, GC-sensitive host-fn ordering, and a Node-compat behaviour change (regular-file writes now synchronous on the JS thread) — is beyond what I should approve unattended.
2 participants
Fixes #21252
Reproduction
write(2)syscalls100000 x 81-byte: bun 1600 ms -> 59 ms; node 70 ms.
Cause
fs.createWriteStream(path)._writedispatched every chunk to the thread pool viafs.write: eachws.write(line)cost onewrite(2)to the file plus one 8-byte eventfd wake back to the JS thread. Withthis._writev = undefinedset in the constructor (#16422), buffered chunks never coalesced either, so N small writes were ~2Nwrite(2)calls.Fix
FileSink: adopt a caller-supplied fd on POSIX
Bun.file(fd).writer()on POSIX previouslydup()ed the fd (viaopen_for_writing'sFdarm) and closed the dup onend(); on Windows it already adopted the fd withowns_fd = false. This PR brings POSIX in line:PosixStreamingWritergains aclose_fd: bool(mirroringPosixBufferedWriterand the Windows writers'owns_fd), threaded throughclose()/close_without_reporting()intoPollOrFd::close_impl.open_for_writingadopts a borrowed fd instead of dup'ing it and does not close it on an error path.FileSink::setup()records the adopted fd onself.fd(sosink._getFd()returns it instead of -1) and setswriter.close_fd = falsefor fd-backed sinks.So
Bun.file(fd).writer()now holds exactly one caller-owned fd on every platform andsink.end()leaves it open.WindowsStreamingWriter::end()no longer skipsclose()for a borrowed fd, soon_closefires and the keep-alive self-ref taken on the firstPendingwrite is released.FileSink:
writev(chunks: ArrayBufferView[])A new
sink.writev(chunks)method reservesoutgoingcapacity once, appends each slice, and drains once, so a batched drain is onewrite(2)on POSIX (oneuv_fs_writeon Windows) instead of a JS-sideBuffer.concat.WriteStream: route writes through the FileSink
For the default
createWriteStream(path)case (nooptions.fs, nooptions.fd, nooptions.start), wrap the fd thatfs.openreturns inBun.file(fd).writer()and route_writethrough the sink and_writevthrough the newsink.writev. Each call writes into the sink's buffer and flushes so the bytes are on disk before the Writable callback fires (test-file-write-stream.js / test-file-write-stream2.js read the file in the'drain'handler). Deferring the callback vianextTicklets the Writable buffer fill so_writevdrains the batch in one pass, anddestroyedis rechecked before the deferred completion.flags/modeare honoured because the fd still comes fromfs.open, and the existingfs.close/{flush: true}/{autoClose: false}paths are unchanged;close()just ends the sink (a no-op on the fd) first.The sink is not created when
options.fs,options.fdoroptions.startis present, or whenfs.writehas been replaced before the fd opens (so error-injection patches like test-fs-write-stream-err.js still see their patch). Those fall back to the per-chunkfs.writepath.For a pollable fd (FIFO, socket, pipe),
FileSinkwrites via its poll-driven path rather than a blockingwrite(2)on the JS thread, and the flush returns a promise, so the Writable callback is deferred until the bytes drain; the event loop is not blocked. For a regular file on the JS thread the write is synchronous, which matchesBun.file().writer()'s existing contract.Related
Same motivation as #31764; that PR keeps the thread-pool
fs.writevpath and additionally teaches nativefs.writev/fs.readvto batch pastIOV_MAX, which is still independently useful for barefs.writevand foroptions.fsimplementations that forward to it.Verification
New tests in
test/js/node/fs/fs.test.ts:coalesces many small writes instead of dispatching one syscall per chunk(Linux only): 5000 x 81-byte writes, asserts the/proc/self/iosyscwdelta is under 1250 (stock bun: ~10000), file size andbytesWrittenare byte-exact, and the fd is closed after'close'.holds a single fd for the stream's lifetime(Linux only, subprocess):/proc/self/fdcount is exactly +1 while the stream is open and +0 after'close'.many small writes produce byte-exact output and bytesWritten: multi-byte UTF-8 chunks.routes writes through fs.write so a monkey-patch is honoured: a patchedfs.writesees both writes.The first test fails on stock bun (
Received: 10001) and passes with this change. The vendoredtest-fs-write-stream*/test-file-write-stream*scripts,fs.WriteStream/createWriteStreamtests,node-stream.test.js,child-process-stdio.test.js,process-stdio,tty, andbun-writesuites run identically to main.bun run rust:check-allpasses on all targets.no test proof · iteration 10 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/util/filesink.test.ts test/js/node/fs/fs.test.ts