fetch: post the FetchTasklet deinit hop through a copy of the loop handle

[robobun]

Problem

• When the HTTP thread drops the last reference to a fetch's tasklet, it hands the tasklet to the JS thread for destruction by posting a task. That post went through a &self reference into the tasklet, and the JS thread may free the tasklet as soon as the task is queued, before the post has returned.
• Freeing memory that a live reference argument points into is UB under both Rust aliasing models (Tree Borrows, which bun run rust:miri uses, rejects it as a deallocation of strongly protected memory), and codegen marks such arguments dereferenceable. Whether anything reads it afterwards does not matter.
• This is the path taken whenever the HTTP thread's deref is the 1 -> 0 transition, the interleaving behind the assert_no_refs report in fetch: hold FetchTasklet mutex through deref_from_thread so HTTP thread is never the final deref #32707.
• Latent today: nothing is read through the reference after the push, so there is no machine-level use after free and behaviour does not change. bundler: hand a finished Bun.build back through its pointer, not a &mut receiver #37723 fixes the same shape for JSBundleCompletionTask.

Fix

• Once the release reports the last ref, the loop handle is cloned out through the raw pointer and the hop is posted through the copy, so no reference to the tasklet exists after the count hits zero. The other release paths in this file and the tree's other cross-thread hand-overs already do it this way.
• Ordering, target loop and task tag are unchanged, so teardown of a queued hop is unaffected. Cost is one Arc increment, at most once per fetch.
• A source lint now checks every caller of ThreadSafeRefCount::release, from the release to the end of the function, for a reference formed to the released object or a post through one of its fields. On main it reports exactly the old line in this file; it passes here, and fails if the tree stops having a release caller.
• Verification: the source-lint suite and the fetch tests pass on a debug ASAN build (the remaining failures are pre-existing). With a local-only sleep that makes the HTTP thread's release the last one, several thousand tasklets went through this post while the JS thread freed them, all tests passed and ASAN reported nothing. The sleep is not in this PR.

Background

• A FetchTasklet is the per-fetch state shared by the JS thread that owns the request and the HTTP thread that runs it. It is refcounted, and its deinit has to run on the JS thread because it drops JSC handles.
• ThreadSafeRefCount::release decrements and returns true on the 1 -> 0 transition instead of destroying. It exists so the releasing thread can hand the object to the thread that is allowed to destroy it.
• The tasklet's loop handle addresses the owning VM's event loop. post_task pushes the task and wakes the loop, so the JS thread can run the task before the posting call returns to the caller.
• In Rust's aliasing models a reference passed as an argument is protected for the whole call: deallocating what it points to during that call is UB even if the callee never touches it again. Miri checks this; the compiler also optimises on it.
• test/internal/source-lints/ holds bun tests that regex-scan the tree's sources for banned patterns; each self-checks its patterns against sample banned and allowed spellings.

Original description

Problem

FetchTasklet::deref_from_thread (src/runtime/webcore/fetch/FetchTasklet.rs) is the HTTP thread's release of a tasklet ref. When release() reports that it dropped the last one, it has to hand the tasklet to the JS thread to be destroyed there, and it did so like this:

let self_ = Self::from_raw_ref(this);
let task = ConcurrentTask::create(Task::init(this.cast::<FetchTaskletDeinitHop>()));
let Posted::Queued = self_.post(task) else { .. };   // post(&self) -> self.loop_handle.post_task(task)

The consumer of that hop (FetchTaskletDeinitHop::run, dispatched by the JS thread) runs deinit and frees the tasklet. It can do so as soon as VmHandle::post has pushed the task and woken the loop, which is before post, LoopHandle::post_task and VmHandle::post have returned, and each of those frames holds a reference argument pointing into the allocation being freed: post's &self is the whole tasklet, post_task's &self is the loop_handle field inside it. A reference argument is protected for the duration of its call, and freeing the memory it points into is UB under both aliasing models whether or not anything reads it afterwards (Tree Borrows, which bun run rust:miri uses, rejects it as a deallocation of strongly protected memory; Stacked Borrows as "deallocating while item is strongly protected"); codegen relies on the same guarantee through dereferenceable. #37723 fixes the same cross-thread shape for JSBundleCompletionTask and carries a Miri reduction of it.

This is the path taken whenever the HTTP thread's deref is the last one: the final callback unlocks the tasklet mutex and then derefs, and if the JS thread runs the whole final progress update in between (the interleaving behind the assert_no_refs report in #32707), the HTTP thread's deref is the 1 -> 0 transition and this code runs. It is latent today: VmHandle::post touches nothing through the handle after the push, so there is no machine-level use after free, and this PR does not change behaviour. It fixes the contract, in the same way the function's neighbours already spell it: release_at_shutdown and callback in this file clone loop_handle out before their own deref so that embedded_work_finished does not go through the possibly freed tasklet, and S3HttpSimpleTask::http_callback, node_zlib_binding::async_job_run and post_job in VmHandle.rs all post their hand-over through a copied-out handle ("clone the handle out first").

Fix

After release() returns true, read loop_handle out through the raw pointer and post through the copy; no reference to the tasklet exists after the count hit zero. post(&self) stays for its two remaining callers (callback, on_write_request_data_drain), whose own ref keeps the tasklet alive across the post; its doc now says that this is its contract.

The ordering is unchanged (release, build the hop, post it) and the hop still goes to the same loop with the same tag, so the teardown handling of a queued hop (release_unrun) is unaffected. The clone costs one Arc increment on a path that runs at most once per fetch.

#37703 (releases through &mut receivers in this file) leaves this function unchanged, and #32707 reorders callback but also keeps it; neither conflicts with this change beyond adjacent lines.

Test

test/internal/source-lints/refcount-release-reborrow.test.ts: ThreadSafeRefCount::release exists only to route a destroy elsewhere (its doc says so), so every caller is by definition writing this hand-over. The lint checks each caller from the release to the end of the function: forming a reference to the released object (&*p, &mut *p, &(*p).field, p.as_ref(), the tree's from_raw_ref/from_raw_mut/callback_ctx helpers, ParentRef::from(..p..)), using a reference binding made from it earlier in the function, or posting through a field of it ((*p).loop_handle.post_task(..), (*p).post(..)) fails. It self-checks its patterns against the old and new spellings of this function and a set of other wrong and right spellings. Against main it reports exactly

src/runtime/webcore/fetch/FetchTasklet.rs:423: let self_ = Self::from_raw_ref(this);

and passes with this branch. It also fails if the tree stops containing a release caller, so it cannot pass vacuously; its header says to delete it in that case.

Its header lists the sites of the same hand-over that have no release() anchor and so stay out of its reach: S3HttpDownloadStreamingTask::http_callback (reported separately), and the napi async work and StatWatcher post_to_js_thread functions that #37723's lint already tracks. The Windows blob mkdirp completions have the same shape and are converted by #37705.

Verification

Debug (ASAN) build. bun test test/internal/source-lints/ (all 19 files) passes. test/js/web/fetch: abort-signal-leak (its three fixture functions run directly, about 7,600 fetches, two thirds aborted mid-flight; inside the runner two of them exceed the 5 s budget on this debug build, as they do without this change), fetch-stream-cancel-leak, fetch-abort-queued, fetch-abort-stream-body, fetch-abort-socket-close-race, exiting, body-stream, fetch-response-finalizer-sweep, fetch-leak and fetch.stream and fetch.test.ts. The failures in the last three and in fetch.test.ts are the ones the released binary also has in this container (localhost listeners vs the address family fetch picks, root ignoring file permissions) plus debug-only timeouts of gc-heavy tests, each of which passes when run alone or without the per-test budget (checked individually).

Because the hop is race-dependent, I also built this branch once with a local-only 2 ms sleep between callback's unlock and its deref (the same trick #37172 proposes as a debug flag) plus a log line in the hop, so the JS thread usually drops its ref first and the HTTP thread's release is the last one: in the multi-chunk stress script 649 of 1,000 tasklets, in the abort fixture 3,885 of 7,650, and across fetch-stream-cancel-leak, fetch-abort-stream-body, body-stream and fetch-abort-queued 6,923 of 11,209 went through the hop, i.e. through this post while the JS thread freed the tasklet underneath it; all tests passed and ASAN reported nothing. Neither the sleep nor the log is in this PR. cargo clippy -p bun_runtime and cargo fmt --check are clean.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 55 seconds

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: e223b1af-3185-4e54-8f68-a4949251cd53

📥 Commits

Reviewing files that changed from the base of the PR and between 9a543cc and a6e03e8.

📒 Files selected for processing (2)

• src/runtime/webcore/fetch/FetchTasklet.rs
• test/internal/source-lints/refcount-release-reborrow.test.ts

---

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

[robobun]

Status: fix and test are up (a6e03e8); ready for a maintainer. CI is red only on infrastructure, see below.

Reproduced structurally: on main, bun test test/internal/source-lints/refcount-release-reborrow.test.ts reports src/runtime/webcore/fetch/FetchTasklet.rs:423: let self_ = Self::from_raw_ref(this); (the &Self formed after release() hit zero, which the deinit hop was then posted through); it passes on this branch. There is nothing to observe at runtime today, as the description explains, so the lint is the test. The fetch suites listed in the description were run on the debug build, including a local-only build that widens the callback unlock/deref window so thousands of tasklets take this hop while the JS thread frees them underneath the post.

CI: builds 93382, 93392 and 93402 each lost build-bun lanes to Failed to download ... github.com/... for vendored tarballs (lolhtml, then also c-ares, mimalloc and the WebKit archive), and 93402's one red test, test/js/third_party/grpc-js/test-tonic.test.ts, is an ECONNRESET downloading protoc from github.com inside the test (reported separately). Every lane that got a binary passed (144 jobs in 93402). The diff is a 2-file change to FetchTasklet.rs and a new source lint, neither of which is involved in any of those; one re-run has already been used, so this needs a maintainer re-run or merge once the downloads recover.

[robobun]

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

⏳ @robobun, your commit a6e03e8 is still building in Build #93402, but has 2 failures so far (All Failures):

• /opt/winsysroot/Windows Kits/10/Include/10.0.26100/um/winver.h - clang warning on 🪟 aarch64 - build-bun
• scripts/build/ci.ts - build failed on 🐧 x64-android - build-bun
• scripts/build/ci.ts - build failed on 🍎 aarch64 - build-bun
• scripts/build/ci.ts - build failed on 🐧 x64-asan - build-bun

[claude]

I reviewed this PR and didn't find any bugs. The deref_from_thread rewrite is sound and mirrors the existing clone-out-then-post pattern elsewhere in the tree, but since this is cross-thread refcount teardown on the fetch path and ships a new tree-wide source lint, a human look is still worthwhile.

What was reviewed:

• Confirmed (*this).loop_handle.clone() runs before the hop is posted, so the auto-ref for .clone() dies before anything can free the tasklet; handle.post_task then borrows only the stack copy.
• Checked LoopHandle is #[derive(Clone)] (Arc-backed) and post_task routes to the same VmHandle::post path as before, so ordering/tag/teardown handling is unchanged.
• The lint follows the sibling source-lint conventions, self-checks its patterns, and guards against vacuous passes; verified its RELEASE regex matches the actual ThreadSafeRefCount::release signature in src/ptr/ref_count.rs.

Extended reasoning...

Overview

The PR touches two files: a ~15-line rewrite of FetchTasklet::deref_from_thread in src/runtime/webcore/fetch/FetchTasklet.rs plus a one-line doc addition on post(&self), and a new 242-line source lint at test/internal/source-lints/refcount-release-reborrow.test.ts. The Rust change stops forming &FetchTasklet after ThreadSafeRefCount::release reports the 1→0 transition and instead clones loop_handle out through the raw pointer before posting the deinit hop, so no protected reference into the tasklet is live while the JS thread may be freeing it. The lint enforces this invariant across every ThreadSafeRefCount::release caller in the tree.

Security risks

None identified. This is an internal aliasing-model correctness fix on an existing teardown path; no new inputs, parsing, or trust boundaries are introduced. The behaviour is stated (and appears) to be unchanged at the machine level today.

Level of scrutiny

High. deref_from_thread sits on the HTTP-thread → JS-thread hand-over for every fetch whose last ref drops off-thread, and the correctness argument rests on Tree Borrows / Stacked Borrows protector semantics. The change is small and matches the pattern already used by callback, release_at_shutdown, S3HttpSimpleTask::http_callback and post_job, but memory-safety edits to cross-thread refcounted teardown deserve a maintainer's eyes even when the diff is short. The new lint is also a tree-wide policy addition (regex-based, though self-checked against a banned/allowed corpus) that a human should sign off on.

Other factors

The comment-cop bot's paragraph-length warnings were addressed in a6e03e8 and the threads are resolved. CI shows build failures on several targets, but they are in scripts/build/ci.ts / a Windows SDK header warning and not in the changed files; still, I would not merge until those go green. The lint mirrors the structure of neighbouring lints (self-receiver-reclaim.test.ts, fn-long-mut-reborrow.test.ts), uses the shared globAllSources helper, filters to git-tracked files, and asserts releases > 0 so it cannot pass vacuously.