workers: join in-flight transpiler jobs before freeing the VM on terminate

[robobun]

Fixes #33936

A TranspilerJob for a dynamic import() runs on the shared work pool and reads the owning VirtualMachine allocation throughout run(): the generation check, a ptr::read of the whole Transpiler (whose defines table the parser walks in e_dot), test_isolation_enabled, source_mappings, and the job slot itself lives inside vm.transpiler_store (the first 64 slots are an inline HiveArray in the VM allocation, so a queued job's WorkPool link is inside that box too). worker.terminate() landing while the job is queued or running freed all of that in WebWorker::shutdown, so the pool thread kept reading freed memory.

Repro

A worker that dynamic-imports a large module and is terminated right after scheduling it:

// racer-worker.js
import("./big.ts").then(() => postMessage("done")).catch(() => {});
postMessage("started");
// parent: on "started" -> worker.terminate()

Under ASAN this aborts deterministically once the process outlives the orphaned transpile:

==23556==ERROR: AddressSanitizer: heap-use-after-free ... thread T12 (Bun Pool 0)
    READ of size 8 in <hashbrown::raw::RawTable<(StringHashMapKey, Vec<DotDefine>), DefaultAlloc>>::len
    ... bun_js_parser P::e_dot -> TranspilerJob::run (RuntimeTranspilerStore.rs:883)
freed by thread T10 (Worker):
    <Transpiler>::deinit <- <VirtualMachine>::destroy <- <WebWorker>::shutdown

Stock release canary segfaults at 0x30 (panic: Segmentation fault at address 0x30, banner Crashed while printing <mod>.mjs).

Fix

Track pool-side jobs with an in_flight counter on RuntimeTranspilerStore:

• incremented in TranspilerJob::schedule() (JS thread, before the job is handed to the pool)
• decremented on the pool thread after run() and its dispatch back to the JS thread complete

VM teardown joins on the counter before freeing anything run() reads: in WebWorker::shutdown before the step-5 loop teardown, and at the top of VirtualMachine::destroy() for the destruct-on-exit path. No new job can be scheduled while it blocks, since scheduling only happens on the (now waiting) VM thread.

The waiter bumps generation_number first so jobs still queued on the pool bail at the top of run() instead of doing a full parse of a dead VM's import, and polls with a 1 ms futex timeout instead of a decrement-side Futex::wake: the worker thread frees the VM allocation (including the counter) right after the wait returns, so the pool thread's final access to the VM must be the fetch_sub itself; a wake after the decrement could land on freed memory.

The dispatch wakeup is a single ConcurrentTask node embedded in the store (guarded by a coalescing flag) instead of a Box per dispatch: a Box'd node still sitting in the concurrent queue at VM teardown is unreachable once the VM allocation is freed. The embedded node dies with the VM box, and repeat dispatches coalesce into one wakeup.

Verification

New ASAN-gated test in test/js/web/workers/worker-terminate-lifetime.test.ts: a racer worker is terminated mid-transpile while a second worker's identical import keeps the process alive for the whole exposure window. Fails on unfixed main with the heap-use-after-free above; passes with the fix (the terminated worker's teardown now waits out the in-flight parse, ~4.3 s under debug ASAN).

Out of scope (pre-existing, reproduce on stock main)

• worker-terminate-lifetime.test.ts dns.lookup case: LSan reports a 4104-byte leak in node_fs_binding::createBinding on current main, unrelated to this diff.
• Under the aggressive multi-lane repro, once the transpiler UAF no longer kills the process first, a separate debug-only JSC assertion object->structure() == this (StructureInlinesLight.h:56) fires during worker terminate(). Confirmed on stock main with tiny modules (so the UAF window never opens). Tracked separately.

Rebased from the earlier revision of this PR onto current main, plus the generation_number bump so queued jobs bail fast instead of parsing to completion before the wait returns.

[robobun]

Status: rebased onto f68e504ae4, gate proof re-derived locally (fail-before: ASAN heap-use-after-free in e_dot reading the freed DotDefine table; pass-after: ~4.3 s). Added generation_number bump so queued jobs bail fast instead of parsing to completion.

Repro: bun bd test test/js/web/workers/worker-terminate-lifetime.test.ts -t "in-flight dynamic import"

PR: #33939. Waiting on CI.

[coderabbitai]

Walkthrough

Changes

Transpiler teardown synchronization

Layer / File(s)	Summary
Track and await in-flight jobs src/jsc/RuntimeTranspilerStore.rs	Adds atomic job tracking, waiting, scheduling increments, and worker completion decrements.
Wait before VM teardown src/jsc/VirtualMachine.rs, src/jsc/web_worker.rs	VM and web worker destruction now wait for transpiler jobs before tearing down event loops and VM resources.
Worker termination regression coverage test/js/web/workers/worker-terminate-lifetime.test.ts	Adds an ASAN-skipped test for terminating a worker during dynamic-import transpilation.

Possibly related issues

• TranspilerJob lives inside the VM allocation, so a pool-thread transpile racing worker.terminate() reads freed memory #33936 — Adds in-flight transpiler job tracking and teardown waits matching the issue’s described race.

🚥 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 and concisely describes the primary fix: waiting for in-flight transpiler jobs before freeing the VM during worker termination.
Description check	✅ Passed	The description explains the defect, fix, reproduction, verification, and out-of-scope issues, covering the required template information.

---

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

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/jsc/web_worker.rs`:
- Around line 1283-1290: Move the wait_for_inflight_jobs() call in the VM
shutdown path to immediately before the JSC VM teardown step, rather than after
teardown has started. Update the surrounding cleanup ordering in the relevant
web worker destruction routine so any TranspilerJob accessing VM state has
joined before WebWorker__teardownJSCVM, while retaining the later destroy()
safety wait if required.

In `@test/js/web/workers/worker-terminate-lifetime.test.ts`:
- Around line 141-159: Update the worker fixtures in main.js so import failures
are observable: post the caught import error from racer-worker.js and
ref-worker.js instead of swallowing it, and reject the corresponding promises
when either worker emits an error event. Ensure the terminated and refDone
handshakes await success messages while propagating all fixture failures
immediately.
- Around line 174-179: Update the worker termination test assertion to validate
the raw stderr value unconditionally before checking stdout and exitCode; remove
the AddressSanitizer-based ternary filtering so any unexpected child diagnostics
cause the test to fail, while preserving the expected stdout and zero exit code
checks.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ca3ee3fe-d2b4-42d0-be3f-9278f1321282

📥 Commits

Reviewing files that changed from the base of the PR and between 095eb31 and 29a175c.

📒 Files selected for processing (4)

• src/jsc/RuntimeTranspilerStore.rs
• src/jsc/VirtualMachine.rs
• src/jsc/web_worker.rs
• test/js/web/workers/worker-terminate-lifetime.test.ts

[claude]

No new issues found. f22c03e addresses the ConcurrentTask node portion of my earlier leak note by embedding the wakeup node in the store (auto_delete: false, so the tick's heap::take skips it) — the job-payload leak remains scoped out per the reply. The coalescing flag looks sound: it's cleared in run_from_js_thread only after concurrent_tasks.pop_batch() has already unlinked the node in the tick that dispatched it, so a re-enqueue can't double-link.

Deferring to a human on this one — cross-thread VM teardown ordering plus a new embedded intrusive-node coalescing scheme is the kind of change that benefits from a second pair of eyes.

Extended reasoning...

Overview

The PR fixes a heap-use-after-free where a TranspilerJob running on the shared work pool reads a worker's VirtualMachine allocation after worker.terminate() has freed it. Four files touched: RuntimeTranspilerStore.rs (+54, adds in_flight counter, wait_for_inflight_jobs, embedded dispatch_task node), VirtualMachine.rs (+6, join at top of destroy), web_worker.rs (+8, join before step-5 loop teardown), and a new ASAN-gated regression test.

Since my previous review (the leak note on web_worker.rs:1290), commit f22c03e replaced the per-dispatch Box<ConcurrentTask> with a single ConcurrentTaskItem embedded in the store, guarded by an AtomicBool coalescing flag.

What I checked in f22c03e

• ConcurrentTaskItem::default() has auto_delete: false, so tick_concurrent_with_count's heap::take path (event_loop.rs:508) skips the embedded node — no bogus free.
• Re-enqueue safety: the flag is cleared in run_from_js_thread, which is reached via dispatch.rs:351 only after the tick has already pop_batch'd the node out of concurrent_tasks and moved its .task to self.tasks. So the intrusive next link is not in the queue when a producer wins the false→true swap and re-enqueues. The .task write is ordered before the consumer's read by the queue's push-Release / pop-Acquire.
• init_in_place (currently unused) and Default both initialize the three new fields.
• The fetch_sub(Release) in run_from_worker_thread is placed after run() returns (whose defer has already called dispatch_to_main_thread → touched dispatch_task / event_loop), so the pool thread's final VM access is indeed the decrement, matching the poll-not-wake rationale in wait_for_inflight_jobs.

Security risks

None user-facing; this is internal lifetime management. The pre-fix state was a UAF (worse).

Level of scrutiny

High. This sits in the numbered-barrier WebWorker::shutdown sequence, adds a futex-polled join on an atomic counter with a documented reason for not waking, and introduces a shared embedded intrusive node with a hand-rolled coalescing gate. Memory-safety-critical, cross-thread, and the correctness argument depends on the exact ordering of tick → pop_batch → dispatch → flag-clear.

Other factors

All CodeRabbit threads resolved (two applied, one withdrawn after verification). My earlier bounded-leak note is acknowledged as pre-existing and deferred to a follow-up alongside #32071. The new test is ASAN-gated with verified fail-on-main / pass-on-PR evidence in the description. Not approving because this is exactly the class of change (concurrency + VM teardown + memory safety) that should get a human maintainer's sign-off.

[robobun]

CI status: the only failure on build 71661 is test/js/bun/http/proxy-stress-concurrent.test.ts, a pre-existing 9 byte leak in the DNS pending cache (Resolver::get_or_put_into_pending_cache, dns.rs:4696) reported by LSAN on the x64-asan lane. That path is untouched by this diff (workers / transpiler store), and the PR's own test passed on this build after f22c03e removed the dispatch-node leak. Everything else is green; this is ready for review.

[robobun]

Superseded by the Worker rewrite in #37075, which landed on main and closed #33936. Transpiler jobs now reach the VM only through the per-VM handle that teardown closes, and teardown waits for or releases in-flight off-thread work before freeing the VM, so the TranspilerJob reads this PR guarded against can no longer land on a freed VM.

Verified on current main (165dc9f, debug+ASAN build): this PR's test, "terminate() racing an in-flight dynamic import transpile does not UAF", passes 4 out of 4 runs. Closing.