Project thread-pool batch tasks out of the object's pointer, not a reference

[robobun]

Problem

• Eleven places that hand an embedded thread-pool task to a Batch (patch install, isolated install, async HTTP, ThreadPool::each, the bundler's source-map and chunk batches) take the task pointer out of a &mut reference to the object that embeds it.
• The pool calls back with exactly that pointer, and every callback recovers the whole object from it with container-of and then touches the object's other fields. For async HTTP this happens on the calling thread, before the batch is even queued.
• Under Stacked Borrows a raw borrow taken through a reference covers only the task field, so the callback's first sibling-field access is undefined behaviour. The bundler's push-then-last_mut() loops are worse: each last_mut() invalidates the pointers already in the batch, so Batch::push itself trips.
• No crash is known or expected from today's codegen; the addresses are right and only the aliasing model objects. Tree Borrows, which is what bun run rust:miri checks, accepts these shapes. Same contract violation as Hand intrusive work-pool tasks to the pool through the object's pointer, not a reference #37768 and install: schedule the Windows hardlink task through the allocation's own pointer #37865, in the one spelling neither covers.

Fix

• Every site now projects the task field out of a raw pointer to the whole object: a *mut the caller already holds, ptr::from_mut of the slot just written, or as_mut_ptr().add(i) in a second loop that runs after the Vec is full.
• This is correct because a projection through a raw pointer keeps that pointer's whole-object range, and building the batch after the Vec is filled means nothing reborrows the buffer once pointers into it exist. The pointer values are the same as before at every site, so there is no behaviour change.
• Two signatures move with it: PatchTask::schedule takes *mut Self, since its one caller already holds the raw pointer, and HTTP preconnect now goes through AsyncHTTP::schedule so the http crate has one projection site.
• Verification: a new source lint bans this spelling in any Batch::from argument, reports exactly the eleven sites on main and nothing here, and is disjoint from install: schedule the Windows hardlink task through the allocation's own pointer #37865's lint. Existing install, fetch, bundler and source-map tests pass on this branch's debug build; fetch.preconnect was checked by hand.

Background

• Embedded task: the object that wants work done embeds a Task node (callback plus next link), a Batch is a linked list threaded through those nodes, and the pool calls each callback with the node's own pointer. The pool allocates nothing.
• container-of: the callback subtracts the field offset from the *mut Task to get the embedding object back. bun_core::container_of documents that this needs a pointer derived from the object's pointer; a reborrow of just the field does not qualify.
• Stacked Borrows and Tree Borrows are the two aliasing models Miri checks against. Stacked Borrows retags &raw mut obj.field taken through a reference down to that field; &raw mut (*p).field through a raw pointer keeps p's range. Tree Borrows is looser and is what bun's Miri run uses, so it misses this class.
• Source lints (test/internal/source-lints/) are bun tests that scan the Rust tree for a banned spelling, with a per-file allowlist that only ratchets down. They are how a pattern Miri does not cover is kept out of the tree.

Original description

Problem

The remaining Batch::from(..) sites that hand an intrusive task to a thread pool projected the *mut Task out of a reference to the object instead of out of the object's pointer:

site	shape
PatchTask::schedule(&mut self, ..) (src/install/patch_install.rs)	Batch::from(&raw mut self.task)
AsyncHTTP::schedule(&mut self, ..) and preconnect (src/http/AsyncHTTP.rs)	Batch::from(addr_of_mut!(self.task)), addr_of_mut!(async_http.task) with async_http: &mut AsyncHTTP
isolated-install Installer::start_task (src/install/isolated_install/Installer.rs)	Batch::from(&raw mut task.task) with task = &mut self.tasks[i]
ThreadPool::each / each_ptr (src/threading/ThreadPool.rs)	Batch::from(addr_of_mut!(runner_task.task)) from tasks.iter_mut()
compute_data_for_source_map (src/bundler/LinkerContext.rs)	Batch::from(&raw mut line_offset.thread_task) from iter_mut(), twice
generate_chunks_in_parallel (src/bundler/linker_context/generateChunksInParallel.rs)	push(..); Batch::from(&raw mut tasks.last_mut().unwrap().task) per element, four loops

In every case the pool calls back with exactly that pointer and the callback recovers the object from it with container-of and then uses the rest of the object: PatchTask::from_task_ptr (run_from_thread_pool), from_field_ptr!(Task, ..) (isolated install, which also writes result and links next), from_field_ptr!(RunnerTask, ..) (each, reads i and ctx), from_field_ptr!(SourceMapDataTask, ..), from_field_ptr!(PrepareCssAstTask, ..) and pending_part_range_prologue. For AsyncHTTP it happens before the task is even queued: HTTPThread::schedule container-ofs every task in the batch on the calling thread, links the object into queued_tasks through the result, and start_queued_task later ptr::reads the whole struct through it. bun_core::container_of documents that this requires a pointer derived from the object's pointer ("a &mut field reborrow does not suffice"), and WorkPool::schedule_owned projects out of the Box::into_raw pointer for that reason. The other Batch::from sites in the tree already do this, either inline (&raw mut (*parse_task).task, addr_of_mut!((*task).task)) or by passing a pointer a helper projected that way (&raw mut (*task).threadpool_task in the package manager's enqueue helpers); WorkPool::schedule forwards its argument, and the one site that narrows through an accessor is #37865's.

The reference-based spelling does not meet that contract. rustc retags the result of &raw mut <place> unless the place is based on a raw pointer, and under Stacked Borrows that retag covers the projected field only, so the callback's first sibling-field access is out of range. The push + last_mut() loops in generate_chunks_in_parallel have a second problem: every last_mut() goes through Vec::deref_mut, which reborrows the whole buffer and invalidates the pointers already in the batch, so Batch::push itself trips when it links the next task onto the previous one (projecting through ptr::from_mut(last_mut()) is not enough there; the batch has to be built after the Vec is filled). Reduction below. Tree Borrows, which is what bun run rust:miri checks, accepts all of these shapes, none of the crates involved are in its crate set, and no crash is known or expected from today's codegen: the addresses are right, and this only matters to the aliasing model. It is the same contract violation as #37768 (the WorkPool::schedule population, which deliberately leaves Batch::from out of its lint) and #37865 (NewTaskQueue::push, the one Batch::from site that narrows through an accessor; not touched here), in the one spelling neither of them covers.

Reduction under Miri (miri 0.1.0 9f36de775b), one mode per shape

ref is the &raw mut task.task / &raw mut self.task shape, iter_mut the addr_of_mut!(runner_task.task) shape, last_mut the chunk loops; last_mut_from_mut is the chunk loop with only the projection fixed; from_mut, as_mut_ptr and raw are the three shapes this PR converts to. The pool side is container_of followed by a sibling-field read and write, run on another thread. The trailing comments on the --> lines name the statement at each location.

=== Stacked Borrows: ref
error: Undefined Behavior: attempting a read access using <2295> at alloc929[0x10], but that tag does not exist in the borrow stack for this location
  --> src/main.rs:72:25                                   let i = (*job).i;          // the callback
help: <2295> was created by a SharedReadWrite retag at offsets [0x8..0x10]
  --> src/main.rs:87:16                                   batch.push(&raw mut task.task);
=== Stacked Borrows: iter_mut
error: Undefined Behavior: attempting a read access using <2079> at alloc842[0x10], but that tag does not exist in the borrow stack for this location
help: <2079> was created by a SharedReadWrite retag at offsets [0x8..0x10]
  --> src/main.rs:123:28                                  batch.push(std::ptr::addr_of_mut!(j.task));
=== Stacked Borrows: last_mut
error: Undefined Behavior: attempting a write access using <1941> at alloc836[0x8], but that tag does not exist in the borrow stack for this location
  --> src/main.rs:48:22                                   (*self.tail).node.next = task   // Batch::push, calling thread
help: <1941> was created by a SharedReadWrite retag at offsets [0x8..0x10]
help: <1941> was later invalidated at offsets [0x0..0x30] by a Unique retag
  --> src/main.rs:133:41                                  jobs.last_mut()                 // next iteration
=== Stacked Borrows: last_mut_from_mut
error: Undefined Behavior: attempting a write access using <1958> at alloc840[0x8], but that tag does not exist in the borrow stack for this location
  --> src/main.rs:48:22                                   (*self.tail).node.next = task
help: <1958> was created by a SharedReadWrite retag at offsets [0x0..0x18]
help: <1958> was later invalidated at offsets [0x0..0x30] by a Unique retag
  --> src/main.rs:135:48                                  jobs.last_mut()
=== Stacked Borrows: from_mut
from_mut: ok [1, 2, 3]
=== Stacked Borrows: as_mut_ptr
as_mut_ptr: ok [1, 2, 3]
=== Stacked Borrows: raw
raw: ok [1, 2, 3]

Tree Borrows (-Zmiri-tree-borrows): all seven modes ok

Fix

Same pointer values as before at every site, so no behaviour change; what changes is which pointer they are projected from.

• PatchTask::schedule becomes unsafe fn schedule(this: *mut Self, batch) and projects out of this. Its only caller, flush_patch_task_queue, already holds the *mut PatchTask it popped from the fifo and now reads the callback kind through it instead of forming a &mut PatchTask.
• AsyncHTTP::schedule keeps &mut self (its callers, send_sync, NetworkTask, FetchTasklet, S3, hold the object by reference or value, and HTTPThread::schedule recovers an AsyncHTTP, which a &mut AsyncHTTP does cover) and projects through ptr::from_mut(self), a reborrow of the whole object. preconnect goes through it instead of projecting by hand, so the http crate has one projection site.
• Installer::start_task and compute_data_for_source_map project through ptr::from_mut of the slot they just wrote.
• ThreadPool::each_impl and the two batches in generate_chunks_in_parallel fill their Vec first and then build the batch in one loop from as_mut_ptr().add(i); the three copies of the last_mut() push in the chunk loop collapse into that one loop, and the with-capacity comments that justified taking pointers mid-fill go away with the pattern.

Verification

test/internal/source-lints/thread-pool-batch-projection.test.ts bans &raw mut / &raw const / addr_of_mut! / addr_of! of a field path rooted at a binding as the argument of any Batch::from( call (however the type is spelled, including the PoolBatch / ThreadPoolBatch aliases), with a self-test over the spellings of every site in the tree and the converted shapes. Against main it reports exactly the eleven sites above:

src/bundler/linker_context/generateChunksInParallel.rs:117, :203, :226, :248
src/bundler/LinkerContext.rs:589, :590
src/http/AsyncHTTP.rs:391, :530
src/install/isolated_install/Installer.rs:171
src/install/patch_install.rs:711
src/threading/ThreadPool.rs:560

and passes on this branch with no allowlist. It is deliberately disjoint from #37865's lint, which bans the reference / accessor / from_mut shapes in the same argument and reports only src/install/PackageInstall.rs:475 both on main and on this branch (checked by running it here), so the two can land in either order without sharing an allowlist; #37768's lint covers WorkPool::schedule, and both of those also report nothing on this branch.

The converted paths are exercised by existing tests, all run against the debug build of this branch: test/cli/install/bun-install-patch.test.ts (18 pass) and bun-patch.test.ts (31 pass) for PatchTask::schedule and the registry downloads that go through NetworkTask → AsyncHTTP::schedule; test/cli/install/isolated-install.test.ts (62 pass) for start_task; test/cli/install/bun-audit.test.ts (17 pass) for the send_sync path; test/js/web/fetch/fetch-redirect.test.ts (30 pass) and client-fetch.test.ts (34 pass) for the fetch path; test/bundler/bundler_html.test.ts (22 pass; JS, CSS and HTML part ranges plus the CSS AST batch), test/bundler/css/css-modules.test.ts (6 pass), test/bundler/bundler_splitting.test.ts (11 pass; many chunks through each_ptr), test/bundler/bun-build-api.test.ts (52 pass; source maps) and test/js/bun/sourcemap/internal-sourcemap-roundtrip.test.ts (22 pass). fetch.preconnect was checked directly with a local listener (the preconnect opens the connection before the first fetch); test/js/web/fetch/fetch-preconnect.test.ts itself cannot run in this container because localhost resolves to ::1 first here, which makes it fail identically on the released binary, and that is tracked separately. cargo clippy --no-deps on bun_threading, bun_http, bun_install and bun_bundler is clean and cargo fmt is 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: 25 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: 9d9e1b0b-6771-4c04-982d-d83418f184db

📥 Commits

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

📒 Files selected for processing (8)

• src/bundler/LinkerContext.rs
• src/bundler/linker_context/generateChunksInParallel.rs
• src/http/AsyncHTTP.rs
• src/install/PackageManager/runTasks.rs
• src/install/isolated_install/Installer.rs
• src/install/patch_install.rs
• src/threading/ThreadPool.rs
• test/internal/source-lints/thread-pool-batch-projection.test.ts

---

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

[robobun]

^{Updated 3:05 PM PT - Aug 12th, 2026}

❌ @robobun, your commit 36c69aa has 5 failures in Build #93422 (All Failures):

• test/cli/install/migration/complex-workspace.test.ts - code 1: error: Failed to install on 🐧 25.04 aarch64
• test/cli/install/migration/complex-workspace.test.ts - code 1: error: Failed to install on 🐧 3.23 aarch64
• test/js/third_party/grpc-js/test-tonic.test.ts - code 1 on 🐧 13 x64-asan
• test/js/node/worker_threads/worker_threads.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
• scripts/build/ci.ts - build failed on 🐧 x64-musl - build-bun
• /opt/winsysroot/Windows Kits/10/Include/10.0.26100/um/winver.h - clang warning on 🪟 x64 - build-bun

---

🧪   To try this PR locally:

bunx bun-pr 37894

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

bun-37894 --bun

[robobun]

Status: fix and lint are up (head 36c69aa); waiting on CI.

How the defect was confirmed: each of the eleven Batch::from sites was traced to its pool callback (from_task_ptr / from_field_ptr!, or HTTPThread::schedule's container-of on the calling thread), and the argument shapes were reduced under Miri with Stacked Borrows (output in the description): the reference-based projections fail on the callback's first sibling-field access, the push + last_mut() loop fails earlier in Batch::push, and the three converted shapes pass. Tree Borrows accepts all of them, so there is no crash to reproduce; test/internal/source-lints/thread-pool-batch-projection.test.ts reports exactly the eleven sites on main and nothing on this branch.

Review so far: the comment-length findings are addressed (each site is down to its one-line SAFETY note; the unsafe fn keeps its one-line Safety contract), threads resolved. The automated review-limit notices above need no action.

[github-actions]

This PR may be a duplicate of:

1. install: schedule the Windows hardlink task through the allocation's own pointer #37865 - Adds a near-identical source lint over the same Batch::from( argument position (thread-pool-batch-field-ref.test.ts vs thread-pool-batch-projection.test.ts), with contradictory doctrine: its self-test declares &raw mut self.task / addr_of_mut!(runner_task.task) allowed, which are exactly the sites this PR bans and converts.

🤖 Generated with Claude Code

[robobun]

Not a duplicate of #37865, though the two are siblings and were checked against each other.

#37865 converts one site, NewTaskQueue::push (src/install/PackageInstall.rs:475), where the argument goes through an accessor returning &mut Task, and its lint bans that family of shapes (a &mut or accessor in the argument, from_mut / from_ref). It does not touch any of the eleven sites converted here, and its lint treats their spellings as acceptable on purpose, so the two PRs have no files or lint findings in common: its lint, run against this branch, still reports exactly PackageInstall.rs:475; the lint added here reports the eleven sites on main and nothing on either branch. They can land in either order without touching each other's allowlists (neither has one).

The disagreement the bot noticed is in the prose: #37865's lint header says &raw mut self.task has the whole object's range. That holds under Tree Borrows, which is what bun run rust:miri runs, and not under Stacked Borrows, where the reduction in the description above fails on exactly that spelling (and container_of's documented contract asks for a projection from the object's pointer either way). That is the gap this PR closes; I have left a note on #37865 so its header can be reworded when it lands, since its self-test strings are literals and keep passing as they are.

[claude]

I reviewed this PR and didn't find any bugs. The pointer-provenance conversions look correct and behavior-preserving at every site, and the new source lint's classifier matches what it claims. Because it touches intrusive-task scheduling across five subsystems (thread pool, bundler, HTTP client, package manager, isolated install) with subtle Stacked Borrows reasoning, changes PatchTask::schedule from a safe method to an unsafe fn(*mut Self), and coordinates with sibling PRs #37768 / #37865, a human look would still be worthwhile.

What was reviewed:

• Each of the eleven converted sites: verified the new projection (ptr::from_mut, as_mut_ptr().add(i), or raw *mut Self) yields the same address with whole-object provenance, and that the batch/schedule call sequence is unchanged.
• generateChunksInParallel.rs: confirmed splitting fill-then-batch preserves the exact task set and order for both the CSS-AST and part-range loops.
• preconnect → schedule refactor: Batch::default() + push produces the identical single-task batch as the old direct Batch::from.
• flush_patch_task_queue: (*patch_task).callback.is_apply() matches the old matches!(pt.callback, Apply(_)); no &mut PatchTask is formed.

Extended reasoning...

Overview

This PR converts eleven Batch::from(..) call sites that hand an intrusive *mut Task to a thread pool so that the task-field pointer is projected out of a raw pointer to the embedding object rather than out of a reference. The affected sites span src/threading/ThreadPool.rs (each_impl), src/bundler/LinkerContext.rs (compute_data_for_source_map), src/bundler/linker_context/generateChunksInParallel.rs (two batches), src/http/AsyncHTTP.rs (schedule and preconnect), src/install/isolated_install/Installer.rs (start_task), src/install/patch_install.rs (schedule, now unsafe fn(this: *mut Self, ..)), and its sole caller in src/install/PackageManager/runTasks.rs. A new source-lint test bans the reference-based projection shape going forward.

The rationale is that every callback behind these sites recovers the whole embedding object via container_of / from_field_ptr! and touches sibling fields; bun_core::container_of documents that the input must be derived from the object's pointer, and under Stacked Borrows a &raw mut <ref>.field retag covers only the field. The PR description includes a Miri reduction demonstrating each shape. Tree Borrows (what bun run rust:miri runs) accepts the old shapes, and no runtime crash is expected — the addresses are identical; only provenance changes.

Security risks

None. This is a pointer-provenance / aliasing-model correctness change with no user-facing input handling, no auth/crypto/permissions surface, and no data-flow changes. The pointer values produced are byte-identical to before.

Level of scrutiny

High. Every touched site is on a concurrency-critical path: intrusive thread-pool task hand-off in the bundler's parallel chunk generation and source-map computation, the HTTP client's request scheduling (including fetch.preconnect), the package manager's patch-task and isolated-install task queues, and ThreadPool::each itself. The changes are mechanically simple per site (rebase the field projection onto a raw pointer), but the correctness argument depends on Stacked Borrows semantics that are not enforced by the compiler or by bun run rust:miri (Tree Borrows). I traced each site's before/after to confirm the projected address is unchanged and the batch contents/order are preserved; for the generateChunksInParallel.rs loops I confirmed the fill-then-batch split yields the identical task set, and for the iter_mut()-based sites (LinkerContext, Installer) I confirmed ptr::from_mut(element) gives whole-element provenance that survives iterator advancement (matching the PR's from_mut Miri mode passing).

Other factors

• API change: PatchTask::schedule goes from a safe &mut self method to an unsafe fn(this: *mut Self, ..) with a documented Safety contract. Its only caller (flush_patch_task_queue) already holds the *mut PatchTask from the fifo and now avoids forming the intermediate &mut PatchTask. This is the right shape for the contract but is a signature change a maintainer should sign off on.
• Sibling-PR coordination: The PR is deliberately disjoint from #37865 (which handles the accessor / &mut argument shapes) and #37768 (WorkPool::schedule). The duplicate-PR bot flagged #37865 and the author explained the split; #37865's lint header states &raw mut self.task has whole-object range, which this PR contradicts (correctly, under Stacked Borrows). That coordination is worth a maintainer's eye.
• CI: Build #93422 is still running with an x64-musl build failure in scripts/build/ci.ts and an unrelated Windows-SDK header clang warning. I could not determine whether the musl failure is related to this change.
• Test coverage: The new lint has self-tests for its classifier and matcher covering every converted spelling and the accepted shapes; the ratcheted allowlist is empty. The PR description lists the existing test suites exercised for each converted path (patch install, isolated install, audit, fetch, bundler HTML/CSS/splitting/sourcemaps), all passing on the debug build. The comment-cop bot's length findings were addressed in follow-up commits and all threads are resolved.
• No prior claude[bot] reviews on this PR.

[robobun]

On the one open point in the review above: the x64-musl build failure in build 93422 is the dependency fetch step (c-ares, mimalloc, the WebKit tarball and lol-html all failed to download from github.com after 5 attempts), not anything this branch compiles. The same outage shows up in that build's two red tests that download from github.com (sharp's libvips in complex-workspace.test.ts on the aarch64 lanes, protoc in test-tonic.test.ts); those and the worker_threads exception-check assertion on the ASAN lane are reported separately. Every lane that ran the install, fetch and bundler suites exercised by this change passed, as did the source-lints job that runs the new lint.