Drain VM cleanup hooks in global_exit so bun test runs napi at-exit cleanup

src/jsc/VirtualMachine.rs

@@ -209,7 +209,8 @@ pub struct VirtualMachine {
 
     pub is_printing_plugin: bool,
     pub is_shutting_down: bool,
-    /// Set once `on_exit()` has finished draining `RareData::cleanup_hooks`.
+    /// Set once `run_cleanup_hooks()` (called from `on_exit()` and
+    /// `global_exit()`) has finished draining `RareData::cleanup_hooks`.
     /// After this point the cleanup-hook list is never iterated again, so
     /// pushing to it (e.g. from a deferred N-API finalizer scheduled during
     /// the final `collectNow()` in `Zig__GlobalObject__destructOnExit`) would
@@ -372,6 +373,10 @@ unsafe extern "C" {
     safe fn Bun__closeAllSQLiteDatabasesForTermination();
     safe fn Bun__WebView__closeAllForTermination();
     safe fn Zig__GlobalObject__destructOnExit(global: &JSGlobalObject);
+    /// Makes `NapiEnv::cleanup()` run only the explicit env cleanup hooks,
+    /// skipping pending `napi_wrap` finalizers (src/jsc/bindings/napi.cpp).
+    /// Sets a process-global flag; no preconditions.
+    safe fn napi_internal_set_cleanup_hooks_only();
 }
 
 pub const HOT_RELOAD_HOT: u8 = 1;
@@ -1488,7 +1493,16 @@ impl VirtualMachine {
 
         ExitHandler::dispatch_on_exit(self);
         self.is_shutting_down = true;
+        self.run_cleanup_hooks();
+    }
 
+    /// Drain `RareData::cleanup_hooks` and set `has_run_cleanup_hooks`.
+    /// Every `NapiEnv` registers its at-exit cleanup on this list (env
+    /// cleanup hooks + pending `napi_wrap` finalizers), so it must run while
+    /// JSC and the global are still fully alive. Idempotent: the drained
+    /// list stays empty and the flag makes later `NapiFinalizerTask`s drop
+    /// instead of re-registering.
+    fn run_cleanup_hooks(&mut self) {
         // Make sure we run new cleanup hooks introduced by running cleanup
         // hooks.
         // Note: each iteration re-fetches `rare_data` so the FFI hook
@@ -1512,6 +1526,24 @@ impl VirtualMachine {
 
     pub fn global_exit(&mut self) -> ! {
         debug_assert!(self.is_shutting_down());
+        // Exit paths that never go through `on_exit()` (the test runner and
+        // its --parallel workers jump straight here) must still drain the
+        // cleanup hooks. Skipping the drain means napi env cleanup hooks
+        // never run, and it leaves `has_run_cleanup_hooks` false, so
+        // `NapiFinalizerTask::schedule()` parks every finalizer deferred by
+        // `destructOnExit`'s final collection on a list that is never walked
+        // again (an LSAN-visible leak of the task boxes). No-op when
+        // `on_exit()` already ran.
+        //
+        // Unlike `on_exit()`, these paths reach here without draining the
+        // event loop, so addons may still have queued async work; running
+        // their pending `napi_wrap` finalizers now could tear down wraps
+        // that the abandoned work still references (see `NapiEnv::cleanup`).
+        // Restrict the napi env cleanup to the explicit cleanup hooks.
+        if !self.has_run_cleanup_hooks() {
+            napi_internal_set_cleanup_hooks_only();
+        }
+        self.run_cleanup_hooks();
         // FIXME: we should be doing this, but we're not, but unfortunately
         // doing it causes like 50+ tests to break
         // self.event_loop().tick();

src/jsc/bindings/napi.cpp

@@ -1,5 +1,7 @@
 #include "BunProcess.h"
 #include "headers.h"
+
+#include <atomic>
 #include "node_api.h"
 #include "root.h"
 #include "JavaScriptCore/ConstructData.h"
@@ -3034,6 +3036,26 @@ extern "C" JS_EXPORT napi_status napi_remove_async_cleanup_hook(napi_async_clean
     NAPI_RETURN_SUCCESS(env);
 }
 
+// Whether NapiEnv::cleanup() should run only the explicit env cleanup hooks
+// and skip pending napi_wrap finalizers. Set (never cleared; the process is
+// exiting) by VirtualMachine::global_exit() on exit paths that never drained
+// the event loop. Process-global rather than per-VM: only the main thread's
+// final exit takes this path, and worker teardown goes through on_exit().
+// Atomic because worker threads can be mid-teardown (their own on_exit() ->
+// NapiEnv::cleanup()) while the main thread stores the flag; relaxed is
+// enough since either value read is safe at exit.
+static std::atomic<bool> s_napiCleanupHooksOnly { false };
+
+extern "C" void napi_internal_set_cleanup_hooks_only()
+{
+    s_napiCleanupHooksOnly.store(true, std::memory_order_relaxed);
+}
+
+extern "C" bool napi_internal_cleanup_is_hooks_only()
+{
+    return s_napiCleanupHooksOnly.load(std::memory_order_relaxed);
+}
+
 extern "C" void napi_internal_cleanup_env_cpp(napi_env env)
 {
     env->cleanup();

src/jsc/bindings/napi.h

@@ -26,6 +26,11 @@
 extern "C" void napi_internal_register_cleanup_zig(napi_env env);
 extern "C" void napi_internal_suppress_crash_on_abort_if_desired();
 extern "C" void Bun__crashHandler(const char* message, size_t message_len);
+// Set by VirtualMachine::global_exit() on exit paths that never drained the
+// event loop (the test runner and its --parallel workers); read by
+// NapiEnv::cleanup().
+extern "C" void napi_internal_set_cleanup_hooks_only();
+extern "C" bool napi_internal_cleanup_is_hooks_only();
 
 static bool equal(napi_async_cleanup_hook_handle, napi_async_cleanup_hook_handle);
 
@@ -211,6 +216,33 @@ struct NapiEnv : public WTF::RefCounted<NapiEnv> {
             drain();
         }
 
+        // When the process exits without draining the event loop (the test
+        // runner jumps straight to global_exit()), pending napi_wrap
+        // finalizers are not safe to run: an addon may have queued async
+        // work whose teardown touches other wraps this loop would already
+        // have deleted (e.g. duckdb's Task destructor Unref()s the
+        // Connection it captured). Run only the explicit env cleanup hooks
+        // above, matching Node, which does not guarantee wrap finalizers run
+        // at process exit. Wraps swept by a later collection (the exit GC
+        // under BUN_DESTRUCT_VM_ON_EXIT) still defer their finalizers, and
+        // NapiFinalizerTask::schedule() drops those tasks because the VM
+        // cleanup hooks already ran, so only the addon's own allocations are
+        // left for the OS to reclaim.
+        if (napi_internal_cleanup_is_hooks_only()) {
+            // The napi_set_instance_data() finalizer is part of env teardown
+            // in Node and is still safe here: nothing on this path has been
+            // freed, so it cannot observe the dangling state the skipped
+            // wrap-finalizer pass avoids. Null the pointer so any finalizer
+            // that still runs later (lastChanceToFinalize calls them
+            // immediately) sees null from napi_get_instance_data instead of
+            // freed memory.
+            instanceDataFinalizer.call(this, instanceData, true);
+            instanceDataFinalizer.clear();
+            instanceData = nullptr;
+            clearExceptionsBetweenFinalizers();
+            return;
+        }
+
         // Defer GC during entire finalizer cleanup to prevent iterator invalidation.
         // This prevents any GC-triggered finalizer execution while m_finalizers is being iterated.
         JSC::DeferGCForAWhile deferGC(m_vm);
@@ -239,6 +271,7 @@ struct NapiEnv : public WTF::RefCounted<NapiEnv> {
 
         instanceDataFinalizer.call(this, instanceData, true);
         instanceDataFinalizer.clear();
+        instanceData = nullptr;
         clearExceptionsBetweenFinalizers();
     }
 

src/runtime/napi/napi_body.rs

@@ -4342,9 +4342,9 @@ impl NapiFinalizerTask {
 
         if vm.is_shutting_down() {
             if vm.has_run_cleanup_hooks() {
-                // `on_exit()` already drained cleanup hooks; we are inside the
-                // final `collectNow()` (Heap::sweepArrayBuffers) and the JSC
-                // VM is being torn down. The cleanup-hook list will never be
+                // `run_cleanup_hooks()` already drained cleanup hooks; we are
+                // inside the final `collectNow()` (Heap::sweepArrayBuffers)
+                // and the JSC VM is being torn down. The cleanup-hook list will never be
                 // walked again, and running the user finalizer here (mid-GC,
                 // with the global about to be freed) is unsafe. Drop the task
                 // so the `Box<NapiFinalizerTask>` and its `NapiEnvRef` are

test/napi/napi-exit-cleanup.test.ts

@@ -0,0 +1,130 @@
+// https://github.com/oven-sh/bun/issues/32144
+//
+// Every `bun test` exit site (and the --parallel worker exit in
+// test/parallel/runner.rs) jumped straight to VirtualMachine::global_exit()
+// without draining RareData::cleanup_hooks, the list on which each NapiEnv
+// registers its at-exit cleanup. Consequences:
+//   - napi_add_env_cleanup_hook hooks silently never ran under `bun test`
+//     (Node runs them whenever the environment tears down);
+//   - with BUN_DESTRUCT_VM_ON_EXIT=1 the final GC deferred the swept wraps'
+//     finalizers as NapiFinalizerTasks parked on that same never-walked
+//     list, which LeakSanitizer reports at exit (the intermittent exit-134
+//     failures of test/regression/issue/30205.test.ts on the asan CI lane).
+//
+// The fix drains the cleanup hooks in global_exit(). Pending napi_wrap
+// finalizers are deliberately NOT run on this path: unlike `bun run`, the
+// test runner exits without draining the event loop, so addons may still
+// have queued async work whose teardown references wraps the finalizer pass
+// would have deleted (duckdb's Task destructor Unref()s its Connection).
+// Node does not guarantee wrap finalizers run at process exit either.
+//
+// This file lives apart from napi.test.ts only because that 100-test
+// concurrent suite is too heavy to run as a whole alongside these.
+
+import { spawn, spawnSync } from "bun";
+import { beforeAll, describe, expect, test } from "bun:test";
+import { bunEnv, bunExe, canBuildNodeAddons, tempDir } from "harness";
+import { existsSync } from "node:fs";
+import { join } from "node:path";
+
+const napiAppDir = join(import.meta.dir, "napi-app");
+const hookAddon = join(napiAppDir, "build", "Debug", "test_cleanup_hook_order.node");
+const wrapAddon = join(napiAppDir, "build", "Debug", "test_wrap_cleanup_order.node");
+
+// CI's ASAN lane sets BUN_JSC_validateExceptionChecks=1, which leaks into the
+// spawned bun processes via bunEnv. The napi addon init path has a known
+// unchecked ThrowScope (see the "3rd party napi" section in
+// test/no-validate-exceptions.txt), so the child aborts while loading the
+// addon, before the fixture runs. Strip the validator from the child env
+// only, same as test/regression/issue/30205.test.ts.
+//
+// BUN_DESTRUCT_VM_ON_EXIT is stripped too: these tests pin the normal exit
+// path, where pending wrap finalizers are skipped. Under destruct-vm the
+// exit collection defers swept wraps' finalizers (and schedule() drops the
+// tasks), but wraps that survive the collection can still be finalized
+// immediately by ~VM's lastChanceToFinalize, which would print
+// "finalize order:" and break the negative assertion below. This file is
+// also in test/no-validate-leaksan.txt so the asan runner does not set
+// destruct-vm or detect_leaks for it in the first place.
+const childEnv = {
+  ...bunEnv,
+  BUN_JSC_validateExceptionChecks: undefined,
+  BUN_JSC_dumpSimulatedThrows: undefined,
+  BUN_DESTRUCT_VM_ON_EXIT: undefined,
+  // detect_leaks aborts debug builds on known-benign exit allocations and is
+  // not what these tests measure; drop back to bun's baked ASAN defaults.
+  ASAN_OPTIONS: undefined,
+  LSAN_OPTIONS: undefined,
+};
+
+describe.concurrent.skipIf(!canBuildNodeAddons())("napi cleanup at bun test exit", () => {
+  beforeAll(() => {
+    if (existsSync(hookAddon) && existsSync(wrapAddon)) return;
+    // Same one-shot build pattern as test/regression/issue/30205.test.ts.
+    const install = spawnSync({
+      cmd: [bunExe(), "install", "--verbose"],
+      cwd: napiAppDir,
+      env: bunEnv,
+      stderr: "inherit",
+      stdout: "inherit",
+      stdin: "inherit",
+    });
+    if (!install.success) throw new Error("node-gyp build failed");
+  }, 120_000);
+
+  test("napi env cleanup hooks run when the process exits from bun test", async () => {
+    using dir = tempDir("napi-cleanup-hooks-bun-test", {
+      "hooks.test.js": `
+        import { test, expect } from "bun:test";
+        const addon = require(${JSON.stringify(hookAddon)});
+        addon.test();
+        test("registers env cleanup hooks", () => expect(1).toBe(1));
+      `,
+    });
+    await using proc = spawn({
+      cmd: [bunExe(), "test", "hooks.test.js"],
+      env: childEnv,
+      cwd: String(dir),
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    // The addon registers hooks 1, 2, 3; they must fire at exit in reverse
+    // order, same as when the process exits from `bun run`.
+    expect(stdout).toContain("hook3 executed at position 0");
+    expect(stdout).toContain("hook2 executed at position 1");
+    expect(stdout).toContain("hook1 executed at position 2");
+    expect(stderr).toContain("1 pass");
+    expect(exitCode).toBe(0);
+  });
+
+  test("pending napi_wrap finalizers are skipped at bun test exit", async () => {
+    // Negative contract for the hooks-only drain: wraps still rooted when
+    // `bun test` exits must NOT have their finalizers run (the event loop
+    // was never drained, so running them can touch wraps that abandoned
+    // async work still references), and the process must exit cleanly. The
+    // equivalent `bun run` exit does run them; that behavior is covered by
+    // "napi_wrap finalizers run in LIFO order during env teardown" in
+    // napi.test.ts.
+    using dir = tempDir("napi-wrap-teardown-bun-test", {
+      "wrap.test.js": `
+        import { test, expect } from "bun:test";
+        const addon = require(${JSON.stringify(wrapAddon)});
+        globalThis.keep = addon.createParentAndChildren(32);
+        test("wraps stay rooted until exit", () => expect(globalThis.keep.length).toBe(33));
+      `,
+    });
+    await using proc = spawn({
+      cmd: [bunExe(), "test", "wrap.test.js"],
+      env: childEnv,
+      cwd: String(dir),
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    // The addon prints "finalize order: ..." if any wrap finalizer runs.
+    expect(stdout).not.toContain("finalize order:");
+    expect(stderr).toContain("1 pass");
+    expect(exitCode).toBe(0);
+  });
+});

test/no-validate-exceptions.txt

@@ -73,6 +73,7 @@ test/js/third_party/prisma/prisma.test.ts
 test/js/third_party/remix/remix.test.ts
 test/js/third_party/resvg/bbox.test.js
 test/js/third_party/rollup-v4/rollup-v4.test.ts
+test/napi/napi-exit-cleanup.test.ts
 test/napi/napi-finalizer-delete-ref.test.ts
 test/napi/uv.test.ts
 test/napi/uv_stub.test.ts

test/no-validate-leaksan.txt

@@ -408,6 +408,19 @@ test/napi/node-napi-tests/test/js-native-api/6_object_wrap/do.test.ts
 # LSAN per-spawn adds no coverage and slows the file significantly.
 test/napi/uv_stub.test.ts
 
+# Spawns bun test children that pin the non-destruct exit path; destruct-vm
+# and detect_leaks would leak into them via bunEnv and change the behavior
+# under test (see the childEnv comment in the file).
+test/napi/napi-exit-cleanup.test.ts
+
+# The inner `bun test --isolate/--parallel` processes inherit destruct-vm and
+# detect_leaks via bunEnv. Their last file's napi wraps are deliberately not
+# finalized at exit (NapiEnv::cleanup runs hooks only on the test-runner exit
+# path), so the addon's own 4-byte wrap payloads are reclaimed by the OS, not
+# freed, and LSAN reports them. The NapiFinalizerTask boxes from issue 32144
+# no longer leak: schedule() drops them once the cleanup hooks have run.
+test/regression/issue/30205.test.ts
+
 test/bake/dev/production.test.ts
 test/js/third_party/pg-gateway/pglite.test.ts
 test/js/bun/test/parallel/test-integration-rspack.ts