http: publish send_sync's result without holding a reference into the channel past the release

[robobun]

Problem

• AsyncHTTP::send_sync (src/http/AsyncHTTP.rs) heap-allocates a SingleHTTPChannel, blocks in read_item(), and frees the channel on the next line. read_item can return the moment the HTTP thread's publish releases the channel's lock.
• The publish was SingleHTTPChannel::write_item(&self): lock the Guarded slot, store the result, notify_one, and release by dropping the GuardedLock. When the release lands, the HTTP thread is still inside write_item(&self), GuardedLock::drop, and Mutex::unlock(&self), all holding references into the channel, while send_sync frees it.
• A reference argument is protected for the whole call (Background), so that free is rejected by both aliasing models, and rustc marks those arguments dereferenceable. The old comment in send_sync ("the callback has finished") was true of the memory accesses but not of the frames still holding references.
• Natively benign today: after the releasing store the only remaining work is those frames returning, plus, when the unlock was contended, a futex wake keyed by the now-dead address, which the kernel ignores. So there is no bun-level repro; the discriminator is miri on a model of the handoff (below). Same class as threading: finish a WaitGroup without holding a reference into it past the release #38330 (WaitGroup::finish), which lists this instance as deliberately left out.

Fix

• Guarded::with_lock_raw(this: *const Self, f) (src/threading/guarded.rs): locks through the pointer, runs f(&mut Value) under the lock, and releases with Mutex::unlock_raw, so the store that releases the lock is the last access to the Guarded. f is where anything that must happen before the reader may free the slot goes (here: the condvar signal), because once the release lands the slot may be gone.
• SingleHTTPChannel::write_item takes this: *const Self and publishes through with_lock_raw; send_sync_callback passes its raw this and makes that call its last statement; read_item(&self) is unchanged (the owner calls it and frees only after it returns). The comments in send_sync and send_sync_callback now state the real contract: the channel may be freed as soon as the publish's release lands.
• Mutex::unlock_raw, the per-backend unlock_raw impls and Futex::wake_raw (src/threading/Mutex.rs, Futex.rs) are needed so that no frame below with_lock_raw holds a reference across the release either. These two files are byte-identical to their state in threading: finish a WaitGroup without holding a reference into it past the release #38330, so the two PRs merge cleanly in either order; whichever lands second loses those hunks on rebase. The Guarded helper, the http change and the tests are the only things specific to this PR.
• Why this is correct: the runtime behaviour is unchanged (same lock, same store, same signal under the lock, same release; the reader still cannot take the value before the release), only which pointers are live across the release changes, and that is exactly what the aliasing models and dereferenceable reason about. It is the tree's existing convention for functions whose last act lets someone else free the object (this: *mut Self receivers, self-receiver-reclaim.test.ts; WaitGroup::finish_raw in threading: finish a WaitGroup without holding a reference into it past the release #38330).
• Verified with:
  • src/threading/guarded.rs gets a crate test modelling the handoff (Guarded<Option<u32>> + Condition + a plain field, like the channel's response_buffer; writer publishes through with_lock_raw, reader takes the value and frees the box, then joins). Passes under Tree Borrows miri on seeds 0 to 5 and under the default Stacked Borrows (300 iterations each, about 9s), and natively (10,000 iterations). The same model with the &self + guard writer is rejected at the reader's Box drop after 8, 7, 17 and 35 iterations on seeds 0 to 3 (diagnostics below).
  • test/internal/threading-guarded-miri.test.ts (new): runs cargo miri test -p bun_threading -- guarded:: under Tree Borrows and requires the model to have run. Scoped to guarded:: because the crate's wait_group test is still rejected on main until threading: finish a WaitGroup without holding a reference into it past the release #38330 lands; it can become a crate-wide run after that. Skips where miri or the cargo workspace is unavailable, like linear-fifo.test.ts.
  • test/internal/source-lints/handoff-publish-raw.test.ts (new): pins that with_lock_raw and write_item take the object as this: *const _, release through the raw path and keep no reference binding, and that send_sync_callback ends at the publish; on main it reports write_item taking &self. Runs in the source-lints workflow on every PR touching src/**/*.rs. Whole test/internal/source-lints/ still passes (111 tests).
  • bun bd test test/cli/install/bun-publish.test.ts --timeout 60000 (39 pass; every publish is a send_sync round trip, against Verdaccio and the mock registries, under the ASAN debug build) and bun-audit.test.ts (17 pass). Without --timeout the three lifecycle-script publish tests exceed bun test's local 5s default on the debug build (6.5s: each of the six scripts starts a debug bun) and take the rest of the file down with them; that is independent of this change, and CI's runner passes a larger per-test timeout.
  • cargo check -p bun_threading -p bun_http --tests on x86_64-pc-windows-msvc, aarch64-apple-darwin, x86_64-unknown-linux-musl and x86_64-unknown-freebsd, cargo check --release on the host (the ReleaseImpl-direct path), cargo clippy --no-deps on both crates, cargo test -p bun_threading, cargo fmt --check.
• Not in this PR, tracked separately: the same shape with bun_threading::Channel in prefetch_remote_images (src/runtime/cli/run_command.rs), and send_sync_callback writing err/elapsed back into the caller's AsyncHTTP while send_sync(&mut self) is on the stack. The s3 download_stream instance from threading: finish a WaitGroup without holding a reference into it past the release #38330's list uses a bare Mutex and needs unlock_raw made pub, which is a one-word change on top of either PR.

Background

• Protectors: under Stacked Borrows and Tree Borrows (the model bun run rust:miri uses), a reference passed as a function argument, &self included, is protected until that function returns: the callee may assume the memory stays valid, and freeing memory a protected reference covers is undefined behaviour even if the callee never touches it again. This is what lets rustc emit dereferenceable for reference arguments. A raw pointer argument makes no such claim, which is why a function whose release is what lets another thread free the object takes *const Self.
• Interior mutability does not help here: the protector also covers the plain fields of the struct (response_buffer in the channel) and the mutex's padding bytes, and it is the deallocation that is rejected, not a read or write.
• Guarded<T> is bun_threading's mutex-plus-value; lock() returns a GuardedLock whose Drop unlocks through a &Mutex. Condition::wait_guarded unlocks and re-locks that same mutex around the wait, which is why read_item cannot observe the value before the writer's release, and why the release is the exact point after which the channel may be freed.
• Futex wakes are address-keyed: the kernel looks the address up in its waiter table without touching the memory, so the wake a contended unlock_raw issues after the releasing store is harmless on a freed word; miri models it as EFAULT, which the Linux backend now ignores under cfg(miri) only (from threading: finish a WaitGroup without holding a reference into it past the release #38330's hunk).

miri diagnostics for the by-reference writer in the same model

The model test with the writer as fn write(&self, v) { let mut g = self.slot.lock(); *g = Some(v); self.cv.notify_one(); }, MIRIFLAGS=-Zmiri-tree-borrows cargo miri test -p bun_threading, fails at iteration 8, 7, 17 and 35 for -Zmiri-seed=0..3. Depending on the interleaving the protected tag it names is the &self of the writer or the &Mutex of the unlock; one run, condensed:

error: Undefined Behavior: deallocation through <220238> at alloc74771[0xc] is forbidden
    = help: the accessed tag <220238> is foreign to the protected tag <220005> (i.e., it is not a child)
    = help: this deallocation (acting as a foreign write access) would cause the protected tag <220005> (currently Frozen) to become Disabled
    = help: protected tags must never be Disabled
help: the accessed tag <220238> was created here
   --> src/threading/guarded.rs   drop(unsafe { Box::from_raw(ch) });
help: the protected tag <220005> was created here, in the initial state Cell
   --> src/threading/guarded.rs   fn unlock(&self) {        (impl RawMutex for Mutex, i.e. GuardedLock::drop)

The fixed test does not reach the contended wake-after-free tail in 300 iterations (checked by instrumenting the cfg(miri) EFAULT arm); that tail is exercised by #38330's wait_group test.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 38 minutes

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

How can I continue?

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

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ea0ed542-7223-4336-8c73-f34e3cb96405

📥 Commits

Reviewing files that changed from the base of the PR and between 1cf8af0 and b00fd0e.

📒 Files selected for processing (6)

• src/http/AsyncHTTP.rs
• src/threading/Futex.rs
• src/threading/Mutex.rs
• src/threading/guarded.rs
• test/internal/source-lints/handoff-publish-raw.test.ts
• test/internal/threading-guarded-miri.test.ts

---

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

[robobun]

Status: fix pushed, waiting on CI and review.

• Reproduced how: no runtime repro exists (the shape is aliasing-model UB, natively benign), so the evidence is miri. The same model as the new guarded::tests test, with the writer spelled as the old write_item(&self) + guard, is rejected by MIRIFLAGS=-Zmiri-tree-borrows cargo miri test -p bun_threading at the reader's Box drop within 8 to 35 iterations on seeds 0 to 3; the raw-pointer publish passes 300 iterations on seeds 0 to 5 and under Stacked Borrows. handoff-publish-raw.test.ts reports write_item taking &self on main.
• Overlap: src/threading/Mutex.rs and Futex.rs are byte-identical to threading: finish a WaitGroup without holding a reference into it past the release #38330, so the PRs merge in either order; whichever lands second drops those hunks on rebase.

[claude]

I reviewed this PR and the automated bug-hunting pass found no issues. Because it rewrites the unlock path of the core Mutex/Futex/Guarded primitives across every platform backend and adds new unsafe raw-pointer API (with_lock_raw, unlock_raw, wake_raw) whose correctness rests on aliasing-model reasoning, a human look is still warranted.

Checked: the condvar signal in write_item sits inside the with_lock_raw closure so it runs before the release; with_lock_raw's own (*this).mutex.lock() and the &mut Value passed to f both drop their protectors before unlock_raw; the FreeBSD/WASM .cast_mut() adaptations preserve the original *mut c_void argument type; Bun__unlock and Mutex::unlock route through unlock_raw so no by-reference unlock remains on any backend.

Extended reasoning...

Overview

This PR fixes an aliasing-model violation in AsyncHTTP::send_sync's one-shot result handoff. The HTTP thread's write_item(&self) held a protected &self (and, via GuardedLock::drop → Mutex::unlock(&self), a protected &Mutex) past the store that releases the lock, while send_sync frees the channel as soon as read_item returns — which it can do the instant that release lands. Under Stacked/Tree Borrows the free is UB (protected tags may not be deallocated), and rustc marks those references dereferenceable. The fix threads raw pointers through the whole release path: new Guarded::with_lock_raw(this: *const Self, f), Mutex::unlock_raw(this: *const Self), per-backend unlock_raw (SRWLOCK, os_unfair_lock, futex), and Futex::wake_raw(*const AtomicU32) for the contended-unlock tail. write_item now takes this: *const Self and publishes through with_lock_raw; send_sync_callback makes that its last statement. Two new tests: a miri model in guarded.rs (driven by test/internal/threading-guarded-miri.test.ts) and a source-lint pinning the raw-receiver shape.

Security risks

None in the direct sense — no untrusted input, auth, or crypto. The risk surface is memory safety: every backend's unlock_raw is now unsafe with a contract that the pointer is live until the releasing store, and wake_raw deliberately allows the address to be dead. A mistake here would affect every mutex user in the runtime. The changes look correct (the release is genuinely the last access on every backend; futex wakes are address-keyed on all four kernels and WASM linear memory), but this is exactly the category REVIEW.md flags as most-blocked.

Level of scrutiny

High. This is core bun_threading — the mutex/futex primitives every subsystem depends on — with platform-specific unsafe on five backends, a new pub unsafe fn on Guarded, and an FFI signature change (os_unfair_lock_unlock from safe fn(&UnsafeCell<_>) to fn(*mut _)). The reasoning is subtle (protectors, dereferenceable, wake-after-free being kernel-benign) and the PR description states it is natively benign today with miri as the only discriminator, so a regression would be silent. The Mutex/Futex hunks are stated to be byte-identical to #38330, which adds a merge-order/coordination dimension a human should confirm.

Other factors

The PR is thoroughly self-documented and tested (miri model on multiple seeds, source-lint with self-tests, cross-target cargo check, bun-publish/bun-audit under ASAN). I traced each concern I'd raise: the closure in write_item derefs (*this).cv while the lock is still held so that reference ends before the release; with_lock_raw's (*this).mutex.lock() autoref and the &mut Value handed to f both end before unlock_raw; the FreeBSD/WASM ptr.cast().cast_mut() correctly reproduces the *mut c_void/*mut i32 the FFI expects now that ptr is *const AtomicU32; the cfg(miri) EFAULT arm is Linux-only and does not weaken native builds. Nothing looked wrong, but the blast radius and the cross-cutting unsafe API addition put this outside what I'd approve without a human reviewer.

[robobun]

^{Updated 10:05 PM PT - Aug 13th, 2026}

@robobun, your commit b00fd0e is building: #95384

[robobun]

^{Updated 1:35 AM PT - Aug 14th, 2026}

✅ @robobun, your commit b00fd0eaebf50ddfb337bbeb1f9ff734c8adcdac passed in Build #95384! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 38345

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

bun-38345 --bun