worker: free the thread's event name table when the worker thread exits

[robobun]

Problem

• Every worker thread leaks its WebCore::EventNames table when it exits. LSan (ASAN build, Malloc=1): Direct leak of 88 byte(s) ... WebCore::EventNames::create<>() EventNames.h:62 / WebCore::eventNames() EventNames.cpp:42, plus the table's atoms as indirect leaks; about 270 bytes per worker, one report per worker that ever ran.
• Every worker allocates the table: WorkerMessagingProxy::workerGlobalScopeStarted (WorkerMessagingProxy.cpp:392) calls eventNames() on the worker thread when the worker comes online, so an empty worker leaks it too.
• Cause: the table lives in thread_local std::unique_ptr<EventNames> eventNames_ (src/jsc/bindings/webcore/EventNames.cpp:37). Bun compiles all C++ with -fno-c++-static-destructors (scripts/build/flags.ts:354), which also covers thread-storage variables, so no destructor is ever registered for it: the TLS wrapper emitted for eventNames_ is a bare address computation with no __tls_init / __cxa_thread_atexit call. When the worker thread returns, nothing frees the table. The main thread's copy is reachable through its TLS until process exit, so only workers leak.

Fix

• EventNames.cpp: add Bun__destroyEventNamesForThreadExit(), which resets the thread's eventNames_.
• web_worker.rs (WebWorker::shutdown): call it after VirtualMachine::teardown, next to Bun__freeSharedHeaderBufferForThreadExit(), which already frees the other heap-owning C++ thread_local (the http2 HPACK buffer) for exactly this reason. shutdown() is the one exit funnel for every worker: natural drain, process.exit() in the worker, terminate() from the parent, and both worker_threads and Web Worker kinds. On the early-terminate paths that never built a VM the reset is a no-op.
• Why this point is correct: the table's atoms live in the worker thread's own AtomStringTable (a Default-type JSC VM shares the thread's table, VM.cpp:264, and ~VM leaves it alone), which belongs to the WTF::Thread and is destroyed only when the thread itself exits, so releasing the atoms here removes them from the table they were registered in. No script can run at this point (teardown forbade it and the JSC VM is gone), and nothing that runs later on the thread calls eventNames() (workerGlobalScopeDestroyed only posts a task to the parent), so the table is not re-created. Nothing stores a reference to the EventNames struct; every caller reads eventNames().xEvent transiently.
• This mirrors WebCore, where the table hangs off ThreadGlobalData and the worker thread calls threadGlobalData().destroy() before it exits; Bun replaced ThreadGlobalData with the bare thread_local and dropped that step.
• Considered [[clang::always_destroy]] on the variable instead: it would also run the destructor during exit() on the main thread (bun exits through exit() on macOS and in ASAN builds) and makes the atom release depend on the platform's ordering of C++ TLS destructors versus pthread-key destructors (which is what tears down the WTF::Thread and its atom table). The explicit call runs at one known point, on worker threads only.
• Updated the comment on the HPACK sibling in c-bindings.cpp, which attributed the missing destructor to process exit rather than to the compiler flag.
• Verification: test/js/node/worker_threads/worker-shutdown-post-leak.test.ts gains four LSan cases (Malloc=1 so WTF's fastMalloc goes through the system allocator and LSan can see the table): worker_threads worker draining, calling process.exit(), terminated by the parent while listening on parentPort, and a Web Worker draining. With src/ stashed and rebuilt all four fail with the 88-byte EventNames leak above; with the fix all five tests in the file pass (bun bd test). The file's existing test gets the same explicit timeout as the new cases: each of these spawns a worker VM under debug+ASAN and then runs LSan, which already takes 4 to 7 seconds on a loaded machine against the 5 second default.
• Also ran worker_destruction, worker-transfer-terminate-stress, worker-terminate-lifetime, worker-terminate-funnels, message-port-context-destroy-leak, message-port-closed-leak, performance-observer-leak and shell-worker-terminate-leak against the fixed build; the only failures were the same five the unmodified tree produces here (listed under details).
• Out of scope, reported separately: with this fixed, a file-based (non-eval) worker_threads worker still leaks its resolved entry path string because VirtualMachine::destroy() never releases main_resolved_path. That is why the new worker_threads cases use eval: true.

Background

• eventNames(): WebCore's per-thread struct of pre-atomized event type names (closeEvent, messageEvent, ...) used by EventTarget code (MessagePort, AbortSignal, Worker, ...) to compare and dispatch event types without re-atomizing strings.
• AtomString / AtomStringTable: an AtomString is a string interned in a per-thread table so equal strings share one StringImpl and compare by pointer. The table holds raw pointers; when the last reference to an atom is dropped, the StringImpl removes itself from the current thread's table, so an atom must be released on the thread that created it while that table still exists.
• -fno-c++-static-destructors: Clang flag that suppresses exit-time destructor registration for every static-storage and thread-storage variable (the same effect as marking each one [[clang::no_destroy]]). Bun uses it because JSC assumes no destructors run at process exit; the side effect is that a thread_local that owns heap memory must be freed by hand when a thread that used it exits.
• WebWorker::shutdown() (src/jsc/web_worker.rs): the last thing a worker thread runs: exit handlers, VirtualMachine::teardown (stop phase, JSC VM destruction, loop teardown), freeing of the thread's remaining state, then workerGlobalScopeDestroyed, after which the parent joins the thread.

Probes and pre-existing failures seen while verifying

TLS wrapper Clang emits for the variable in the debug binary (no init or destructor registration call):

<TLS wrapper function for WebCore::eventNames_>:
  push %rbp
  mov  %rsp,%rbp
  mov  %fs:0x0,%rax
  lea  -0x4770(%rax),%rax
  pop  %rbp
  ret

Unfixed tree, three workers (repro from the report), BUN_DESTRUCT_VM_ON_EXIT=1 Malloc=1 with test/leaksan.supp:

Direct leak of 264 byte(s) in 3 object(s) allocated from:
    ... WebCore::EventNames::create<>() EventNames.h:62
    ... WebCore::eventNames() EventNames.cpp:42
Indirect leak of 447 byte(s) in 15 object(s) ...
SUMMARY: AddressSanitizer: 711 byte(s) leaked in 18 allocation(s).

With an empty worker the allocating frame is WorkerMessagingProxy::workerGlobalScopeStarted, called from WebWorker::spin once the entry script has run. A table first created by top-level script code is attributed to module evaluation, which leaksan.supp suppresses, which is why the new cases register listeners and exit from callbacks.

Failures that are identical with and without this change on this machine (debug ASAN build, heavily loaded host):

• worker-terminate-lifetime.test.ts "terminate() while dns.lookup() is in flight": LSan reports a 4104 byte node_fs_binding::Binding box; already tracked by node:fs: mark the per-VM Binding box as LSan-ignored (fixes worker-terminate-lifetime.test.ts on main) #35159.
• worker_destruction.test.ts: four cases (Bun.connect / Bun.listen / fetch in a terminating worker, child process with pending stdin write) hit the 5 second default timeout at 5.0 seconds; they take that long here regardless of this change.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 3 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: 0fac1636-2d20-4a07-af48-d8221573a6d1

📥 Commits

Reviewing files that changed from the base of the PR and between b7a0431 and 950daf0.

📒 Files selected for processing (4)

• src/jsc/bindings/c-bindings.cpp
• src/jsc/bindings/webcore/EventNames.cpp
• src/jsc/web_worker.rs
• test/js/node/worker_threads/worker-shutdown-post-leak.test.ts

---

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

[robobun]

Status: fix pushed; CI build 94623 ran 177 jobs green, the only non-green jobs are the two darwin 14 aarch64 - test-bun jobs, which expired without ever being assigned an agent. The debian 13 x64 asan lane, where the new LSan cases run, passed. Reviews so far have no open items; #38157 carries the same timeout hunk for the pre-existing test, so whichever of the two lands second rebases that hunk.

• Reproduced on an unmodified debug ASAN build with the report's script (BUN_DESTRUCT_VM_ON_EXIT=1 Malloc=1, detect_leaks=1, test/leaksan.supp): 3 workers gave Direct leak of 264 byte(s) in 3 object(s) from WebCore::eventNames(); an empty worker leaks it too (allocated from WorkerMessagingProxy::workerGlobalScopeStarted).
• Cause: -fno-c++-static-destructors leaves the thread_local table without a destructor, so nothing frees it when a worker thread exits. The worker now resets it at the end of WebWorker::shutdown.
• Test: the four new cases in test/js/node/worker_threads/worker-shutdown-post-leak.test.ts fail with the leak above on a build of the parent commit's src/ and pass with this one.

[robobun]

^{Updated 5:05 AM PT - Aug 13th, 2026}

@robobun, your commit 950daf0 is building: #94623

[robobun]

Heads-up on one hunk: #38157 (opened a few minutes before this PR, test-only) gives the existing test in worker-shutdown-post-leak.test.ts the same 90s timeout that this PR applies via LEAK_TEST_TIMEOUT. Whichever of the two lands second needs a trivial rebase of that hunk; if this PR lands first, #38157 gets closed.

[claude]

LGTM — mirrors the existing Bun__freeSharedHeaderBufferForThreadExit pattern one line below it.

What was reviewed:

• Placement in shutdown(): after VirtualMachine::teardown (JSC VM gone, script forbidden) and before workerGlobalScopeDestroyed; verified the latter only postTaskTos the parent and does not touch eventNames() on the worker thread.
• eventNames_.reset() is safe to call when never populated (early-terminate paths) and would re-allocate if anything did call eventNames() afterward, so no UAF window.
• LSan tests cover all four worker exit routes (drain / process.exit / parent terminate() / Web Worker) and follow the file's existing conventions.

Extended reasoning...

Overview

Adds a one-line C++ helper Bun__destroyEventNamesForThreadExit() that resets the thread_local std::unique_ptr<EventNames> eventNames_, declares it as a safe fn FFI import in web_worker.rs, and calls it in WebWorker::shutdown() immediately before the existing Bun__freeSharedHeaderBufferForThreadExit() sibling. Also corrects the comment on that sibling in c-bindings.cpp to attribute the missing destructor to -fno-c++-static-destructors (the actual cause) rather than to process exit. Four new LSan cases in worker-shutdown-post-leak.test.ts cover natural drain, process.exit(), parent terminate(), and Web Worker drain; the file's existing test gets the same explicit 90s timeout constant.

Security risks

None. This is a per-thread cleanup of an internal atom-string table on worker exit; no user-facing API surface, no untrusted input handling, no auth/crypto/permissions.

Level of scrutiny

Worker teardown is memory-safety-sensitive, so the ordering matters — but the change is a strict addition of unique_ptr::reset() on a thread_local, placed at exactly the point the sibling HPACK-buffer reset already sits. I verified the PR description's ordering claims: workerGlobalScopeDestroyed (the only thing that runs on the worker thread after this call) does nothing but ScriptExecutionContext::postTaskTo(m_loaderContextIdentifier, ...); every eventNames() call in WorkerMessagingProxy.cpp runs inside a posted task on the parent's context, not on the exiting worker thread. On the early-terminate paths where the VM was never built the unique_ptr may already be null and .reset() is a no-op. Even if something did call eventNames() after the reset, the getter re-allocates on demand (same as the HPACK sibling), so this cannot introduce a UAF — worst case a re-leak. The AtomStringTable-lifetime argument (atoms must be released on the thread that interned them, before the WTF::Thread's table is destroyed at thread exit) is sound and matches upstream WebCore's ThreadGlobalData::destroy().

Other factors

• The change is a copy of an established, already-merged pattern (Bun__freeSharedHeaderBufferForThreadExit) applied to the second heap-owning C++ thread_local.
• Tests follow harness conventions (tempDir, bunEnv spread, concurrent Promise.all pipe drain, combined-object assertion, skipIf(!isASAN || isWindows), describe.concurrent for independent subprocess cases) and were verified to fail on the unfixed tree with the exact 88-byte EventNames::create LSan report and pass with the fix.
• The 90s timeout is justified by the LSan symbolization cost against a debug binary and is applied consistently to the file's existing test.
• The rejected alternative ([[clang::always_destroy]]) is explained: it would run on the main thread at exit() and depend on platform TLS-vs-pthread-key destructor ordering.