install: schedule the Windows hardlink task through the allocation's own pointer

[robobun]

Problem

• On Windows, the bun install hardlink backend hands each link task to the thread pool as a pointer taken from a &mut accessor to the task's embedded pool node, so the pointer covers that one field only.
• The pool's callback treats that pointer as the whole object: it walks back to the enclosing task, reads and writes its other fields, and frees it. All of that is outside the field. Stacked Borrows rejects the first such access; Tree Borrows (what bun run rust:miri checks) rejects the free.
• No crash is known; the address is right, so current codegen does what was intended. Same shape as Hand intrusive work-pool tasks to the pool through the object's pointer, not a reference #37768 and node:zlib: schedule the async write through the handle's own pointer #37772. Every other hand-off of this kind in the tree already projects the field from the object's pointer; this was the only one using an accessor.

Fix

• The queue projects the node field out of the object's own pointer, and the callback walks back through the same declaration, so both directions share one offset. The accessor trait existed only for this site and is removed.
• The pointer value the pool receives is unchanged (same field, same allocation), so behaviour is unchanged; it now carries the whole allocation's range, which is what the callback's writes and free need.
• A new source lint bans forming a batch's task pointer from a reference to the field. Against main it reports exactly this site; on this branch it passes with no allowlist.
• Verification: on a Windows x64 debug build, bun install --backend hardlink of a 51-file tarball linked every file, a --force reinstall succeeded, and the install tests that use this queue pass. cargo check passes for both Windows targets. Make the Windows hardlink install backend link serially #33113 would delete this path; if it lands first, this change goes with it.

Background

• Intrusive work tasks: bun's thread pool allocates nothing per job. Each job struct embeds a pool node, the pool is given a pointer to that node, and the callback subtracts the node's offset (container-of) to recover the job, usually freeing it at the end.
• Provenance: under Rust's aliasing rules a raw pointer may only touch the range it was derived from. A pointer from &mut obj.field covers the field; a raw projection from *mut Obj (&raw mut (*p).field, addr_of_mut!) covers the object. Same address either way, so this only shows up under Miri.
• Stacked Borrows and Tree Borrows are the two aliasing models Miri can check. Tree Borrows accepts more programs; bun's miri run uses it, and it still rejects the writes and the free here.
• intrusive_work_task! declares which field of a struct is its pool node and derives both projections (object to node, node to object) from that one fact.
• Source lints: test/internal/source-lints/ holds bun test files that scan the Rust tree for a banned shape and fail on any occurrence outside a per-file allowlist that only ratchets down.

Original description

Problem

NewTaskQueue::push (src/install/PackageInstall.rs, Windows only, the bun install hardlink backend) handed the thread pool its task like this:

self.thread_pool.schedule(Batch::from(unsafe {
    std::ptr::from_mut::<WorkPoolTask>((*task).task())   // HasWorkPoolTask::task(&mut self) -> &mut WorkPoolTask
}));

task() is &mut self.task, so the *mut WorkPoolTask the pool gets is derived from a reborrow of the task field and carries provenance for that field only, even though task: *mut HardLinkWindowsInstallTask, the pointer heap::into_raw returned in init(), was right there. The pool calls run_from_thread_pool with exactly that pointer, and the callback treats it as a pointer to the whole object: from_field_ptr! walks back to the HardLinkWindowsInstallTask, run() reads src_len / basename and writes through bytes, the error path writes err, and both paths end in heap::take, which frees the Box through it. All of that is outside the range the pointer was derived from. bun_core::container_of's contract says so explicitly ("a &mut field reborrow does not suffice"); Stacked Borrows rejects the first out-of-range access and Tree Borrows (the model bun run rust:miri checks) rejects the free.

No crash is known. The address is right, so today's codegen does what was intended; this is an aliasing-model violation of the same shape as the FileCloser accessor in #37768 and the zlib task().as_ptr() site in #37772, in the one population (Batch::from, the raw ThreadPool API) neither of those touches. The other Batch::from sites in the tree, including UninstallTask a few hundred lines down in the same file, already project the field out of the object's pointer (addr_of_mut!((*task).task), &raw mut (*p).task); this was the only one that went through an accessor.

Fix

push bounds TaskType: IntrusiveWorkTask and schedules Batch::from(TaskType::field_of(task)), projecting the field out of the into_raw pointer itself, the same thing WorkPool::schedule_owned (src/threading/work_pool.rs) does. HardLinkWindowsInstallTask gets bun_threading::intrusive_work_task!(HardLinkWindowsInstallTask, task) and run_from_thread_pool recovers the object with Self::from_task_ptr, so the outbound projection and the inbound container-of share one offset declaration. HasWorkPoolTask existed only for this site and is removed.

The pointer value the pool receives is unchanged (same field of the same allocation), so there is no behaviour change; the task's provenance now matches what the callback does with it, which is what makes heap::take in the callback correct rather than correct by accident.

#33113 would delete this code path altogether by linking serially; it has been open since June with conflicts and a pending design question, so this fixes the site as it stands. If that lands first, this change goes away with it.

Verification

test/internal/source-lints/thread-pool-batch-field-ref.test.ts scans the argument of every Batch::from( call (however the type is reached, including the PoolBatch / ThreadPoolBatch aliases) and bans projecting the task out of a reference to the field: a method chain rooted at a receiver ((*task).task(), this.task(), self.task.as_ptr()), a reference formed in the argument (&mut task.task), or from_mut / from_ref. Raw projections (&raw mut (*p).task, addr_of_mut!(..)), associated fns given the object's pointer (T::field_of(p)) and locals pass; the self-test covers the spellings of all 30 sites in the tree. Against main it reports exactly

src/install/PackageInstall.rs:475

and passes with this branch, with no allowlist. WorkPool::schedule, the other hand-over, is the subject of #37772's lint; this one is scoped to Batch::from so the two do not share an allowlist.

Behaviour, on a Windows x64 debug build of this branch: bun install --backend hardlink --linker hoisted of a local tarball with 51 files across nested directories installs all 51 as hard links of the cache entry (the nested directories take the mkdir_recursive_os_path branch of run()), a --force reinstall goes through the queue's re-init path and succeeds, and test/cli/install/bun-add.test.ts (54 pass) and bun-link.test.ts (4 pass) pass; every package those install on Windows goes through NewTaskQueue::push.

cargo check -p bun_install passes for x86_64-pc-windows-msvc, aarch64-pc-windows-msvc and the Linux host; cargo clippy -p bun_install on the host is clean, and on the Windows target reports the same pre-existing items as main (none on the changed lines); cargo fmt is clean. bun test test/internal/source-lints/ passes (87 tests).

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 15 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: 80d46148-85ea-414b-8b0a-b512cc61e6da

📥 Commits

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

📒 Files selected for processing (2)

• src/install/PackageInstall.rs
• test/internal/source-lints/thread-pool-batch-field-ref.test.ts

---

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

[robobun]

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

❌ @robobun, your commit ea61fa1 has some failures in Build #93329 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 37865

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

bun-37865 --bun

[robobun]

Status: fix and lint are in place; CI is running.

Reproduced how: the new test/internal/source-lints/thread-pool-batch-field-ref.test.ts reports src/install/PackageInstall.rs:475 (the from_mut((*task).task()) argument of Batch::from in NewTaskQueue::push) against main and passes with this branch.

Windows x64 debug build of this branch: a 51-file nested local tarball installs with all 51 files hard-linked to the cache, a --force reinstall succeeds, and test/cli/install/bun-add.test.ts (54 pass) and bun-link.test.ts (4 pass) pass. The crate also checks on aarch64-pc-windows-msvc and Linux.

[claude]

I reviewed this PR and didn't find any bugs. The change looks correct and follows the established IntrusiveWorkTask::field_of / from_task_ptr pattern, but since it touches unsafe pointer-provenance handling in a thread-pool hand-off path, a human look would still be worthwhile.

What was reviewed:

• field_of projects via byte_add on the raw into_raw pointer, so the scheduled *mut Task carries whole-allocation provenance for the callback's container-of and heap::take.
• intrusive_work_task! and from_task_ptr share one offset_of! declaration, so outbound projection and inbound recovery cannot drift; HasWorkPoolTask had no other users.
• The new source-lint follows the sibling lints in test/internal/source-lints/ (non-vacuous scan guard, self-tested classifier, ratcheted allowlist).

Extended reasoning...

Overview

This PR fixes a pointer-provenance issue in NewTaskQueue::push (src/install/PackageInstall.rs, Windows-only bun install hardlink backend). The old code derived the *mut WorkPoolTask handed to the thread pool from a &mut self.task reborrow via a local HasWorkPoolTask::task() accessor, which under Stacked/Tree Borrows carries provenance for the field only — but the callback run_from_thread_pool recovers the whole HardLinkWindowsInstallTask via container-of, reads/writes sibling fields, and frees the allocation with heap::take. The fix bounds push on IntrusiveWorkTask and schedules Batch::from(TaskType::field_of(task)), projecting the field directly out of the heap::into_raw pointer (byte_add on the raw pointer, no intermediate reference). HardLinkWindowsInstallTask gets intrusive_work_task!(.., task) and the callback uses Self::from_task_ptr, so both directions share one OFFSET constant. The now-unused HasWorkPoolTask trait is deleted. A new source lint (thread-pool-batch-field-ref.test.ts) scans every Batch::from( argument tree-wide and bans the reference-derived shapes.

Security risks

None identified. This is an aliasing-model correctness fix; the address handed to the pool is unchanged, so runtime behaviour on today's codegen is identical. No untrusted input, auth, crypto, or network surface is touched.

Level of scrutiny

High. This is unsafe Rust in a threading hand-off — precisely the "native code: memory safety" category REVIEW.md calls the most-blocked. The change itself is small and mechanical (converge on the canonical IntrusiveField::field_of / from_task_ptr pattern already used by WorkPool::schedule_owned and every other Batch::from site in the tree), and it's the third in a series of identical-shape fixes (#37768, #37772). I verified: field_of in bun_core is this.byte_add(OFFSET).cast() on the raw pointer (provenance-preserving); intrusive_work_task! expands to intrusive_field! which sets OFFSET = offset_of!(Self, task); from_task_ptr wraps from_field_ptr with the same offset; Batch::from still receives *mut WorkPoolTask as before; and grep confirms HasWorkPoolTask had no other users. The source lint mirrors the structure of the existing lints (self-receiver-reclaim.test.ts, frozen-nonnull-reborrow.test.ts) with a non-vacuous scan guard and classifier self-test.

Other factors

The PR description is thorough, cites the bun_core::container_of contract, cross-references the two sibling PRs, and reports cargo check passing on both Windows targets plus the Linux host. The lint's allowlist is empty and ratcheted. Everything I checked lines up. That said, unsafe pointer-provenance changes in a thread-pool ownership hand-off are exactly where a human maintainer with the relevant memory-model context should sign off rather than an automated review, so I'm deferring rather than approving.

[robobun]

Cross-reference from #37894, which converts the remaining Batch::from sites that project through a reference (&raw mut self.task, addr_of_mut!(runner_task.task), the last_mut() loops in generateChunksInParallel.rs) and adds a lint for that family of spellings, deliberately disjoint from the one here: this lint still reports exactly PackageInstall.rs:475 on that branch, and that one reports nothing on this branch, so the two land independently.

One prose point for whenever this is next touched: the header here says &raw mut self.task "has the whole object's range and passes". That is true under Tree Borrows (what bun run rust:miri runs) but not under Stacked Borrows, where rustc retags a &raw mut taken through a reference to the field's range; the reduction in #37894's description fails on that spelling. The self-test strings in the allowed list are literals and keep passing either way, so nothing here needs to change for #37894; only the sentence would be worth rewording to "is the other lint's business" or similar.