bundler: join in-flight pool tasks before tearing the bundle down

[robobun]

Repro

echo 'console.log(1)' > app.js
bun build ./missing.js ./app.js --outdir dist        # debug build: aborts every run
bun build ./app.js ./nodir/x.js --outdir dist        # debug build: SEGV on most runs
printf 'import { nope } from "./lib.js";\n' > entry.js; echo 'export const yes = 1' > lib.js
bun build ./entry.js --sourcemap --outdir dist       # debug build: SEGV or abort on most runs

All three should print the build error and exit 1. On a debug (ASAN) build of main they die during teardown instead, in one of two ways:

AddressSanitizer: SEGV on unknown address (READ), rdi = 0xbebebebebebebee6
    #2 <bun_threading::thread_pool::node::Queue>::push            src/threading/ThreadPool.rs:1523
    #3 <bun_threading::thread_pool::Thread>::push_idle_task        src/threading/ThreadPool.rs:1129
    #4 <bun_bundler::thread_pool::Worker>::deinit_soon             src/bundler/ThreadPool.rs:575
    #5 <BundleV2>::deinit_without_freeing_arena                    src/bundler/bundle_v2.rs:5032
    #6 <BundleV2>::generate_from_cli                               src/bundler/bundle_v2.rs:4016

mimalloc: assertion failed: at "../../vendor/mimalloc/src/threadlocal.c":184, mi_thread_local_get_regular
  assertion: "tls!=NULL"

(the second one aborts on a Bun Pool N thread while the main thread is already inside exit). Release builds have the same race without the diagnostics; the usual visible outcome there is just the error line.

Cause

Every entry point is handed to the thread pool as soon as it resolves (the runtime's own parse task even earlier), and link() schedules the source-map tasks before any of its error returns. A pass that fails after either point (generate_from_cli, generate_from_bake_production_cli, and the enqueue failure path of run_from_js_in_new_thread) went straight to deinit_without_freeing_arena with those tasks still running Worker::get on the pool:

• ThreadPool::get_worker_slow publishes the Worker pointer in workers_assignments before writing the struct, so teardown walking the map read the thread field of an uninitialized allocation (ASAN's 0xbe fill, hence the address above) and pushed onto it.
• A Worker created after teardown walked the map was never torn down; the work its thread has left to do on exit then races the main thread's exit() once the error is printed, which is the mimalloc assertion.

The Zig version never freed anything on the CLI error path, so this appeared with the port's teardown-on-every-exit. run_from_js_in_new_thread already waited for the parse stage even on error, and Bun.build's caller waited on the two source-map wait groups before deinit; the CLI drivers did neither.

Fix

deinit_without_freeing_arena now joins whatever is still on the pool before it touches the workers: wait_for_parse() when pending_items > 0 (synchronous passes only), then the two source-map wait groups (both are no-ops when nothing was scheduled or the join already happened). Teardown is the one place every driver funnels through and the point where the "no task is still running against this bundle" invariant is actually needed, so enforcing it there covers bun build, bake production, Bun.build, and the exits no driver handled (the dependency scanner return after link, enqueue failures) in one place. The now redundant wait-group waits in init_and_run are removed.

The dev server's asynchronous pass is excluded on purpose: it is driven by the JS event loop and only reaches this function once is_done(), and ticking that loop from teardown would be wrong. Tearing a dev server or VM down with tasks in flight is a different bug (#31702).

A user-visible consequence for the CLI: when one entry point does not resolve, errors from the entry points that did resolve are now reported as well, which is what Bun.build() already did.

Verification

test/bundler/cli.test.ts gains the three repros above, asserting the exact stderr and exit code. On the unfixed debug build the first fails on every run (exit 134 plus the mimalloc lines), the other two on most runs (ASAN report); all pass with the fix. The existing Bun.build "sourcemap + build error crash case" covers the JS API side now relying on the join in teardown.

Rates on the unfixed and fixed debug build (20 sequential / 24 runs at 8-way concurrency each)

layout	unfixed	fixed
./missing.js ./app.js	20/20 abort, 24/24	0/20, 0/24
./app.js ./nodir/x.js	19/20 SEGV, 19/24	0/20, 0/24
./entry.js --sourcemap (bad named import)	17/20, 20/24	0/20, 0/24

Also run with the fix: bun-build-api.test.ts, bundler_html.test.ts, bake/dev/production.test.ts, bake/dev/bundle.test.ts, bake/deinitialization.test.ts, cli/test/test-changed.test.ts.

[robobun]

Status: reproduced on a debug build of main (bun build ./missing.js ./app.js aborts 20/20, ./app.js ./nodir/x.js SEGVs 19/20, --sourcemap plus a bad named import 17/20); 0 failures in the same runs with this branch. Fix and tests are in this PR; waiting on CI.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 4 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: 1467196f-04aa-474d-8f79-8bfed483393b

📥 Commits

Reviewing files that changed from the base of the PR and between da3851e and 7655228.

📒 Files selected for processing (4)

• src/bundler/BundleThread.rs
• src/bundler/bundle_v2.rs
• src/runtime/api/js_bundle_completion_task.rs
• test/bundler/cli.test.ts

---

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

[claude]

I reviewed this PR and didn't find any bugs. Because it changes thread-pool teardown ordering across every bundler driver, a human look at the !asynchronous guard and the assumption that every synchronous caller has an event loop set would still be worthwhile.

What was reviewed:

• WaitGroup::wait() is a no-op on a default (count 0) or already-joined group, so the unconditional waits in deinit_without_freeing_arena are safe on paths that never scheduled source-map tasks or already joined them in generateChunksInParallel.
• All three synchronous drivers (generate_from_cli, generate_from_bake_production_cli, init_and_run) pass an event_loop to BundleV2::init, so wait_for_parse()'s r#loop().expect(...) won't panic when reached from teardown.
• The removed error-path waits in init_and_run are subsumed by the new waits inside deinit_without_freeing_arena, which now runs on both arms.

Extended reasoning...

Overview

The PR moves the join of in-flight parse and source-map thread-pool tasks into BundleV2::deinit_without_freeing_arena so that every driver (CLI, bake production, Bun.build) drains outstanding work before tearing down the workers. Previously the CLI error paths (entry-point resolution failure, link failure with --sourcemap) reached teardown with tasks still running Worker::get, causing SEGV/abort in debug builds. Production change is ~14 lines in bundle_v2.rs plus a simplification in js_bundle_completion_task.rs that removes now-redundant error-path waits; a comment update in BundleThread.rs; and three subprocess regression tests in test/bundler/cli.test.ts.

Security risks

None. The change affects internal teardown sequencing of the bundler thread pool; no user-controlled input handling, auth, crypto, or trust boundaries are touched.

Level of scrutiny

High. This is a concurrency/memory-safety fix in a critical path — bundler thread-pool teardown — where the invariant being restored ("no task references this bundle when workers are destroyed") depends on subtle reasoning about which drivers are synchronous vs asynchronous, when pending_items can be nonzero, and whether wait_for_parse() can safely tick the event loop from inside teardown. The !self.asynchronous guard deliberately excludes the dev server path on the basis that it only reaches deinit once is_done(); that reasoning is stated in the doc comment and PR description but merits a human check by someone familiar with the bake dev-server lifecycle.

Other factors

I verified: WaitGroup::wait() locks, checks raw_count > 0, and returns immediately when zero, so the two unconditional waits are safe no-ops on paths that never scheduled source-map work or already joined it. All three synchronous callers pass a non-None event_loop to BundleV2::init, so the r#loop().expect(...) inside wait_for_parse() will not panic when invoked from teardown. The refactor in init_and_run collapses the Ok/Err match into a single deinit_without_freeing_arena() call on both paths, which is behavior-preserving now that the waits live inside deinit. The new tests follow harness conventions (tempDir, bunEnv, concurrent pipe drain, exact stderr assertions, describe.concurrent) and cover all three repro shapes from the PR description. The PR also notes a user-visible side effect (CLI now reports errors from resolved entry points when a sibling fails to resolve), which is a behavior change worth a maintainer ack.

[robobun]

On the two points flagged above, for whoever takes the human look:

!asynchronous guard: the dev server is the only caller that sets asynchronous, and its only route to deinit_without_freeing_arena is finalize_bundle_cleanup (DevServer.rs), reached through finish_from_bake_dev_server, which on_after_decrement_scan_counter calls only once is_done() returned true, i.e. with pending_items == 0. So for the dev server the guard changes nothing today; it is there so a future teardown with work outstanding skips the wait instead of ticking the JS event loop from inside teardown. Tearing a dev server down mid-pass is #31702.

Event loop: every caller that can reach the wait_for_parse() branch passes one to BundleV2::init. bun build (build_command.rs) and bun test --changed (ChangedFilesFilter.rs) pass a Mini loop, Bun.build (init_and_run) passes a Mini loop, and bake production passes the VM's JS loop, which is the same loop its normal wait_for_parse() call already ticks, so the teardown path is not introducing a new way of driving it.

[robobun]

^{Updated 4:52 AM PT - Aug 11th, 2026}

✅ @robobun, your commit 7655228968385d777932929d862205f4f24f3800 passed in Build #92202! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 37480

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

bun-37480 --bun

[robobun]

I ended up at the same crash from the get_worker_slow side, so leaving the numbers here instead of opening a second PR.

Measured on main at f89d370 (debug/ASAN build):

• bun build ./missing.js ./app.js --outdir dist no longer aborts in mimalloc here (0/20 runs); that symptom stopped reproducing after ThreadPool: wait for the batch you scheduled, not for the whole pool to go idle #38604. The SEGV in Worker::deinit_soon (rdi = 0xbebebebebebebee6) still does: ./app.js ./nodir/x.js --outdir dist hit it in 48 of 60 runs, and ./a.js ./b.js ./nodir/x.js --outdir dist in 55 of 60. A single build therefore still exits cleanly on an unfixed binary about one run in five, so the single-run nodir test in this PR can pass without the fix; running the build a handful of times in a loop failed 10 out of 10 attempts for me (6 builds per attempt, first crash fails the test).
• Building the Worker before it is inserted into workers_assignments removes the SEGV by itself (0 failures in 90 runs across the three layouts): main...farm/ddf0ac49/bundler-worker-publish-initialized. It is a local change to get_worker_slow (construct with Box::new, insert under the lock, drop the new_uninit + write pair), plus the looped test in test/bundler/cli.test.ts. On its own it only turns the window into a leaked Worker until the pool is dropped, so the join added in this PR is still the fix for the teardown; the reorder makes "the map only holds written Workers" hold regardless of which driver drains. Since the description above already names the publish-before-write as the cause, it may be worth folding that branch in here.

[robobun]

#39855 carries this teardown join (the two wait groups in deinit_without_freeing_arena, the enqueue error paths draining in the drivers) together with the get_worker_slow reorder from the comment above, an IO pool count for the callback that is still inside the worker pool when the CLI frees it, and a release assert in worker_pool(). The unresolvable entry point shape itself no longer crashes on main since #39799, so #39855 keeps the --sourcemap shape from here as the test that fails on an unfixed build. This PR can be closed in favour of #39855 if that one lands.

[robobun]

Closing in favor of #39855.

#39855 carries this change: the two source map waits in deinit_without_freeing_arena, and the parse drain, which it moves into the four drivers. It also adds the IO pool drain and the get_worker_slow fix. It keeps the --sourcemap test from this PR. On a debug build of current main that shape still crashes (18 of 30 runs here). The two missing entry point shapes do not crash since #39799 (0 of 60 runs), so #39855 does not carry those tests.