blob(windows): free CopyFileWindows and ReadFileUV through the task pointer, not under &mut self

[robobun]

Problem

• On Windows, the task behind a file-to-file Bun.write copy and the task behind reading a Bun.file() each free themselves from inside a &mut self method, while callers up the stack still hold a &mut to the same allocation.
• Freeing an allocation while a reference to it is a live argument is undefined behaviour even if the reference is never touched again. Miri (bun run rust:miri) reports it as the strongly protected tag disallows deallocations, and the dereferenceable attribute on the argument lets the optimizer move a load of self past the free.
• No crash is known from these sites. Same family as blob: delete Blob::deinit, which freed the allocation through &mut self #37672, bundler: make Worker::deinit_soon take the worker pointer instead of &mut self #37685, node:fs: let the fs completions own their task box instead of freeing it through &mut self #37693 and install: lifecycle exit returns Disposition; never free self via &mut #37551; these are the two sites in the blob module.

Fix

• The &mut self steps of both tasks no longer free anything. Each returns Pending or Done(result); the entry points that hold the raw task pointer run one step and hand the result to finish(*mut Self), which settles the promise or completion and drops the Box. Teardown (opened descriptors, the libuv request, the event-loop reference) moves into Drop.
• This is correct because the only frame that frees the task holds it as a raw pointer, never as a reference: every &mut self has returned by then, and the back pointers stored in libuv requests are only used after that.
• Also: the read task opens its file through libuv itself (the shared open helper becomes POSIX-only, body unchanged); the mkdirp hop in the copy and write tasks posts to the JS thread through a cloned handle, since the JS thread could free the task mid-post; the copy task holds one event-loop reference for its lifetime instead of a pair per hop.
• Verification: a new source lint fails on main at exactly the three converted sites and passes here; a new copy-with-mode test covers the chmod chain, which nothing ran on Windows before; 13 blob and Bun.write suites pass on Windows (175 tests), the touched tests and lints pass under ASAN on Linux, and a script exercising each converted path prints identical output on the base build and this one. No behaviour is intended to change.

Background

• The blob file tasks are hand-written state machines: a Box is leaked at creation, its raw pointer is stashed in the libuv request's data field, and each libuv completion recovers the pointer and runs the next step. Whichever step ends the operation must free the task exactly once.
• Windows uses these libuv-based tasks (CopyFileWindows, ReadFileUV) instead of the blocking POSIX ones; only the Windows paths change here.
• A &mut self argument is protected for the whole call: Rust promises the callee the allocation stays valid until it returns, so freeing it inside the callee is UB with no later access needed. Miri's Tree Borrows checks this, and it is what dereferenceable tells LLVM.
• The mkdirp hop creates the missing directory on a thread pool, then posts a task back to the JS thread; once posted, the JS thread may run and free the task before the posting call returns.
• test/internal/source-lints/ holds tests that grep the source for banned patterns behind a ratcheting allowlist. bundler: make Worker::deinit_soon take the worker pointer instead of &mut self #37685 and node:fs: let the fs completions own their task box instead of freeing it through &mut self #37693 add the same lint file at the same path, so whichever lands last merges the allowlists.

---

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

Original description

Problem

Two of the Windows blob tasks free themselves from inside &mut self methods:

• CopyFileWindows::throw(&mut self) and resolve_promise(&mut self) (src/runtime/webcore/blob/copy_file.rs) end in Self::destroy(ptr::from_mut(self)), an unconditional heap::take. They are reached from further &mut self frames: copyfile, prepare_read_write_loop, mkdirp, on_mkdirp_complete, on_complete, on_read_write_loop_complete, and the &mut the four libuv callbacks and init formed to call them.
• ReadFileUV::on_finish(&mut self) (src/runtime/webcore/blob/read_file.rs) ends in Self::finalize(ptr::from_mut(self)), also a heap::take. Above it sit on_file_open(&mut self), queue_read(&mut self), the libuv callbacks, and on the open path FileOpener::get_fd(&mut self, callback) / get_fd_by_opening(&mut self, ..) in src/runtime/webcore/Blob.rs, whose own comment notes that callback may free self.

A reference argument has to stay valid until the call it was passed to returns. Freeing the allocation underneath one is undefined behaviour under both aliasing models whether or not the reference is touched again (Stacked Borrows: deallocating while item is strongly protected; Tree Borrows, which bun run rust:miri uses: the strongly protected tag disallows deallocations), and the protector is the model of the dereferenceable attribute rustc puts on reference arguments, so the optimizer is entitled to move a load of self past the free. No crash is known from these sites. Same family as #37672 (Blob::deinit), #37685 (bundler Worker), #37693 (node:fs tasks) and #37551 (lifecycle scripts); these are the two sites in the blob module.

Fix

Both state machines get the same shape: the &mut self steps never free, they return a #[must_use] Step, and the entry points that hold the heap pointer run one step through it and hand the result to finish(this: *mut Self). On Step::Done, finish turns the pointer back into the Box the constructor leaked and drops it; teardown is the types' Drop impls (ReadWriteLoop queues aio::Closer closes for the two descriptors it opened, which are always libuv-owned, CopyFileWindows / ReadFileUV clean up their libuv request and release the loop reference their constructor took, and the store references, the promise handle and, for a read that never finished, the self-cancelling completion go with the field drops). The destroy / finalize routines are gone; heap::take in the two finish functions is the only place either task's allocation is touched as a whole.

CopyFileWindows: copyfile, prepare_read_write_loop, mkdirp, on_mkdirp_complete, on_complete and on_read_write_loop_complete return Step::Pending (a libuv request or the mkdirp pool task is in flight) or Step::Done(Result<usize, bun_sys::Error>). throw and resolve_promise, which differed only in which way they settled the promise, become finish, which keeps their order (take the promise handle, build the error instance, enter the event loop scope, drop the task, then resolve or reject). The bodies of the four libuv callbacks become &mut self steps (on_copyfile_complete, on_chmod_complete, on_read_complete, on_write_complete) and the extern "C" thunks all go through one on_uv_complete that recovers the task pointer from req.data, runs the step and calls finish; init and the ManagedTask hop for mkdirp do the same. init still reads the promise value before running copyfile, so a synchronous failure still settles and frees before init returns. The per-hop ref_keep_alive / unref_keep_alive pairs (copyfile, mkdirp, chmod, the read/write loop) become one reference taken in init and released by Drop, which is what ReadFileUV already did: whenever the task exists across a return to the loop one of those hops is in flight, so the loop sees the same count at every point where it matters, and a synchronous failure now takes and releases the reference inside init where it previously touched neither.

ReadFileUV: on_file_open, queue_read, on_finish and the bodies of the stat / read callbacks return Step::Pending or Step::Done; finish delivers the result to the completion and drops the task (the old finalize, minus the manual request cleanup and unref, which Drop does). Opening moves into the task: get_fd(&mut self) -> Step uses the store's fd directly or queues uv_fs_open with the same mode and error mapping the trait used (the flags become libuv::O::RDONLY, which is what uv::O::from_bun_o made of the trait's RDONLY | NONBLOCK | CLOEXEC on Windows anyway, spelled the way WriteFileWindows::open spells its flags), and its completion continues in on_file_open. The trait's get_fd was the top of the freeing chain on Windows and ReadFileUV was its only Windows caller (WriteFileWindows already opens through libuv itself), so FileOpener loses its Windows-only hooks (loop_, req, set_open_callback, open_callback, the libuv half of get_fd_by_opening) together with the unreachable!() stubs for them on ReadFile and WriteFile; get_fd / get_fd_by_opening become #[cfg(not(windows))] and their POSIX body is unchanged (the diff of Blob.rs with whitespace ignored adds two cfg attributes and a doc comment). ReadFileUV also drops the open_callback field that only existed to carry the trait's callback across the hop.

The back pointers the steps store (io_request.data, req.data, the mkdirp completion_ctx) are still taken from the &mut self of the step that schedules the request. That stays sound: the reference's protector ends when the step returns, nothing touches the allocation through an older pointer afterwards, and the completion that later reclaims the box through that pointer does so with no reference argument live. What moved is the free: it now happens in a frame whose only handle on the task is the raw pointer, and it is a Box drop.

One more site in the same module, found in review: the pool half of the mkdirp hop (on_mkdirp_complete_concurrent, in both CopyFileWindows and its WriteFileWindows twin in write_file.rs) posted the JS-thread hop through the LoopHandle stored in the task, so post_task's &self pointed into the task while the JS thread was already free to pop the hop and free it. Both now write the error through the raw pointer and post through a clone of the handle, which is what post_job does for the same reason, so nothing pointing into the task is live across the post.

No behaviour is intended to change. Every settle/complete happens in the same order relative to the free as before (copy: queue the closes and free, then settle, with script running when the event-loop scope exits; read: deliver the result, then release the store, the request and the loop reference).

Tests

• test/internal/source-lints/self-receiver-teardown.test.ts bans handing the receiver's address (ptr::from_mut(self), self as *mut _, &raw mut *self, addr_of_mut!(*self), NonNull::from(self)) to a destroy / deinit / finalize, directly or through a scopeguard, checks the patterns against positive and negative examples, and ratchets an allowlist. On main it reports exactly

  src/runtime/webcore/blob/copy_file.rs:1652: Self::destroy(core::ptr::from_mut(self))
  src/runtime/webcore/blob/copy_file.rs:1739: Self::destroy(core::ptr::from_mut(self))
  src/runtime/webcore/blob/read_file.rs:1127: Self::finalize(core::ptr::from_mut(self))
  

  The file is the one bundler: make Worker::deinit_soon take the worker pointer instead of &mut self #37685 and node:fs: let the fs completions own their task box instead of freeing it through &mut self #37693 add at the same path, deliberately: whichever of the three lands last resolves the conflict by keeping one copy and deleting the entries for conversions that have landed, instead of landing a stale ratchet on main. This copy is theirs plus finalize in the name set (the ReadFileUV line is that shape; an in-place finalize(&mut self) is never called with the receiver's address, so it does not match), with the allowlist holding the sites owned elsewhere at their current counts: src/install/lifecycle_script_runner.rs (5, install: lifecycle exit returns Disposition; never free self via &mut #37551), src/bundler/ThreadPool.rs (1, bundler: make Worker::deinit_soon take the worker pointer instead of &mut self #37685), and src/runtime/node/node_fs.rs (3, node:fs: let the fs completions own their task box instead of freeing it through &mut self #37693). The entry this copy carried for src/runtime/webcore/ArrayBufferSink.rs was dropped after JSSink: pass the sink to finalize as *mut instead of &mut #37716 landed (the rebase onto it is what the ratchet is for: the count went to 0 and the test failed until the entry was removed).

• test/js/bun/io/bun-write.test.js gets a file-to-file copy with { mode } into an existing and into a missing directory, asserting the mode and the contents. This is the on_complete -> chmod request -> on_chmod_complete -> finish chain (and, for the second copy, the mkdirp hop before it), which no test ran on Windows before; this file already re-runs itself on Windows with BUN_FEATURE_FLAG_DISABLE_UV_FS_COPYFILE=1, so both the uv_fs_copyfile and the read/write-loop variants of the chain are covered. 0o444 is the one mode every platform reports back exactly.

• test/js/bun/util/bun-file-fd-read.test.ts now runs on Windows too; on Windows those reads are ReadFileUV::get_fd's fd branch, which had no coverage there.

Verification

• cargo check --workspace for the host and for x86_64-pc-windows-msvc / aarch64-pc-windows-msvc; cargo clippy -p bun_runtime on the host and rustfmt --check on the touched files are clean. (The Windows-only code in these files has pre-existing clippy findings that CI does not lint; this change does not add any.)
• Windows (x64, debug build): test/js/bun/io/bun-write.test.js (41 pass, including the nested run without uv_fs_copyfile), bun-write-leak, test/js/bun/util/bun-file{,-read,-windows,-exists}.test.*, bun-stdin-slice, bun-file-fd-read (3 pass), and test/js/web/fetch/blob{,-cow,-write,-file-name-ownership}.test.ts plus utf8-bom (114 pass); no failures. In addition a script exercising each converted path (read of a missing path, a directory, an empty file, a regular file, a slice, an fd, an empty fd, NUL, a closed fd; copies to a path, with mode, with mode plus a missing directory, from a missing source with and without a missing destination directory, into a missing directory with createPath: false, onto a directory, from a directory, onto and from a directory fd, onto a file fd) prints identical results (resolved values, error codes, syscalls and paths) on the base build and on this build, in both copy modes. The two pre-existing oddities it shows (the copy resolving with 0 bytes in uv_fs_copyfile mode, Bun.write: fix file-to-file copy resolving 0 bytes on Windows and macOS overwrite #33715, and UV_-prefixed codes in the read/write loop, Report EBADF instead of UV_EBADF for async libuv fs errors on Windows #37465) are unchanged. The same suites and the same script were re-run after the switch to Drop (175 pass across the 13 files, script output again identical to the base build in both modes), plus a script with no top-level await that starts a copy into a missing directory with { mode }, a read of a missing file and a copy from a missing source: in both copy modes the process stays alive across the mkdirp pool hop (the only hop during which nothing but the task's own loop reference keeps the loop alive), all three settle, beforeExit fires and it exits 0, so the single reference is neither dropped early nor leaked. After the review fixes (cloned handle on the mkdirp hop, ReadWriteLoop::drop collapsed to the Closer call) the suites were run once more (175 pass), the script is still identical to the base build in both modes, and the no-top-level-await script still shows both mkdirp hops completing and the process exiting 0 in both modes. The final head (72707d3) was checked the same way once more against a fresh build of the current base commit: the per-path script is identical in both modes and the 13 suites pass (175 pass).
• Linux (debug, ASAN): bun-write.test.js, bun-file-fd-read.test.ts and bun test test/internal/source-lints/ (18 files) pass; the new lint fails with the two source files restored to main as shown above.

Textual overlap only: #37072 touches the throw / resolve_promise lines that became finish, and #31844 changes the lifetimes on these same structs.

[coderabbitai]

Walkthrough

Summary

Windows Blob reads and copies now use Step-based libuv flows with centralized completion and task cleanup. POSIX file opening remains isolated. New source-lint and JavaScript tests validate teardown safety, Windows fd reads, and copied file modes.

Changes

Windows Blob I/O

Layer / File(s)	Summary
Platform-specific file opening contract src/runtime/webcore/Blob.rs	FileOpener documents POSIX-only opening and retains the POSIX retry, directory creation, error, and callback flow.
Windows read state flow src/runtime/webcore/blob/read_file.rs	ReadFileUV chains open, stat, read, and close operations through Step, shared callbacks, and destructor-managed cleanup.
Windows copy and write state flow src/runtime/webcore/blob/copy_file.rs, src/runtime/webcore/blob/write_file.rs	Copy, fallback read/write, mkdirp, chmod, and completion paths return Step values and centralize task ownership and promise settlement.
Teardown and Windows I/O validation test/internal/source-lints/self-receiver-teardown.test.ts, test/js/bun/io/bun-write.test.js, test/js/bun/util/bun-file-fd-read.test.ts	Source lint coverage detects self-receiver teardown calls. JavaScript tests cover copied modes and Windows fd-backed reads.

Possibly related PRs

• oven-sh/bun#37703: Adds related Rust teardown-safety linting in another runtime component.
• oven-sh/bun#37716: Adds related source-lint checks for unsafe teardown patterns.
• oven-sh/bun#37787: Refactors related ReadFile/WriteFile and Blob.rs I/O lifecycle handling.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly identifies the Windows blob tasks and the raw-pointer teardown change.
Description check	✅ Passed	The description explains the change and provides extensive verification details, although it uses different headings than the template.

---

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

[robobun]

^{Updated 10:35 AM PT - Aug 12th, 2026}

❌ @robobun, your commit 72707d3 has 1 failures in Build #93131 (All Failures):

• test/js/node/worker_threads/worker-transfer-terminate-stress.test.ts - unchecked exception at JSC__JSModuleLoader__loadAndEvaluateModule @ src/jsc/bindings/bindings.cpp:3788 (thrown from vendor/WebKit/Source/JavaScriptCore/runtime/VMTraps.cpp:443) on 🐧 13 x64-asan

---

🧪   To try this PR locally:

bunx bun-pr 37705

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

bun-37705 --bun

[robobun]

Status: fixed; teardown reworked as Drop per review, self-review and bot findings folded in (421d0f4, 72707d3), rebased onto main; waiting on re-review.

Reproduced how: the new lint in test/internal/source-lints/self-receiver-teardown.test.ts reports exactly the three sites on main (copy_file.rs:1652 and :1739 Self::destroy(core::ptr::from_mut(self)), read_file.rs:1127 Self::finalize(core::ptr::from_mut(self))) and is clean with this branch. The converted paths were run on a Windows x64 debug build (bun-write in both copy modes, Bun.file and blob suites, plus a per-path script whose output is identical on a build of the base commit and on this branch); the final head was re-checked the same way. Details in the description.

CI for the final head (build 93131): 180 of 181 jobs passed, every Windows lane included. The one failed job is the x64-asan test lane, whose only red test is test/js/node/worker_threads/worker-transfer-terminate-stress.test.ts, a worker-termination exception-check assertion that this diff does not touch (the Linux build of this branch differs from main only by whitespace and cfg attributes in Blob.rs); it is intermittent on that lane across builds of this PR (green on build 92948 with the same source files) and has been reported for main. Everything else in the build is retry-passed flakes.

[claude]

I reviewed this PR and didn't find any bugs. Because it's a ~600-line refactor of unsafe raw-pointer / libuv lifecycle code in the Windows blob tasks, and it deliberately conflicts with the sibling conversions (#37685, #37693) on the shared source-lint file, a human look would still be worthwhile.

What was reviewed:

• Traced every Step-returning path in both state machines to confirm each terminal branch reaches finish exactly once and each Pending has a completion that runs the next step.
• Checked ReadFileUV::get_fd preserves the old trait's flags (RDONLY | NONBLOCK | CLOEXEC, DEFAULT_PERMISSION), fd-branch handling, and error mapping; do_close for ReadFileUV always returns false on Windows so on_finish never returns a dangling Pending.
• Confirmed CopyFileWindows::finish keeps the old throw/resolve_promise order (swap promise → build value → enter scope → destroy → settle) and that promise.swap() yields a GC-cell reference that survives destroy(this).

Extended reasoning...

Overview

This PR restructures the two Windows-only blob async state machines — CopyFileWindows (in copy_file.rs) and ReadFileUV (in read_file.rs) — so that no &mut self method frees its own receiver. Each step now returns a #[must_use] Step enum, and the entry points that hold the raw heap pointer (libuv completions, init/start_with_ctx, the mkdirp ManagedTask hop) run one step and hand the result to finish(*mut Self), the sole free site. The FileOpener trait loses its Windows-only hooks (loop_, req, set_open_callback, open_callback) since ReadFileUV was their only user and now opens through libuv itself; get_fd/get_fd_by_opening become POSIX-only. write_file.rs just drops the unreachable!() stubs. It also adds a source-lint test that ratchets the remaining sites owned by sibling PRs, a new { mode } copy test, and un-skips the fd-read test on Windows.

Security risks

None. This is an internal memory-model soundness refactor of file-copy/read state machines. No user input parsing, auth, crypto, or trust boundaries are touched.

Level of scrutiny

High. This is exactly the category REVIEW.md flags as most-blocked: raw-pointer lifetime, heap ownership, libuv FFI, and &mut self aliasing on Windows-only paths that most reviewers can't run locally. The change is careful and the PR description's differential-output verification against a base build is convincing, but the diff is large enough (four libuv callback bodies moved into &mut self steps, two throw/resolve_promise collapsed into one finish, the open path moved out of the trait) that a maintainer familiar with the sibling conversions should confirm the shape matches what landed in #37672/#37693.

Other factors

The new source-lint file is intentionally the same path as #37685/#37693 add, with an allowlist covering the other PRs' sites at their current counts — whichever lands last must reconcile. The PR also notes textual overlap with #37072 and #31844 on the same structs. These coordination points argue for a human in the loop. The bug-hunting system found nothing; my own trace of every Step return, the open-flag equivalence, the do_close return on Windows, and the finish ordering vs. the old throw/resolve_promise all check out.

[github-actions]

This PR may be a duplicate of:

1. node:fs: let the fs completions own their task box instead of freeing it through &mut self #37693 - Adds the same new test/internal/source-lints/self-receiver-teardown.test.ts (near-identical content) for the same "free through the task pointer, not &mut self" conversion, and its allowlist pins src/runtime/webcore/blob/copy_file.rs: 2 — the exact violations this PR removes.
2. bundler: make Worker::deinit_soon take the worker pointer instead of &mut self #37685 - Adds the same new test/internal/source-lints/self-receiver-teardown.test.ts (an earlier revision of the identical lint) with the same copy_file.rs: 2 allowlist entry, so the two versions of the file directly conflict.
3. blob: delete Blob::deinit, which freed the allocation through &mut self #37672 - Same blob-module soundness conversion ("stop freeing the allocation through &mut self"), touching src/runtime/webcore/Blob.rs and adding a sibling source-lints/self-receiver-*.test.ts guard.

🤖 Generated with Claude Code

[robobun]

Not a duplicate of #37685, #37693 or #37672: each of those converts a different site (the bundler Worker, the node:fs tasks, Blob::deinit), and none of them touches copy_file.rs or read_file.rs; #37685 and #37693 allowlist copy_file.rs at its current count precisely because the conversion is here. The only shared file is the lint, which all three add at the same path on purpose so the last one to land gets a merge conflict to resolve (keep one copy, drop the entries for conversions that have landed) rather than a stale ratchet on main. This copy is #37693's plus finalize in the name set, which is what the read_file.rs site was spelled as.

[robobun]

Heads-up on the allowlist: #37716 converts the JsSinkType::finalize hop (and the generated thunk above it) to *mut, removing the src/runtime/webcore/ArrayBufferSink.rs line this lint carries at 1. Whichever side lands second should drop that entry; the same applies to the copies of this lint in #37685 and #37693.

[Jarred-Sumner]

Can we use a RAII type?

[robobun]

Done in ebd6f3c (comments shortened in 6d47e67): teardown is now Drop.

• ReadWriteLoop closes the descriptors it opened in Drop (was close() called from destroy).
• CopyFileWindows and ReadFileUV implement Drop for the libuv request cleanup and the loop reference; store refs, the promise handle and the self-cancelling completion were already RAII fields. The copy task now takes one loop reference in init and releases it in Drop instead of a ref/unref pair around each hop (same thing the read task already did; some hop is always in flight while the task outlives a return to the loop, so the count the loop sees is unchanged).
• destroy() and finalize() are gone. On Step::Done, finish does heap::take(this) and drops the box (explicitly, before settling the promise, so the descriptors are closed before script runs); the promise is taken out as its owning Strong rather than swapped through a raw JSPromise.

What stays raw is the identity pointer libuv holds between hops (req.data), since that has to be a raw pointer, and finish(this: *mut Self), which is where it turns back into a Box. I did look at reclaiming the Box at every callback entry and re-leaking it on Pending, but that re-leak would invalidate the back pointers the step just stored under Stacked Borrows, so the box is only rebuilt once, at the end. If you had a different type in mind, happy to adjust.

Re-verified on Windows after the change: the same suites (175 pass across 13 files, bun-write in both copy modes), the per-path script is still byte-identical to the base build in both modes, and a no-top-level-await script shows the process staying alive across the mkdirp pool hop and exiting once the promises settle (so the single reference is neither dropped early nor leaked). Description updated.

[claude]

I reviewed this PR and didn't find any bugs. Because it restructures two Windows-only libuv state machines end to end (new Step return contract, Drop impls, moving ref_keep_alive/unref_keep_alive to init/Drop, and inlining the open path into ReadFileUV), a human look would still be worthwhile.

What was reviewed:

• ref_keep_alive accounting: one ref at init/start_with_ctx, one unref in Drop; every Step::Pending leaves the task allocated and every Step::Done reaches finish → heap::take → Drop, so the pair is balanced on all paths including synchronous failure.
• ReadFileUV::on_finish returning Step::Pending on do_close: close_after_io() is hard-coded false and io_request() is None, so do_close always returns false on Windows — no unpaired Pending.
• CopyFileWindows::finish ordering (take promise, build error, enter scope, drop task, settle) matches the old throw/resolve_promise destroy-then-settle order.
• The removed FileOpener Windows hooks had ReadFileUV as their only real caller; ReadFile/WriteFile stubs were unreachable!().

Extended reasoning...

Overview

This PR converts CopyFileWindows (src/runtime/webcore/blob/copy_file.rs) and ReadFileUV (src/runtime/webcore/blob/read_file.rs) so that no &mut self frame frees its own allocation. Each step method now returns a #[must_use] Step (Pending / Done), and a single unsafe fn finish(this: *mut Self, step) — reached only from raw-pointer entry points (init, on_uv_complete, the mkdirp ManagedTask hop, start_with_ctx) — performs heap::take. Teardown moves into Drop impls (fd close for ReadWriteLoop, io_request.deinit() + unref_keep_alive() for both tasks). The Windows arm of FileOpener::get_fd[_by_opening] and its trait hooks are removed and the libuv open is inlined into ReadFileUV::get_fd. write_file.rs drops the now-dead unreachable!() stubs. A source-lint test bans the destroy/deinit/finalize(ptr::from_mut(self)) shape with a ratcheted allowlist, a bun-write test covers the copy→chmod chain (both copy modes), and the fd-read test is un-skipped on Windows.

Security risks

None identified. This is an aliasing-model soundness refactor of file-copy/read paths; no auth, crypto, or untrusted-input parsing surface changes. Error message construction and path handling are moved but not altered.

Level of scrutiny

High. This is ~500 lines of restructuring in memory-safety-critical Windows I/O with subtle Stacked/Tree Borrows reasoning about provenance and reference protectors. It changes when ref_keep_alive/unref_keep_alive fire (from per-hop to once-per-task via Drop), introduces two new Drop impls that close descriptors, and rewrites every libuv callback body. The PR description documents thorough Windows verification (test suites plus a per-path differential script against the base build), but the scope and the number of behavior-preservation obligations put it well outside auto-approval.

Other factors

• The comment-cop bot left a batch of "paragraph-long comment" flags after the last commit (6d47e67d, which already shortened comments). The remaining doc/SAFETY comments look proportionate to what they document (Step, finish # Safety, on_uv_complete provenance note), but a maintainer should confirm whether the bot's threshold is satisfied.
• The lint file is intentionally shared with #37685/#37693; whichever lands last resolves the allowlist, so merge order matters.
• Spot-checked the read_write_loop.close() → Drop conversion: the dropped must_close_* = false / *_fd = INVALID resets and read_buf = Vec::new() are dead in Drop (fields are about to be freed), so their removal is fine.
• on_mkdirp_complete_concurrent's Posted::Refused path still leaks the CopyFileWindows (it only frees the ConcurrentTask), but that is pre-existing behaviour and unchanged by this PR.

[robobun]

Rebased onto main (b18b9b8). #37716 landed in the meantime and removed the ArrayBufferSink line, so the lint's ratchet failed on the rebased tree until that allowlist entry was deleted, which is the intended flow; the three remaining entries (#37551, #37685, #37693) are still open. The Windows suites and the per-path script were re-run on the rebased head with the same results (175 pass across 13 files, script output identical to the base build in both copy modes).

One note on the review's last point, since the new Drop could make it look like an omission: the Posted::Refused branch in on_mkdirp_complete_concurrent (VM already torn down while the mkdirp was on the pool) still leaks the copy task on purpose. It runs on the pool thread and the loop it would unref is gone, and ReadWriteLoop's close goes through the loop's closer too, so dropping the box there is not an option; that path is unchanged from before.

[claude]

I reviewed this PR and didn't find any bugs. Because it's a substantial restructuring of the Windows CopyFileWindows / ReadFileUV state machines — teardown moved to Drop, per-hop ref_keep_alive collapsed into one, finish ordering relative to promise settlement, and the FileOpener Windows hooks removed — a human look from someone familiar with the blob module would still be worthwhile.

Checked that every step returning Step::Pending has a completion that reaches finish (including on_finish's do_close branch, which is unreachable on ReadFileUV since close_after_io() is false and io_request() is None).
Checked the inlined ReadFileUV::get_fd preserves the trait's open flags (RDONLY | NONBLOCK | CLOEXEC), mode, and error mapping; set_open_error only runs on the path branch so .path() is safe.
Checked CopyFileWindows::finish keeps the old order (take promise, build error, enter scope, drop task → close descriptors, then settle) and that init reads the promise value before finish can free the task.

Extended reasoning...

Overview

This PR converts two Windows-only blob I/O tasks (CopyFileWindows in src/runtime/webcore/blob/copy_file.rs and ReadFileUV in src/runtime/webcore/blob/read_file.rs) from a shape where &mut self methods free the task via Self::destroy(ptr::from_mut(self)) / Self::finalize(ptr::from_mut(self)) — UB under Stacked/Tree Borrows because the reference argument is protected — to a Step-returning state machine where only a raw-pointer finish(this: *mut Self) reclaims the Box. Teardown moves to Drop impls (ReadWriteLoop closes its descriptors; the two task types deinit their libuv request and release the loop reference). The four libuv callbacks become one-line thunks through a shared on_uv_complete. The FileOpener trait loses its Windows-only hooks (loop_, req, set_open_callback, open_callback) since ReadFileUV was the only Windows caller and now opens through libuv itself; get_fd/get_fd_by_opening become #[cfg(not(windows))]. A new source lint (test/internal/source-lints/self-receiver-teardown.test.ts) ratchets the pattern, and two behavioural tests are added/enabled (copy-with-mode+mkdirp in bun-write.test.js, fd-backed reads on Windows in bun-file-fd-read.test.ts).

Security risks

None identified. This is an internal soundness refactor of file-copy/read state machines; no new user-facing surface, no parsing of untrusted input, no auth/crypto/permissions.

Level of scrutiny

High. This is the memory-safety category REVIEW.md flags as most-blocked: raw-pointer lifetime across libuv callbacks, Drop ordering, reference-count balancing (the per-hop ref_keep_alive/unref_keep_alive pairs are consolidated into one taken in init and released by Drop), and provenance-sensitive back-pointer stashing in req.data. Nearly all of it is #[cfg(windows)], so it isn't exercised by the default Linux build. The control-flow reshaping is faithful to the original as far as I traced (each old throw/resolve_promise/finalize site maps to a Step::Done returned to a finish caller that holds the heap pointer), and the PR description documents byte-identical output on a Windows debug build across both copy modes plus a keep-alive script — but the ref-count consolidation and the Drop-driven descriptor close ordering are exactly the sort of non-local invariants a maintainer should confirm.

Other factors

The bug hunting system found nothing. The comment-cop bot flags are all resolved (comments shortened or pre-existing). The lint file is intentionally shared with #37685/#37693/#37716 and the allowlist will need reconciling on merge, which the description calls out. Given the size (~650 lines of state-machine restructuring in unsafe Rust) and the design decisions involved (Step pattern, single loop reference, FileOpener trait surface reduction), this is beyond what should land without a human sign-off.

[robobun]

Self-review pass done; two findings survived, both fixed in 421d0f4:

1. The pool half of the mkdirp hop (on_mkdirp_complete_concurrent) posted the JS-thread hop through the LoopHandle stored inside the task, so post_task's &self pointed into the task while the JS thread was already free to pop the hop and free it: the same reference-outlives-the-free class this PR is about, one frame over. It now writes the error through the raw pointer and posts through a clone of the handle (what post_job does for the same reason). The WriteFileWindows twin in write_file.rs had the identical shape and gets the identical change.
2. The comment on drop(task) in finish claimed the descriptors are closed before script can see the settlement; on Windows the drop only queues uv_fs_close, and script runs when the scope guard drops anyway. Comment and description now say what actually holds (same teardown-then-settle order as before). While there, ReadWriteLoop::drop lost its make_libuv_owned fallback arms, which were unreachable because prepare_pathlike only ever stores libuv-owned descriptors, so it is now just the two Closer calls.

Re-verified on Windows after the change: same suites (175 pass across 13 files, bun-write in both copy modes), per-path script still identical to the base build in both modes, and the no-top-level-await script still shows both mkdirp hops completing and the process exiting 0 in both modes.

[coderabbitai]

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/blob/read_file.rs`:
- Around line 1059-1068: Update the uv_fs_open invocation in Self::on_open’s
surrounding open flow to translate the Bun flags through
libuv::O::from_bun_o(...) before passing them as the open flags. Preserve the
existing read-only and permission behavior while ensuring Windows receives
libuv-compatible flags instead of raw NONBLOCK and CLOEXEC bits.

🪄 Autofix

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: c9b22f1d-685a-450e-96d1-0909958a6de3

📥 Commits

Reviewing files that changed from the base of the PR and between 626034f and 421d0f4.

📒 Files selected for processing (7)

• src/runtime/webcore/Blob.rs
• src/runtime/webcore/blob/copy_file.rs
• src/runtime/webcore/blob/read_file.rs
• src/runtime/webcore/blob/write_file.rs
• test/internal/source-lints/self-receiver-teardown.test.ts
• test/js/bun/io/bun-write.test.js
• test/js/bun/util/bun-file-fd-read.test.ts

[claude]

I reviewed this PR and the bug-hunting pass found no issues. Because it restructures the teardown/lifetime of two Windows libuv state machines (heap ownership, Drop impls, event-loop keep-alive accounting, cross-thread mkdirp hop) across ~700 lines of unsafe Rust, a human look is still worthwhile — the author is also flagged as waiting on re-review after the Drop rework.

What was reviewed:

• The Step/finish(*mut Self) shape in both tasks — every &mut self step returns Step and only the raw-pointer entry points (on_uv_complete, init/start_with_ctx, call_erased) reclaim the box; no path frees under a live reference.
• Keep-alive balance: the single ref_keep_alive in init/start_with_ctx is released by Drop; checked that every hop that used to bracket its own ref/unref is covered while the task is live, and that a synchronous failure in init still drops it.
• ReadWriteLoop::drop losing make_libuv_owned() — prepare_pathlike already produces libuv-owned fds, so passing them straight to Closer::close is equivalent.
• on_mkdirp_complete_concurrent cloning loop_handle before post_task (both here and the matching change in WriteFileWindows) so nothing borrows the task across the post.

Extended reasoning...

Overview

The PR converts CopyFileWindows and ReadFileUV (Windows-only libuv-backed blob tasks) so they no longer free their own heap allocation from inside &mut self methods, which is UB under Stacked/Tree Borrows. Each state machine's steps now return a #[must_use] Step; the C-callback thunks and init/start_with_ctx hold the raw *mut Self, run one step, and hand the result to finish(this: *mut Self), which reclaims the Box on Step::Done. Teardown moves into Drop impls (ReadWriteLoop closes fds it opened; the tasks deinit their libuv request and release the loop keep-alive). The Windows half of FileOpener::get_fd/get_fd_by_opening and its unreachable!() stubs on ReadFile/WriteFile are deleted, with ReadFileUV opening through libuv itself. on_mkdirp_complete_concurrent in both copy_file.rs and write_file.rs now clones the LoopHandle out before posting so the pool thread holds nothing pointing into the task across post_task. A new source lint (self-receiver-teardown.test.ts) ratchets the pattern out of the tree, and two behavioural tests are added/unskipped.

Security risks

None identified. This is internal lifetime/ownership plumbing for file I/O tasks; no user-input parsing, auth, or trust-boundary changes. The risk class is memory safety (use-after-free, double-free, refcount leak/imbalance), not security in the injection/bypass sense.

Level of scrutiny

High. This is exactly the category the repo's review guidance treats as most-blocked: pairing acquisition with release, exactly-one-owner, refcounts balanced on every terminal path, and cross-thread lifetime. The change touches ~700 lines of #[cfg(windows)] unsafe Rust with libuv FFI, replaces per-hop ref_keep_alive/unref_keep_alive bracketing with a single ref taken at construction and released in Drop, changes what ReadWriteLoop's close path does (dropping the make_libuv_owned() fallback), and reorders promise-settle vs. task-drop in finish. Each of these is individually reviewable, and the PR description walks through them carefully with Windows verification, but the surface area and the nature of the invariants put this well outside what should be auto-approved.

Other factors

• The author's status comment says the Drop rework was done "per review" and they are "waiting on re-review" — a human already has context on this change and asked for the current shape.
• Two comment-cop inline comments (copy_file.rs:1182, :1800) from the latest push are still unresolved; earlier rounds were addressed with either shortening or a "this is the item's contract, not a workaround" justification, so these likely fall in the same bucket, but they are outstanding.
• This PR intentionally shares test/internal/source-lints/self-receiver-teardown.test.ts with #37685 and #37693 so whichever lands last resolves the allowlist conflict; that coordination is a merge-order concern a human should be aware of.
• CI on the rebased head is reported green on every Windows and ASAN lane; the behavioural tests added ({ mode } copy + mkdirp on Windows, and unskipping bun-file-fd-read on Windows) exercise the converted paths.

[claude]

I reviewed this PR and didn't find any bugs. Because it's a substantial refactor of two unsafe Windows async I/O state machines (Drop ordering, event-loop refcount consolidation, cross-thread post via cloned LoopHandle, raw-pointer finish reclaiming the Box), a human look would still be worthwhile.

Checked: the Step-returning paths in both tasks all reach finish exactly once and none free self; the consolidated ref_keep_alive/Drop pair is balanced on every terminal path including synchronous failure in init; on_mkdirp_complete_concurrent now posts through a cloned handle in both CopyFileWindows and WriteFileWindows so nothing borrows the task across the post; ReadWriteLoop::drop only closes libuv-owned fds prepare_pathlike opened. The CodeRabbit note about uv_fs_open flags was addressed in 72707d3 (now libuv::O::RDONLY only).

Extended reasoning...

Overview

Restructures CopyFileWindows and ReadFileUV (Windows-only libuv blob tasks) so their &mut self state-machine steps return a #[must_use] Step instead of freeing the allocation they run on; a single unsafe fn finish(this: *mut Self, step) reclaims the leaked Box on Step::Done. Teardown moves to Drop impls (io_request cleanup, event-loop unref, ReadWriteLoop fd closes). The per-hop ref_keep_alive/unref_keep_alive pairs collapse to one pair taken in init/start_with_ctx and released in Drop. FileOpener loses its Windows-only hooks (loop_, req, open-callback stash) and get_fd/get_fd_by_opening become #[cfg(not(windows))]; ReadFileUV opens through uv_fs_open itself. Also fixes on_mkdirp_complete_concurrent in both CopyFileWindows and WriteFileWindows to clone the LoopHandle out before post_task, so no reference into the task is live across the post. Adds a source-lint test (shared with #37685/#37693), a { mode } copy test in bun-write.test.js, and enables bun-file-fd-read.test.ts on Windows.

Security risks

None. This is internal memory-lifetime plumbing for file I/O; no auth, crypto, permissions, or untrusted-input parsing is touched.

Level of scrutiny

High. The change is ~700 lines across four Rust files, all of it in the memory-safety-critical category the repo's review guide singles out: raw-pointer ownership, Drop ordering vs. promise settlement, event-loop refcount balance across every terminal path, cross-thread task posting where the JS thread may free the task before the pool thread's post_task returns, and libuv FFI back-pointers derived from &mut self. The reasoning about why each of these is sound is subtle (e.g., why re-leaking the Box at every callback would invalidate stored back-pointers under Stacked Borrows, why the Posted::Refused branch still intentionally leaks the task). The author has verified extensively on a Windows debug build with a per-path comparison script and the full test suite in both copy modes, but the surface area and the unsafe-Rust density warrant a maintainer's eyes.

Other factors

The PR has already been through several rounds: comment-cop trimming, a self-review pass that found and fixed a real reference-outlives-free in the mkdirp pool hop (421d0f4), and CodeRabbit's open-flags note addressed by 72707d3. It follows the same pattern as sibling conversions (#37672, #37685, #37693, #37551) and the source-lint ratchet is coordinated across them. CI on the previous head had all Windows and ASAN lanes green. No bugs surfaced in the multi-agent review. The do_close path in on_finish returning Step::Pending was traced — FileCloser::do_close on ReadFileUV always returns false (no io_request field), so that branch is effectively dead and Step::Done is always reached there; worth a maintainer confirming that reading of the trait's default.