jsc: release the cached Bun.main path in VirtualMachine::destroy()

[robobun]

Problem

• Every node:worker_threads Worker started from a file leaks one WTF string after the worker thread exits. LSan (ASAN build, Malloc=1): Direct leak of 44 byte(s) in 1 object(s) allocated from BunString__tryCreateAtom (BunString.cpp:80), called from bun_runtime::api::bun_object::get_main. The size is 20 bytes plus the length of the resolved entry path; it scales with the number of workers. eval: true workers do not hit it.
• The Bun.main getter (src/runtime/api/BunObject.rs:841-896) resolves the entry path once and caches it in vm.main_resolved_path (src/jsc/VirtualMachine.rs:159) as a +1 on a WTF string: an atom when the path is ASCII, a clone_utf8 copy otherwise.
• The only code that released it was the entry-point reload paths (reload_entry_point, reload_entry_point_for_test_runner, the test-isolation reset). VirtualMachine::destroy() (src/jsc/VirtualMachine.rs:4681) never did, and a worker's VM storage is freed with a raw dealloc (src/jsc/web_worker.rs:1034), so no field drop could do it either.
• A node-kind worker always populates the field: its bootstrap reifies process.mainModule, whose builder (constructMainModuleProperty, src/jsc/bindings/BunProcess.cpp:4416) reads Bun.main. A Web Worker or any other VM whose script reads Bun.main leaked the same way.

Fix

• VirtualMachine::destroy() derefs main_resolved_path and resets it to empty, next to the overridden_main.deinit() it already does; this is the same two-line release the three reload paths use.
• Correct place: destroy() runs on the VM's own thread (worker teardown in WebWorker::shutdown, or the main thread under BUN_DESTRUCT_VM_ON_EXIT), and get_main only ever runs on that thread, so an atom is released in the table it was registered in. The JSC VM is already gone at this point, but a default-type JSC VM uses the thread's atom table (VM.cpp:264) and ~VM leaves it alone; the table lives until the thread itself exits, after shutdown() returns.
• Safe to do unconditionally: String::deref() is a no-op on the empty tag (a VM that never read Bun.main, or whose reload path already released it), and nothing reads the field after destroy(); the only reader is get_main, and script is forbidden before teardown reaches destroy().
• bun_core::String is Copy without a Drop, so the explicit release is the codebase's idiom for this field rather than a type change.
• Verified with test/js/node/worker_threads/worker-shutdown-post-leak.test.ts: two new cases run a file-based worker_threads Worker under LSan with Malloc=1, one with an ASCII path (atom branch) and one with a non-ASCII directory in the path (clone_utf8 branch). With src/ stashed and rebuilt they fail with 67 byte(s) ... BunString__tryCreateAtom and 114 byte(s) ... BunString__fromUTF8 <- String::clone_utf8 <- get_main respectively; with the fix all three tests in the file pass under bun bd test.
• The cases tolerate one unrelated leak on top of test/leaksan.supp: the worker thread's EventNames table, which every worker leaks today and which worker: free the thread's event name table when the worker thread exits #38164 fixes. That line in the test can go once worker: free the thread's event name table when the worker thread exits #38164 lands; the two PRs are otherwise independent (that PR's worker_threads cases use eval: true precisely to avoid the leak fixed here).
• Also ran worker_threads.test.ts, web/workers/worker.test.ts and worker_destruction.test.ts against the fixed build; the only failures are timing assumptions that this slow debug+ASAN container also fails on the unmodified tree (details below).
• The file's existing test gets the same explicit timeout as the new cases: each one starts a worker VM under debug+ASAN and then runs LSan, which takes 5 to 8 seconds here against the 5 second default.

Background

• bun_core::String / BunString: the tagged string union shared with C++. When it holds a WTFStringImpl it carries a reference count; it is Copy and has no Drop, so whoever receives a +1 from a constructor such as try_create_atom or clone_utf8 must call deref() explicitly.
• Atom: a string interned in a per-thread AtomStringTable so equal strings share one StringImpl. When its last reference goes away it removes itself from the current thread's table, which is why it has to be released on the thread that created it, while that table still exists.
• VirtualMachine::destroy(): the last step of VirtualMachine::teardown, after the JSC VM and the event loops are gone. It releases the Rust-side per-VM state explicitly because the VM's memory is then freed without running field drops.
• Malloc=1: makes WebKit's allocator (bmalloc) forward to the system allocator, so WTF strings become visible to LSan. Without it this leak, and any other fastMalloc'd one, is invisible to the ASAN lanes, which is why it went unnoticed.

Repro and unrelated local failures

Repro on the unfixed tree (debug ASAN build):

mkdir -p /tmp/nodedrain && cd /tmp/nodedrain
printf 'import { Worker } from "node:worker_threads";\nnew Worker(new URL("./worker.js", import.meta.url)).on("exit", c => console.log("exit", c));\n' > main.mjs
: > worker.js
BUN_DESTRUCT_VM_ON_EXIT=1 Malloc=1 ASAN_OPTIONS=detect_leaks=1 \
  LSAN_OPTIONS=suppressions=$BUN/test/leaksan.supp $BUN/build/debug/bun-debug main.mjs

Direct leak of 44 byte(s) in 1 object(s) allocated from:
    ...
    #29 in WTF::tryMakeAtomString<WTF::String>(...)
    #30 in BunString__tryCreateAtom src/jsc/bindings/BunString.cpp:80:21

(plus the 88-byte EventNames leak from #38164). With this change the run reports only the EventNames leak; the tryCreateAtom one is gone.

A Web Worker whose script reads Bun.main leaks the same +1, but LSan attributes that string to the module loader, which interned the path first (JSC::Identifier::fromString, suppressed in leaksan.supp), so it cannot serve as a failing test; the node bootstrap reads Bun.main before the entry module is loaded, which is what makes the leak attributable.

Pre-existing failures in this container, identical with and without this change (debug+ASAN, slow host): worker_destruction.test.ts hits the 5 second default timeout on its four spawn-based cases, and web/workers/worker.test.ts "a message flood from a worker does not starve the parent's event loop" sees zero messages within its three 10 ms timer turns because a worker takes longer than that to boot here.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 29 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: 9590f3c3-abaf-4b6e-92c0-51c66201b1e7

📥 Commits

Reviewing files that changed from the base of the PR and between b7a0431 and 0eeb9af.

📒 Files selected for processing (2)

• src/jsc/VirtualMachine.rs
• test/js/node/worker_threads/worker-shutdown-post-leak.test.ts

---

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

[robobun]

Status: reproduced on main (debug ASAN build, Malloc=1): a file-based worker_threads Worker leaves a Direct leak of 44 byte(s) from BunString__tryCreateAtom via the Bun.main getter once the worker exits; an eval: true worker does not. Fix and the two LSan cases in test/js/node/worker_threads/worker-shutdown-post-leak.test.ts are in this PR; the cases fail on the unfixed tree (67 and 114 byte leaks, atom and clone_utf8 branches) and pass with it. Related: #38164 fixes the separate per-worker EventNames leak that these cases tolerate.

[github-actions]

Found 1 issue this PR may fix:

1. macOS Apple Silicon: memory invisible to RSS — bmalloc slabs, worker cleanup gaps, GC safety bugs #28318 - Its "Worker thread cleanup is incomplete" section names this defect class directly (VirtualMachine.deinit() as a stub, "freeing workers will leak the vm"); this PR releases one piece of that per-VM state (main_resolved_path) in VirtualMachine::destroy().

Note: #28318 is an umbrella issue whose main measured symptom is 100 MB-1 GB per worker of RSS-invisible bmalloc/pas slab memory, plus mi_thread_done() never being called and an empty JSC__VM__deinit. This PR addresses none of those and recovers ~20 bytes + path length per worker, so it is a partial contribution - linking it as "relates to" is more accurate than auto-closing.

If this is helpful, copy the block below into the PR description to auto-close these issues on merge.

Fixes #28318

🤖 Generated with Claude Code

[claude]

LGTM — a two-line release in VirtualMachine::destroy() that mirrors the exact main_resolved_path.deref() + reset-to-empty idiom already used at the three reload sites in the same file.

What was reviewed:

• Confirmed String::deref() is a no-op on the empty tag (src/bun_core/string/mod.rs:512), so the unconditional call is safe when Bun.main was never read or a reload already cleared it.
• Confirmed destroy() runs on the VM's own thread (worker-thread teardown), so releasing an atom here targets the correct per-thread table.
• Checked the new LSan tests: tempDir + using, concurrent pipe drain, combined {stdout, stderr, exitCode} assertion, test.concurrent for independent spawns, and both the atom and clone_utf8 branches are covered. The 90s timeout and the temporary WebCore::eventNames suppression are each justified in comments.

Extended reasoning...

Overview

The PR adds a two-line release of self.main_resolved_path (a bun_core::String holding a +1 WTF string reference) in VirtualMachine::destroy() at src/jsc/VirtualMachine.rs:4744-4745, placed immediately after the existing overridden_main.deinit(). It also adds two LSan-gated regression tests (ASCII/atom and non-ASCII/clone_utf8 paths) in test/js/node/worker_threads/worker-shutdown-post-leak.test.ts, extracts the shared leaksan.supp path into a const, and gives all three tests in the file an explicit 90-second timeout.

Security risks

None. This is a resource release on a teardown path; no user input handling, no auth/crypto/permissions surface. Worst-case failure mode of a bug here would be double-free/UAF, but the field is only ever populated once by get_main and every other site that derefs it resets to empty() afterward, so at destroy() time it is either empty (deref is a no-op — verified at src/bun_core/string/mod.rs:512, which only acts on Tag::WTFStringImpl) or holds exactly one +1. Nothing reads the field after destroy(); the VM storage is raw-dealloc'd.

Level of scrutiny

Low-to-medium. The source change is mechanical: it copies verbatim the self.main_resolved_path.deref(); self.main_resolved_path = bun_core::String::empty(); pair used at lines 2630-2631, 4806-4807, and 5069-5070 of the same file, into the one lifecycle exit that was missing it. Placement next to overridden_main.deinit() matches the reload paths' ordering. destroy() is documented and called as worker-thread teardown, satisfying the atom-string same-thread release requirement noted in the PR description and in src/CLAUDE.md.

Other factors

• No CODEOWNERS entry covers these files.
• Tests follow the repo's harness conventions to the letter: tempDir with using, await using on Bun.spawn, Promise.all drain of stdout/stderr/exited, single combined-object assertion, test.concurrent for independent subprocess cases, skipIf(!isASAN || isWindows) gating.
• The 90s timeout is justified in a comment (LSan symbolization on the debug binary), and applying it to the pre-existing test is a strict improvement over the marginal 5s default.
• The temporary leak:WebCore::eventNames suppression is scoped to a per-test suppressions file (built from the shared one), documented as covering the separate #38164 leak, and does not weaken the shared test/leaksan.supp.
• The PR description demonstrates the tests fail on the unfixed tree with the expected LSan attributions, satisfying the "prove the test fails for the right reason" requirement.

[robobun]

^{Updated 11:18 AM PT - Aug 13th, 2026}

✅ @robobun, your commit 0eeb9afb5e3e1513dfcaa25d5dc01846d9fc4c5d passed in Build #94663! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 38178

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

bun-38178 --bun