threading: publish into a Channel without holding a reference into it past the release

[robobun]

Problem

• RunCommand::prefetch_remote_images (src/runtime/cli/run_command.rs) keeps its done-channel (DoneChannel, a bun_threading::Channel) as a local, blocks in done_channel.read_item() once per download, and returns as soon as the last tick arrives, which ends the channel's storage and drops the RemoteImageDownload boxes.
• Each tick was published from the HTTP thread by RemoteImageDownload::on_done as (*this.done).write_item(0). read_item can return the moment that publish's mutex release lands, and at that moment the HTTP thread is still inside write_item(&self), write_all(&self), write_items(&self) and the MutexGuard drop's Mutex::unlock(&self), all of which hold references into the channel.
• A reference argument is protected for the whole call (Background), so freeing the channel while those frames are live is undefined behaviour under both aliasing models, and rustc marks those arguments dereferenceable. Natively nothing observable happens today: after the releasing store the only work left is those frames returning, plus an address-keyed futex wake when the unlock was contended. So there is no bun-level repro; the discriminator is miri on a model of the handoff.
• The model this PR adds to src/threading/channel.rs (a reader that frees each channel as soon as read_item has handed it the items), run against main's Channel with the writer using write_item(&self) (the only publish main has), is rejected at the reader's Box drop: Tree Borrows Undefined Behavior: deallocation through <tag> at alloc[0xc] is forbidden, pointing at the end of write_all(&self); Stacked Borrows not granting access to tag <..> because that would remove [SharedReadOnly for <..>] which is strongly protected, naming write_all's &self. Every seed tried fails within the first half of a run (16 Tree Borrows seeds, 4 Stacked Borrows seeds; table below).
• Same class as threading: finish a WaitGroup without holding a reference into it past the release #38330 (WaitGroup::finish) and http: publish send_sync's result without holding a reference into the channel past the release #38345 (SingleHTTPChannel::write_item in send_sync), which lists this Channel instance as tracked separately. bun_threading::Channel has no other in-tree user.

Fix

• Channel::write_item_raw(this: *const Self, item): locks through the pointer, runs the critical section (write_item_locked(&self): closed check, push, getters.signal(), or wait on putters while the buffer is full), then releases with Mutex::unlock_raw. The &Self formed for the critical section ends before the unlock; the releasing store inside unlock_raw is the thread's last access to the channel. write_item(&self) delegates to it and its doc says when &self is still fine (a channel that something other than the matching read keeps alive). write_all and write_items (a slice path whose only caller passed one item) are folded into the single-item function; the read side is unchanged.
• on_done publishes with DoneChannel::write_item_raw(this.done, 0) as its last statement. After the terminal callback the HTTP thread only touches its own threadlocal allocation and statics (on_async_http_callback_raw), and the result dropped after the publish owns everything it frees, so the publish's release really is the last access to anything the main thread frees.
• src/threading/Mutex.rs and Futex.rs (Mutex::unlock_raw, the per-backend impls, Futex::wake_raw) are byte-identical to threading: finish a WaitGroup without holding a reference into it past the release #38330 and http: publish send_sync's result without holding a reference into the channel past the release #38345, so those merge with this in any order and whichever lands later just loses the hunks on rebase. s3: reach the streaming download task through its raw pointer on both threads #38351 (the s3 instance, stacked on threading: finish a WaitGroup without holding a reference into it past the release #38330) additionally makes unlock_raw pub; against it this PR differs in that one block of Mutex.rs, resolved by keeping pub (this PR only needs pub(crate)). (unlock_raw's doc names WaitGroup::finish_raw, which threading: finish a WaitGroup without holding a reference into it past the release #38330 adds.) The hunks are needed here for the same reason as there: without them the frame below write_item_raw would hold a &Mutex across the release.
• Why this is correct: the runtime behaviour is unchanged (same lock, same push, same signal under the lock, same release; the reader still cannot take the item before the release, because it re-acquires the mutex inside read_item), 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 another thread free the object (this: *mut Self receivers, test/internal/source-lints/self-receiver-reclaim.test.ts; WaitGroup::finish_raw in threading: finish a WaitGroup without holding a reference into it past the release #38330; UnboundedQueue::push_raw in event_loop: post to a MiniEventLoop through raw pointers and publish last #37883).
• Verified with:
  • channel::tests::reader_may_free_a_channel_once_it_has_the_items (src/threading/channel.rs): per batch one writer thread publishes two items into each of 32 one-slot channels and the reader frees each channel right after its second read_item; the one-slot buffer makes the writer wait for the reader on every channel, so every channel is a real chance for the free to race the publishing write's return (with one item per channel the writer runs ahead and the model only probes the first channel of each thread). 32 batches under miri (about 18s): passes on Tree Borrows seeds 0 to 11 and Stacked Borrows seeds 0 to 3; natively 1000 batches in 0.3s. items_arrive_in_order_through_a_full_buffer covers write_item(&self) and the full-buffer wait path the two functions share.
  • test/internal/threading-channel-miri.test.ts (new): runs cargo miri test -p bun_threading -- channel:: under Tree Borrows, as rust:miri does, and requires the model to have run. Fails on main's src/ (no model there; the model itself cannot be written against main's API other than in the &self shape shown above), passes with this change (23s under bun bd test here). Scoped to channel:: 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; same structure as http: publish send_sync's result without holding a reference into the channel past the release #38345's guarded test, and both can fold into a crate-wide run after that.
  • test/cli/run/markdown-entrypoint.test.ts: new test rendering a document with 300 remote images (the channel has 256 slots) under a kitty PTY against a local server; all 300 are requested and staged and the process exits 0 (0.65s under the ASAN debug build). Whole file: 31 pass.
  • cargo check -p bun_threading --tests on x86_64-pc-windows-msvc, aarch64-apple-darwin, x86_64-unknown-freebsd and aarch64-unknown-linux-musl, --release (the ReleaseImpl-direct path) on linux, windows and darwin, cargo check -p bun_runtime, cargo clippy -p bun_threading --no-deps (with --tests the only finding is main's pre-existing std::thread::spawn in the wait_group test, which threading: finish a WaitGroup without holding a reference into it past the release #38330 replaces), cargo fmt --check, bun bd test test/internal/source-lints/ (98 pass).
  • Instrumenting the cfg(miri) EFAULT arm: the fixed model did not reach the contended wake-after-free tail in 6 runs (6144 channels); threading: finish a WaitGroup without holding a reference into it past the release #38330's test is the one that exercises that tail.

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 claims nothing, which is why a function whose release is what lets another thread free the object takes *const Self.
• Interior mutability does not exempt the struct: a &Channel also covers the padding between its fields (the 0xc in the diagnostic is inside the mutex), and it is the deallocation that is rejected, not a read or write.
• bun_threading::Channel is a mutex, two condition variables (getters, putters) and a LinearFifo. read_item waits on getters, and a condition-variable wait re-acquires the mutex before returning, so a blocked read_item cannot return before the publisher's unlock; the store inside that unlock is the exact point after which the channel may be gone, and everything the publisher needs the channel for (the push and the signal) happens before it.
• 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 ignores under cfg(miri) only (part of the shared hunk).

miri diagnostics against main's Channel, and the seed tables

Model test appended to main's src/threading/channel.rs with the writer calling (*channel).write_item(..), cargo miri test -p bun_threading -- channel::.

Tree Borrows (MIRIFLAGS=-Zmiri-tree-borrows), default seed:

error: Undefined Behavior: deallocation through <216753> at alloc47340[0xc] is forbidden
    = help: the accessed tag <216753> has state Reserved (conflicted) which forbids this deallocation (acting as a child write access)
help: the accessed tag <216753> was created here, in the initial state Reserved
   --> src/threading/channel.rs     drop(Box::from_raw(channel.cast_mut()));
help: the accessed tag <216753> later transitioned to Reserved (conflicted) due to a protector release (acting as a foreign read access) on every location previously accessed by this tag
   --> src/threading/channel.rs:84:6      (closing brace of write_all(&self))

Other interleavings report the same free as
the accessed tag is foreign to the protected tag <..> (currently Frozen) ... protected tags must never be Disabled, with the protected tag being write_all's &self (channel.rs:80).

Stacked Borrows (MIRIFLAGS=""):

error: Undefined Behavior: not granting access to tag <139416> because that would remove [SharedReadOnly for <223648>] which is strongly protected
help: <223648> is this argument
   --> src/threading/channel.rs:80:29
    |  pub(crate) fn write_all(&self, items: &[T]) -> Result<(), ChannelError> {
    |                          ^^^^^

Batch (of 32, 32 channels each) in which the &self shape is rejected, by -Zmiri-seed:

• Tree Borrows, seeds 0..15: 2, 0, 4, 2, 0, 0, 2, 5, 12, 5, 2, 16, 0, 2, 7, 0
• Stacked Borrows, seeds 0..3: 2, 0, 4, 2

The fixed shape completes all 32 batches with no report on Tree Borrows seeds 0..11 and Stacked Borrows seeds 0..3 (about 17.7s of interpretation each).

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 40 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: 0d259ab2-9c3c-4086-bd92-94cd3aa95507

📥 Commits

Reviewing files that changed from the base of the PR and between 3753c8b and 0d632d9.

📒 Files selected for processing (6)

• src/runtime/cli/run_command.rs
• src/threading/Futex.rs
• src/threading/Mutex.rs
• src/threading/channel.rs
• test/cli/run/markdown-entrypoint.test.ts
• test/internal/threading-channel-miri.test.ts

---

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

[robobun]

Status: ready for review.

Reproduced with the model test this PR adds to src/threading/channel.rs, run against main's Channel (writer publishing through write_item(&self)): cargo miri test -p bun_threading -- channel:: rejects the reader's free under Tree Borrows (deallocation through <tag> at alloc[0xc] is forbidden, protected tag is write_all's &self) and under Stacked Borrows (... which is strongly protected), on every seed tried. With this change the same model passes on 12 Tree Borrows seeds and 4 Stacked Borrows seeds, and bun bd test test/internal/threading-channel-miri.test.ts and test/cli/run/markdown-entrypoint.test.ts pass against the debug build.

Overlap: src/threading/Mutex.rs and Futex.rs are byte-identical to #38330 and #38345 (whichever lands later drops those hunks on rebase); #38351, stacked on #38330, additionally makes unlock_raw pub, so against it this PR differs in that one block of Mutex.rs only.

[claude]

I reviewed this PR and didn't find any bugs. Because it rewires the unlock path of Mutex and Futex::wake across every platform backend and threads raw-pointer receivers through the core Channel publish, a human look is still worthwhile.

Checked: write_all/write_items have no remaining crate callers; LinearFifo::write_item exists and matches the new error branching; on_done's last access to *this (reading this.done) precedes the publish and the local &mut carries no protector; write_item_locked's &self and its putters.wait(&self.mutex) reference both end before unlock_raw while the item is unpublished.

Extended reasoning...

Overview

This PR fixes a Stacked/Tree Borrows protector violation in bun_threading::Channel: RemoteImageDownload::on_done publishes into a DoneChannel that lives on the reading thread's stack, and the reader may drop that stack frame as soon as read_item returns — while the writer's write_item(&self) (and the write_all/write_items/MutexGuard::drop frames beneath it) still hold protected &self references into the channel. The fix adds Channel::write_item_raw(this: *const Self), Mutex::unlock_raw, and Futex::wake_raw, so no reference is protected across the releasing store; the critical section (write_item_locked(&self)) forms a &Self that dies before unlock_raw. The dead write_all/write_items slice path is removed. on_done now calls DoneChannel::write_item_raw(this.done, 0) as its last statement. Tests: a miri model test in channel.rs, a bun-level miri driver in test/internal/, and a 300-image markdown-entrypoint test that exercises the full-buffer wait path.

Security risks

None identified. This is an aliasing-model correctness fix in threading primitives; no auth, crypto, or untrusted-input parsing is touched. The runtime behavior (lock → push → signal → unlock) is unchanged; only which pointer tags are live across the release changes.

Level of scrutiny

High. Mutex::unlock() now routes through unlock_raw on every platform (SRWLOCK, os_unfair_lock, futex, FreeBSD umtx, wasm), and os_unfair_lock_unlock's extern signature changed from safe fn(&UnsafeCell<..>) to unsafe fn(*mut ..). These are the most-used sync primitives in the runtime, and per-platform cfg-gated code is only type-checked on its own target. The PR description says these Mutex/Futex hunks are byte-identical to #38330 and #38345, so cross-checking those is worthwhile. The reasoning is very carefully documented and matches the tree's established *mut Self receiver convention (self-receiver-reclaim source lint, UnboundedQueue::push_raw), and the miri seed tables show the model failing on main and passing here.

Other factors

• Verified write_all/write_items were pub(crate) with no other in-crate callers, so their removal is safe. LinearFifo::write_item exists (linear_fifo.rs:492) and returns Result<(), AllocError>, matching the B::DYNAMIC branch.
• Checked on_done: the local let this = &mut *this is not a function argument, so it carries no protector; its last use is the field read this.done evaluated before write_item_raw runs. The result parameter dropped after the unsafe block owns its own state and doesn't reference the channel or the box.
• Checked write_item_locked: putters.wait(&self.mutex) releases the mutex mid-call, but the item is unpublished at that point so the channel is contractually live; the &self protector on write_item_locked ends before unlock_raw runs.
• The unlock_raw doc comment references WaitGroup::finish_raw, which per the description is added in #38330 (not yet landed) — a forward reference the author calls out explicitly.
• Test coverage is thorough: miri model with seed sweep, native 32k-channel stress, plus an integration test that overflows the 256-slot static buffer. The miri driver test correctly guards on toolchain availability and asserts the model actually ran.

[github-actions]

This PR may be a duplicate of:

1. http: publish send_sync's result without holding a reference into the channel past the release #38345 - Same miri protector-violation fix on the same shape (a channel publish whose reader frees the channel at the lock release), and ships byte-identical Mutex::unlock_raw / Futex::wake_raw hunks.
2. threading: finish a WaitGroup without holding a reference into it past the release #38330 - Same release-then-free fix applied to WaitGroup::finish, with the same byte-identical src/threading/Mutex.rs and src/threading/Futex.rs changes.
3. s3: reach the streaming download task through its raw pointer on both threads #38351 - Same bug class (final unlock lets another thread free the object) and edits the very Mutex::unlock_raw this PR adds, so the two conflict directly in src/threading/Mutex.rs.

🤖 Generated with Claude Code

[robobun]

Not a duplicate of any of the three; they are sibling instances of one bug class, each fixing a different primitive or caller, and none of them touches bun_threading::Channel or prefetch_remote_images:

• threading: finish a WaitGroup without holding a reference into it past the release #38330 fixes WaitGroup::finish (callers in LinkerContext). It introduces Mutex::unlock_raw / Futex::wake_raw; this PR carries those two files byte-identical, so either order lands cleanly and whichever is second drops the hunks on rebase.
• http: publish send_sync's result without holding a reference into the channel past the release #38345 fixes SingleHTTPChannel::write_item in send_sync via Guarded::with_lock_raw, and lists this Channel instance as tracked separately. Same byte-identical Mutex.rs / Futex.rs.
• s3: reach the streaming download task through its raw pointer on both threads #38351 fixes the s3 download_stream task. It is stacked on threading: finish a WaitGroup without holding a reference into it past the release #38330's commit and additionally makes unlock_raw pub (it needs it outside the crate), so relative to this PR it differs in that one block of Mutex.rs (Futex.rs is identical). Whichever of the two lands second resolves that block by keeping pub; this PR only needs pub(crate).

What is specific to this PR: Channel::write_item_raw and the crate test in src/threading/channel.rs, the on_done change in src/runtime/cli/run_command.rs, test/internal/threading-channel-miri.test.ts, and the 300-image case in test/cli/run/markdown-entrypoint.test.ts.

[robobun]

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

❌ @robobun, your commit 0d632d9 has some failures in Build #95443 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 38358

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

bun-38358 --bun