From fafaab4b0776d87221781276bb4fee7d3a75c6eb Mon Sep 17 00:00:00 2001
From: robobun <117481402+robobun@users.noreply.github.com>
Date: Fri, 17 Jul 2026 04:56:10 +0000
Subject: [PATCH] destructOnExit: release nested JSLockHolder VM refs so ~VM
 runs

When process.exit() is reached from inside a JSEventListener (e.g.
worker.onmessage), JSEventListener::handleEvent's JSLockHolder holds a
RefPtr<VM> on the stack. destructOnExit only released the two creation
refs, so the VM stayed at refcount>=1, ~VM() never ran, and
Heap::lastChanceToFinalize() was skipped. Any wrapper that survived
collectNow() as conservatively reachable then leaked its native m_ctx.

timer-heap-race.test.ts was flakily hitting this on the asan lane: the
fixture calls process.exit(0) from worker.onmessage while a few
just-fired Timeout wrappers are still conservatively live on the stack.

Release every remaining VM ref. global_exit is noreturn so the stacked
holders' destructors never run, and workers were joined by
terminate_all_workers_and_wait, so no other thread uses the VM.

Co-authored-by: Ciro Spaciari <ciro.spaciari@gmail.com>
---
 src/jsc/bindings/ZigGlobalObject.cpp          | 11 ++++++--
 .../timer-heap-exit-onmessage-fixture.ts      | 25 +++++++++++++++++++
 test/js/web/timers/timer-heap-race.test.ts    | 21 ++++++++++++++++
 3 files changed, 55 insertions(+), 2 deletions(-)
 create mode 100644 test/js/web/timers/timer-heap-exit-onmessage-fixture.ts

diff --git a/src/jsc/bindings/ZigGlobalObject.cpp b/src/jsc/bindings/ZigGlobalObject.cpp
index 567404092abb..728cc0fdbe31 100644
--- a/src/jsc/bindings/ZigGlobalObject.cpp
+++ b/src/jsc/bindings/ZigGlobalObject.cpp
@@ -4132,8 +4132,15 @@ extern "C" void Zig__GlobalObject__destructOnExit(Zig::GlobalObject* globalObjec
     gcUnprotect(globalObject);
     globalObject = nullptr;
     vm.heap.collectNow(JSC::Sync, JSC::CollectionScope::Full);
-    vm.derefSuppressingSaferCPPChecking();
-    vm.derefSuppressingSaferCPPChecking();
+    // Drop every remaining VM ref so ~VM -> Heap::lastChanceToFinalize()
+    // destroys cells collectNow left conservatively reachable (whose native
+    // m_ctx would otherwise leak). Two refs are the creation pair; any beyond
+    // that are RefPtr<VM> in JSLockHolders still on the stack (e.g.
+    // JSEventListener::handleEvent when process.exit() came from
+    // worker.onmessage). global_exit is noreturn so their dtors never run,
+    // and workers were already joined, so no other thread holds the VM.
+    for (unsigned refs = vm.refCount(); refs > 0; --refs)
+        vm.derefSuppressingSaferCPPChecking();
     runLoop->threadWillExit();
 }
 
diff --git a/test/js/web/timers/timer-heap-exit-onmessage-fixture.ts b/test/js/web/timers/timer-heap-exit-onmessage-fixture.ts
new file mode 100644
index 000000000000..41353e218aa7
--- /dev/null
+++ b/test/js/web/timers/timer-heap-exit-onmessage-fixture.ts
@@ -0,0 +1,25 @@
+// process.exit() from inside worker.onmessage: JSEventListener::handleEvent
+// has a JSLockHolder (RefPtr<VM>) on the stack, so the VM refcount is one
+// higher than the two creation refs destructOnExit used to release. ~VM() and
+// therefore Heap::lastChanceToFinalize() never ran, and any wrapper collectNow
+// left conservatively reachable leaked its native m_ctx. Keep a Timeout
+// wrapper live via globalThis so conservative reachability is deterministic.
+declare var self: Worker;
+
+if (!Bun.isMainThread) {
+  self.onmessage = () => postMessage("ready");
+} else {
+  const worker = new Worker(import.meta.url);
+  worker.onerror = (e: ErrorEvent) => {
+    console.error("worker error:", e.message);
+    process.exit(1);
+  };
+  worker.onmessage = () => {
+    // Rooted via globalThis so collectNow()'s mark phase keeps the JSTimeout
+    // alive regardless of stack layout; only lastChanceToFinalize frees it.
+    (globalThis as any).__pin = setTimeout(() => {}, 1 << 30);
+    console.log("OK");
+    process.exit(0);
+  };
+  worker.postMessage({});
+}
diff --git a/test/js/web/timers/timer-heap-race.test.ts b/test/js/web/timers/timer-heap-race.test.ts
index 9afea163eab9..c3dbeb7d6687 100644
--- a/test/js/web/timers/timer-heap-race.test.ts
+++ b/test/js/web/timers/timer-heap-race.test.ts
@@ -62,3 +62,24 @@ it.skipIf(!isASAN)(
   },
   20_000,
 );
+
+it.skipIf(!isASAN)(
+  "process.exit() from worker.onmessage finalizes the JSC heap",
+  async () => {
+    // handleEvent's JSLockHolder leaves an extra RefPtr<VM> on the stack at
+    // destructOnExit, so dropping only the two creation refs left ~VM (and
+    // lastChanceToFinalize) unreached and conservatively-live m_ctx leaked.
+    const { stdout, stderr, signal, exitCode } = await runFixture("timer-heap-exit-onmessage-fixture.ts", {
+      BUN_DESTRUCT_VM_ON_EXIT: "1",
+      ASAN_OPTIONS: "allow_user_segv_handler=1:disable_coredump=0:detect_leaks=1:abort_on_error=1",
+      LSAN_OPTIONS: `malloc_context_size=30:print_suppressions=0:suppressions=${path.join(import.meta.dir, "..", "..", "..", "leaksan.supp")}`,
+    });
+    expect({ stdout, stderr, signal, exitCode }).toEqual({
+      stdout: "OK\n",
+      stderr: expect.not.stringContaining("LeakSanitizer"),
+      signal: null,
+      exitCode: 0,
+    });
+  },
+  20_000,
+);