bundler: stop the plugin hops from reaching back into the pass that posted them

[robobun]

Problem

• Nothing misbehaves today and there is no user-visible bug. This is an aliasing fix in the same family as event_loop: post to a MiniEventLoop from other threads through &self #37691 and io: make KEventWaker::wake take &self like the Linux and Windows wakers #37626.
• A Bun.build pass posts three kinds of task to the plugins' JS thread (one per onResolve request, one per onLoad request, one to settle .defer() promises). Each of them formed a fresh &mut BundleV2 back into the pass just to fetch the plugin handle.
• While those tasks run, the bundle thread is still writing to that same pass through its own &mut BundleV2, and parse workers hold &BundleV2. A second live &mut to a struct another thread is mutating is a false claim under Stacked/Tree Borrows and to LLVM's noalias, even though these bodies only read one pointer.
• The .defer() task also read the build's completion result from the plugin thread to decide whether to reject the promises. That result is still pending whenever the task runs, so the flag was always false, and the read touched state the bundle thread holds &mut to.

Fix

• Each of the three tasks copies the plugin pointer into itself when it is built and, on the JS thread, reads only its own fields; the pass's plugins_mut accessor is deleted. This is sound because plugins is set before the pass starts and never written again, so a copy taken at construction cannot go stale.
• The .defer() task used to be embedded inside the pass and walked back to it with from_field_ptr!. It is now allocated per drain from the pass's arena, the same shape as the resolve and load tasks.
• The always-false rejected flag is removed end to end; the drain always resolves the promises, which is what happened before as well.
• Verification: there is nothing to observe at runtime, so the test is a source lint over the three task bodies; it reports nine hits on main and passes here. The existing bundler plugin, defer and bake plugin tests pass on a debug ASAN build. A few unrelated worker-terminate cases timed out on a loaded box and are left to CI.

Background

• BundleV2 is one bundle pass. For Bun.build it lives on a dedicated bundle thread, which sits in wait_for_parse holding &mut BundleV2 and updating the module graph through it until every file is parsed.
• Plugin callbacks are JS, so they run on the JS thread that owns the plugins. The pass posts a small task (a "hop") to that thread per request, and the answer is posted back to the loop that owns the pass. Only the JS-thread half of each hop changes here; the answer path keeps its bv2 backref because it runs on the pass's own loop.
• args.defer() inside an onLoad plugin hands out a promise the pass settles later in one batch; DeferredBatchTask is the hop that performs that batch.
• The plugin handle is an opaque object owned by the JS side, not stored inside the pass. Plugin::opaque_mut turns the raw pointer into a reference to it directly, so a hop does not need the pass to reach it.
• A Rust &mut asserts exclusive access for as long as it is live. Rust's aliasing models (Stacked/Tree Borrows) and LLVM's noalias optimisations rely on that assertion, so a second &mut is wrong even when no bytes actually race.

Original description

What does this PR do?

Resolve::run_on_js_thread and Load::run_on_js_thread (src/bundler/bundle_v2.rs) and DeferredBatchTask::run_on_js_thread are the three tasks a bundle pass posts to the plugins' JS thread through enqueue_on_js_loop_for_plugins; the runtime's run_task dispatches them there. For Bun.build the BundleV2 they point back at lives on and is driven by the bundle thread, which while they run is inside wait_for_parse -> tick_raw, reborrowing the whole struct as &mut BundleV2 on every is_done and writing graph.* through it (parse workers hold &BundleV2 through Worker.ctx at the same time). All three hops nevertheless formed their own &mut BundleV2 on the plugin thread:

unsafe { &mut *self.bv2 }.plugins_mut().expect("plugins").match_on_resolve(..)   // Resolve, Load
let bv2 = self.get_bundle_v2();   /* &mut *from_field_ptr!(BundleV2, drain_defer_task, ..) */
bv2.plugins_mut().expect("plugins").drain_deferred(rejected);                   // DeferredBatchTask

That is a second live &mut to a struct another thread is mutating through its own, created only to satisfy plugins_mut(&mut self), whose SAFETY comment ("&mut self ensures no other projection overlaps") is also not true across threads: the bundle thread and the parse workers call plugins_ref() during the same window. Nothing misbehaves today (the bodies read one pointer-sized field, and the plugin handle is an opaque_ffi! ZST), but under Stacked/Tree Borrows and for LLVM's noalias the &mut is a false claim, the same family as the cross-thread posting fixes in #37691 and #37626.

The only thing the hops need from the pass is plugins, which is set before the pass starts (init_and_run / BundleV2::init) and never written again. Resolve and Load already copy everything else their plugin-thread body needs when they are built (MiniImportRecord, path, namespace, default_loader), so plugins now goes the same way:

• Resolve and Load get a plugins: Option<NonNull<Plugin>> field, copied from the pass in init; DeferredBatchTask gets the same field, copied in schedule. The three run_on_js_thread bodies read their own field and call the FFI through the safe Plugin::opaque_mut; they contain no unsafe and do not name the pass. BundleV2::plugins_mut has no callers left and is removed.
• DeferredBatchTask used to be embedded in the pass (BundleV2::drain_defer_task), which is why it walked back to it with from_field_ptr!, and also why the &mut DeferredBatchTask the dispatch arm forms on the plugin thread would still have pointed into the pass once the struct carried a real field. It is now allocated from the pass's arena per drain, like Resolve / Load (schedule takes the pass and the arena from its only caller, Graph::drain_deferred_tasks, which holds &mut Graph at that point and so passes self.heap rather than having schedule read it back through the pass), and the field is gone from BundleV2, along with get_bundle_v2(), init() and the debug running flag (which was never set to true). One pointer-sized allocation per drain round; release_unrun stays a no-op for the same reason as Resolve's.
• The deferred batch also read completion.result_is_err() to decide whether to reject the .defer() promises. result is Pending until run_from_js_in_new_thread has returned, and the batch only runs while the pass is still waiting on the deferred loads, so it was always false (bake passes have no completion at all); it was also a read, from the plugin thread, of the completion task the bundle thread holds &mut to for the whole pass. It is removed end to end: CompletionDispatch::result_is_err, CompletionHandle::result_is_err, and the rejected parameter of JSBundlerPlugin__drainDeferred, which now always resolves. No observable change: the promises were always resolved before as well.

Not changed: the bv2 backref itself stays on Resolve / Load for dispatch() and for delivering the answer, which run on the loop that owns the pass (on_load_from_js_loop, on_notify_defer), as do the same-thread &mut *x.bv2 sites there. The runtime-side answer thunks in src/runtime/api/JSBundler.rs (bv2_mut in onResolveAsync / onLoadAsync / addError / on_defer) are the other half of this and are handled by #37691; the two apply independently, and with this PR bv2_plugin there could read the copied field later. DeferredBatchTask is kept as the posted task rather than posting the pass pointer itself: something tagged has to be posted, and a separately allocated task that carries what its body needs is the same shape as Resolve / Load.

How did you verify your code works?

There is nothing to observe at runtime, so the test is a source lint, test/internal/source-lints/bundler-plugin-hops-no-pass.test.ts: it extracts the bodies of the three run_on_js_thread fns in src/bundler/ and fails on any route back to the pass (bv2, BundleV2, an accessor named after the pass, the parse task's ctx, from_field_ptr! / container_of, plugins_ref() / plugins_mut()), with a guard that the three hops are still where it looks. On main it reports all three hops (nine hits); it passes here.

• cargo clippy -p bun_bundler -p bun_runtime --no-deps clean; rustfmt --check and clang-format clean.
• Debug (ASAN) build of the final revision: test/bundler/bundler_plugin.test.ts (53 pass), bundler_plugin_chain.test.ts (13 pass), bundler_defer.test.ts (10 pass, covers the drain) exercise all three hops on the Bun.build bundle thread; test/bake/dev/plugins.test.ts (3 pass) covers the bake arm, where the hops run on the loop that owns the pass. In test/js/web/workers/worker-terminate-funnels.test.ts the pool case (a build cancelled while plugin requests are outstanding) passes; the whole file passed on the earlier revision, and on the final one the unrelated timers/messaging/net/http/fs cases hit their 30s timeouts on a heavily loaded box, so those are left to CI.

While testing this I noticed that an onLoad plugin calling args.defer() without awaiting it underflows the scan counter (debug panic in on_parse_task_complete, hang on the released build); that is pre-existing and unrelated to these hops, and has been reported separately.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: a134517b-313a-4b8a-9e27-2a83b8f87860

📥 Commits

Reviewing files that changed from the base of the PR and between 3c87727 and 9f569d0.

📒 Files selected for processing (7)

• src/bundler/BundleThread.rs
• src/bundler/DeferredBatchTask.rs
• src/bundler/Graph.rs
• src/bundler/bundle_v2.rs
• src/jsc/bindings/JSBundlerPlugin.cpp
• src/runtime/api/js_bundle_completion_task.rs
• test/internal/source-lints/bundler-plugin-hops-no-pass.test.ts

💤 Files with no reviewable changes (1)

• src/runtime/api/js_bundle_completion_task.rs

---

Walkthrough

Changes

The PR refactors deferred plugin scheduling to use arena-allocated tasks and cached plugin pointers. It removes completion-error inspection from completion dispatch and makes deferred promise draining resolve with undefined. A source-lint test validates direct JS-thread plugin hops.

Suggested reviewers: jarred-sumner

Possibly related PRs

• oven-sh/bun#37740: Updates completion dispatch and related bundler task-handling lint coverage.
• oven-sh/bun#37746: Refactors deferred plugin request handling in Graph.rs and bundle_v2.rs.
• oven-sh/bun#37805: Changes bundler plugin handling and JSBundleCompletionTask ownership.

🚥 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 summarizes the main change: preventing plugin hops from accessing the BundleV2 pass.
Description check	✅ Passed	The description explains the problem, fix, verification steps, and test results in sufficient detail.

---

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

[robobun]

Status: ready for review (current revision 9f569d0).

Reproduced as a source-level finding rather than a runtime failure: on main, test/internal/source-lints/bundler-plugin-hops-no-pass.test.ts reports all three plugin hops (&mut *self.bv2 + plugins_mut() in Resolve / Load::run_on_js_thread, get_bundle_v2() + plugins_mut() in DeferredBatchTask::run_on_js_thread); with this branch it passes. The plugin suites listed in the description pass on a debug (ASAN) build of this branch.

Revisions since the first review: the hops carry a copy of the plugin handle and the always-false rejected path of drainDeferred is removed end to end (2f73107); DeferredBatchTask is allocated from the pass's arena instead of being embedded in BundleV2 (107eac1); schedule gets that arena from drain_deferred_tasks instead of reading it back through the pass (c536f60); doc cleanups (d4ade19, 9f569d0). All review threads are resolved and the description matches the current diff.

Related: #37746 and #37740 touch the same lines (different bugs, compose with this one; whichever lands second needs a small rebase, see the comment below).

[claude]

I reviewed this PR and didn't find any bugs. Because it reworks cross-thread unsafe access to BundleV2 (raw-pointer field reads under Stacked/Tree Borrows reasoning) and is one of a family of related aliasing fixes (#37691, #37626), a human look at the memory-model claims would still be worthwhile.

What was reviewed:

• Confirmed BundleV2::plugins_mut had no other callers (the hit in js_bundle_completion_task.rs is a different type's method); get_bundle_v2 / init are fully removed.
• Checked plugins and completion are both Copy fields, so (*this).field reads them without forming a &BundleV2; .is_some_and is equivalent to the old .map().unwrap_or(false).
• Verified the folded schedule() preserves the exact debug-flag sequence of the old init() + schedule() pair; drain_defer_task is pub(crate) and reachable.
• The new source-lint follows the established test/internal/source-lints/ pattern and guards against the hop list drifting.

Extended reasoning...

Overview

This PR removes three sites where the bundler's plugin-thread hops (Resolve::run_on_js_thread, Load::run_on_js_thread, DeferredBatchTask::run_on_js_thread) formed &mut BundleV2 on a thread that does not own the pass, while the bundle thread concurrently holds and mutates through its own &mut BundleV2. It replaces plugins_mut(&mut self) with plugins_on_js_thread(this: *const Self), which reads the immutable-for-the-pass plugins field through a raw pointer and returns the opaque ZST handle. DeferredBatchTask::get_bundle_v2 (the &mut-returning container-of walk-back) and init() are removed; schedule() now takes the &mut BundleV2 its sole caller already has. Four files touched: three Rust sources in src/bundler/ and one new source-lint test.

Security risks

None. This is an aliasing-model correctness change with no observable runtime behavior difference; no user input handling, auth, crypto, or permissions are involved.

Level of scrutiny

High. This is native memory-safety code — the most-blocked category per REVIEW.md — with new unsafe blocks whose SAFETY comments make specific claims about cross-thread field-read races, Stacked/Tree Borrows provenance, and ZST &mut overlap. The reasoning is careful and I believe correct ((*raw_ptr).copy_field is a place-expression read that forms no reference; both fields are Copy and set once before the pass; JSBundlerPlugin is an opaque_ffi! ZST so &mut to it covers zero bytes), but these are exactly the subtle claims a maintainer tracking the #37691 / #37626 family should sign off on.

Other factors

• Verified no orphaned callers: plugins_mut in js_bundle_completion_task.rs is an unrelated inherent method on the completion task, not BundleV2::plugins_mut; get_bundle_v2 and drain_defer_task.init()/.schedule() have zero remaining hits.
• The init() + schedule() fold is behavior-preserving: init did debug_assert!(!running) then mem::take(self) (only field is the debug running: bool, default false); schedule did debug_assert!(!running) then running = false. The new schedule does the same assert + assignment. The odd-looking running = false (rather than true) is pre-existing and preserved verbatim.
• CompletionHandle is #[derive(Copy, Clone)] (bundle_v2.rs:1451) and plugins: Option<NonNull<JSBundlerPlugin>> is Copy, so both raw-pointer field reads are valid copy-outs.
• The source-lint test matches sibling lints (fn-long-mut-reborrow.test.ts, frozen-nonnull-reborrow.test.ts) in structure, uses globAllSources from the shared helper, guards against vacuous passes (scanned > 0, hop-location assertion), and strips comments before matching so prose about the banned shapes doesn't false-positive.
• Existing plugin/defer/bake test suites were run on a debug ASAN build per the PR description; clippy and rustfmt clean.

[robobun]

For whoever takes the human look the review above asks for, the change rests on three facts, each checkable in one place:

1. Both fields are write-once before the pass runs: bv2.plugins / bv2.completion are assigned in JSBundleCompletionTask::init_and_run (src/runtime/api/js_bundle_completion_task.rs:1238-1239) before run_from_js_in_new_thread, and bake assigns plugins in BundleV2::init (bundle_v2.rs:2891). grep -rn '\.plugins = \|\.completion = ' src/ shows no other writes to a BundleV2.
2. plugins_on_js_thread (bundle_v2.rs:221) and the deferred batch read those fields as (*ptr).field on Copy types (Option<NonNull<_>>; CompletionHandle derives Copy at bundle_v2.rs:1452), which is a place read through the raw pointer, not a reborrow of the struct. This is the same shape stop_for_vm_teardown already uses for the completion task's own fields.
3. The returned &mut JSBundlerPlugin points at an opaque_ffi! ZST (bundle_v2.rs:722), so it covers no bytes; plugins_ref() on the bundle thread and the parse workers produce & to the same ZST, as before this change.

Source lints, clippy and Format are green on this push; the rest of CI is still running.

[robobun]

^{Updated 8:32 AM PT - Aug 12th, 2026}

✅ @robobun, your commit 9f569d0d99b93a50a5d1503106ecd03cbae7aa95 passed in Build #93318! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 37709

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

bun-37709 --bun

[robobun]

107eac1 addresses the remaining review finding: DeferredBatchTask is no longer embedded in BundleV2; it is allocated from the pass's arena per drain like Resolve / Load, so nothing the plugin thread reborrows lives inside the pass any more. Description updated; the Bun.build and bake plugin suites pass on that revision.

[claude]

The embedded-field concern from my earlier comment is resolved by 107eac1 — DeferredBatchTask is now arena-allocated per drain and the drain_defer_task field is gone from BundleV2, so the dispatch-site cast! no longer points into the pass. Also verified the removed rejected path was unreachable: completion.result is set only after run_from_js_in_new_thread returns (js_bundle_completion_task.rs:1261), and the drain runs inside is_done while it is still Pending, so result_is_err() was always false.

Extended reasoning...

My prior inline comment (2026-08-12T13:38) flagged that copying plugins into an embedded DeferredBatchTask still left the dispatch-site &mut pointing 8 bytes into BundleV2; commit 107eac1 addressed that by arena-allocating the task and removing the drain_defer_task field, matching the Resolve/Load shape. I also checked that enqueue_on_js_loop_for_plugins (the other call schedule makes through transpiler) reads only plugins/completion/js_poster — no graph.* beyond the heap read the inline nit already covers. The JSBundlerPlugin__drainDeferred signature change is consistent end-to-end (Rust FFI decl, C++ definition, sole caller). Leaving the approval decision to a human given this is cross-thread unsafe/aliasing code.

[robobun]

c536f60 picks up the follow-up nit: drain_deferred_tasks now hands schedule the arena it already holds (self.heap) instead of schedule reading it back through the pass, so the SAFETY comment at the is_done call site stays accurate as written. d4ade19 is a doc trim. No open review threads; description updated.

[claude]

Both earlier findings are addressed at d4ade19: DeferredBatchTask is now arena-allocated per drain (no longer a subobject of BundleV2, so the dispatch arm's &mut no longer points into the pass), and schedule takes the arena as a separate argument so drain_deferred_tasks no longer re-projects graph through transpiler — the SAFETY comment at the is_done call site holds as written. Grepped for drain_defer_task / result_is_err: no remaining references in src/ beyond the one stale trait doc noted inline.

Extended reasoning...

The two aliasing concerns raised in earlier passes were re-checked against the current head (d4ade19). grep -rn 'drain_defer_task' src/ returns nothing — the embedded field, get_bundle_v2(), init(), and the debug running flag are all gone, and DeferredBatchTask is a separate arena allocation like Resolve/Load. schedule(bv2, arena) reads only bv2.plugins (direct BundleV2 field) and calls bv2.enqueue_on_js_loop_for_plugins; nothing reaches bv2.graph. The remaining inline finding is a doc-only nit in a file this PR does not touch.

[robobun]

9f569d0 removes the last doc reference to the deleted result check (CompletionStruct::as_js_bundle_completion_task in BundleThread.rs); git grep for result_is_err / result == .err under src/ is empty now. All review threads are resolved; the diff is otherwise unchanged since c536f60.

[robobun]

Two notes for whoever merges this:

• The review summary above describes drainDeferred as now resolving the promises with undefined; that is what it always did. The removed branch (reject when completion.result was an error) was unreachable, since the batch only runs while result is still Pending, so there is no observable change in this PR.
• Overlap with two later PRs in the same family: bundler: touch outstanding plugin requests through their pointer, not as &mut #37746 rewrites the same Resolve / Load::run_on_js_thread bodies and the tail of drain_deferred_tasks (it still goes through bv2.plugins_mut(), which this PR removes; with this PR its raw-pointer version would read the request's own plugins field instead), and bundler: run a Bun.build on the bundle thread through the task pointer, not a build-long &mut self #37740 rewrites the result_is_err vtable entry this PR deletes. Both compose with this one; whichever lands second needs a small rebase, and I have left a note on each. Bun.serve: release the [serve.static] plugin cell when the server is freed #37805, also listed above, does not touch the same code.

[claude]

I reviewed this revision and found no issues — all three prior findings (embedded DeferredBatchTask storage, schedule re-projecting graph.heap through transpiler, stale result == .err trait doc) are addressed in 107eac1, c536f60, and 9f569d0 respectively. Because the change turns on cross-thread &mut aliasing reasoning in the bundler and drops the rejected arm of JSBundlerPlugin__drainDeferred, a human look would still be worthwhile.

The open comment-cop note on BundleThread.rs:82 is the same pre-existing trait-doc case as the resolved ones on bundle_v2.rs — the edit shortens the comment.

What was reviewed:

• Verified DeferredBatchTask is now arena-allocated (not embedded), so the dispatch-site &mut no longer points into BundleV2.
• Checked schedule(bv2, arena) takes the arena from drain_deferred_tasks's self.heap, keeping the is_done SAFETY comment accurate.
• Confirmed rejected was always false (result is Pending until run_from_js_in_new_thread returns; bake has no completion), so removing the reject branch is behavior-preserving.
• grep for result_is_err / plugins_mut on BundleV2 — no remaining callers.

Extended reasoning...

Overview

This PR removes cross-thread &mut BundleV2 aliasing from the three plugin hops (Resolve, Load, DeferredBatchTask) that a Bun.build pass posts to the plugins' JS thread. Each hop now carries a copy of the plugins handle instead of reaching back into the pass via &mut *self.bv2 / from_field_ptr!. DeferredBatchTask moves from an embedded field of BundleV2 to a per-drain arena allocation. The always-false rejected path of JSBundlerPlugin__drainDeferred is removed end to end (Rust FFI decl, C++ body, CompletionDispatch::result_is_err vtable entry). A source-lint test guards the three hop bodies against reintroducing routes back to the pass.

Security risks

None. No user-controlled input handling, no auth/crypto, no new external surface. The change narrows an existing unsound aliasing pattern.

Level of scrutiny

High. This is memory-safety reasoning about Rust's aliasing model across threads in the bundler core, with an FFI signature change and an arena-lifetime argument for the newly heap-allocated DeferredBatchTask. The reasoning is detailed and I found it sound after three review rounds, but it is exactly the kind of subtle invariant a maintainer should confirm — particularly the claim that completion.result is always Pending during the drain window (which justifies dropping the reject branch) and that the pass's arena outlives every posted DeferredBatchTask including on the cancel path.

Other factors

All three findings I raised in earlier passes were addressed with targeted commits and the author's replies match the code. The bug-hunting system found nothing on this revision. The plugin test suites (bundler_plugin, bundler_defer, bake plugins.test.ts) are reported passing on a debug ASAN build. The one open comment-cop note on BundleThread.rs:82 is a false positive on a trait doc comment this PR shortens. Given the complexity and the critical-path location, deferring rather than approving.

[robobun]

The comment-cop note on BundleThread.rs mentioned above was already answered and resolved (pre-existing trait doc, only shortened here); there are no open review threads on 9f569d0. Format, source lints, clippy and comment-cop are green on this revision; the Buildkite build is still running.

[robobun]

Overlap note: #37883 (under review by alii) now has Resolve / Load::run_on_js_thread on raw pointers, reading the plugin through bv2 raw rather than via plugins_mut(), so the &mut BundleV2 in those two hops is gone there. What remains distinct here is the DeferredBatchTask allocation (not a hand-off frame, so #37883 leaves it alone) and the always-false rejected flag; rebasing onto #37883 once it lands should reduce this PR to those.

[robobun]

Update to my earlier note: #37883 no longer touches run_on_js_thread (it was cut down to the mini-loop entry points and the queue primitive at review; the bundler frames go through #37997 instead), so the run_on_js_thread half of this PR is not covered there after all.