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

[robobun]

Fixes #32144

Problem

On the debian x64-asan CI lane, test/regression/issue/30205.test.ts intermittently fails with exit 134: the inner bun test --isolate process trips LeakSanitizer at exit:

==5438==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 31904 byte(s) in 997 object(s) allocated from:
    #8 new<bun_runtime::napi::napi_body::NapiFinalizerTask>
    ...
    #24 Zig__GlobalObject__destructOnExit src/jsc/bindings/ZigGlobalObject.cpp:3966
    #25 VirtualMachine::global_exit       src/jsc/VirtualMachine.rs:1581

Cause

Every bun test exit site (4 in test_command.rs, plus the --parallel worker exit in test/parallel/runner.rs) sets vm.is_shutting_down = true by hand and jumps straight to global_exit(). None of them calls on_exit(), which was the only place that drained RareData::cleanup_hooks and set has_run_cleanup_hooks. Each NapiEnv registers its at-exit cleanup on that list. So under bun test:

• napi_add_env_cleanup_hook hooks silently never ran (Node runs them whenever the environment tears down, regardless of entry point)
• with BUN_DESTRUCT_VM_ON_EXIT=1 + detect_leaks=1 (the asan lane), the final collectNow() in destructOnExit deferred each swept wrap's finalizer as a NapiFinalizerTask, and NapiFinalizerTask::schedule() parked all of them back on the same cleanup-hook list, which is never walked again. 997 task boxes (32 bytes each, matching the report) plus their NapiEnvRefs were live when LSAN scanned.

Fix

1. global_exit() now drains the cleanup hooks (the loop extracted from on_exit() into run_cleanup_hooks()), so napi env cleanup hooks fire on the test-runner exit paths and has_run_cleanup_hooks is set before destructOnExit. Exit paths that go through on_exit() (bun run, process.exit(), repl, workers) are unchanged; their second drain sees an empty list.

2. On these undrained-loop exit paths, NapiEnv::cleanup() runs only the explicit env cleanup hooks and skips the pending napi_wrap finalizer pass (napi_internal_set_cleanup_hooks_only, set by global_exit() before its drain). Running that pass proved unsafe when the event loop never drained: an addon can still have queued async work whose teardown references other wraps. Concretely, duckdb keeps a ConnectTask queued on its Database; the task's destructor Unref()s the Connection ObjectWrap that the finalizer pass had already deleted, and test/js/third_party/duckdb/duckdb-basic-usage.test.ts crashed at exit with SEGV ... in NapiEnv::vm() reading the freed wrap's env pointer (reproduced 6/8 runs locally, 0/10 after this change). Node does not guarantee wrap finalizers run at process exit either. bun run exits keep the full cleanup: pending async work holds the loop alive there, so the queue is empty by the time on_exit() runs.

3. With has_run_cleanup_hooks set by the drain, NapiFinalizerTasks deferred by destructOnExit's final collection under BUN_DESTRUCT_VM_ON_EXIT take the existing drop branch in schedule() instead of being parked on the dead list: the task boxes and their NapiEnvRefs (the leak in the report) are freed. The addon's own wrap payloads are deliberately not freed (their finalizers are skipped, matching the hooks-only rule above), so test/regression/issue/30205.test.ts is added to test/no-validate-leaksan.txt next to the other tests whose intentional at-exit non-cleanup LSAN would report. An earlier revision instead requested termination before the exit collection so the finalizers ran immediately mid-sweep; that trips JSC's validateIsNotSweeping assertion for addons whose finalizers call JS-heap-touching napi functions (test_cannot_run_js on the asan lane), which is exactly what deferral exists for, so it was reverted.

The load-bearing lines are the run_cleanup_hooks() call plus the hooks-only flag in global_exit(), and the napi_internal_cleanup_is_hooks_only() early return in NapiEnv::cleanup().

Verification

New test/napi/napi-exit-cleanup.test.ts (deterministic on every build; the hooks test fails on unfixed bun including the current release):

• napi env cleanup hooks run when the process exits from bun test (existing test_cleanup_hook_order addon; on unfixed builds the hook lines never appear)
• pending napi_wrap finalizers are skipped at bun test exit (negative contract for the hooks-only drain; the bun run finalizer behavior is covered by the existing LIFO test in napi.test.ts)

Verified locally on a debug-asan build:

• the exact CI scenario from the issue (8 isolate files x 1000 addon.wrap(...), BUN_JSC_collectContinuously=1, BUN_DESTRUCT_VM_ON_EXIT=1, detect_leaks=1:abort_on_error=1) exits 0 with no leak report and zero NapiFinalizerTask frames on a debug-asan build
• test/js/third_party/duckdb/duckdb-basic-usage.test.ts (duckdb built from source) passes 10/10; before the hooks-only restriction it crashed at exit most runs
• test/regression/issue/30205.test.ts (4 pass), test/napi/napi.test.ts -t "LIFO order during env teardown" (bun run wrap finalizers unchanged), test/cli/test/bun-test.test.ts (73 pass)

The leaking branch itself only fires on release-asan CI timing (conservative stack scanning keeps the wraps alive through the final collection on local debug builds), so the asan lane on this PR is the direct check for the original report.

The test file is added to test/no-validate-exceptions.txt and test/no-validate-leaksan.txt, and strips BUN_JSC_validateExceptionChecks, BUN_DESTRUCT_VM_ON_EXIT, and the sanitizer options from its child env: all of these leak into spawned bun processes via bunEnv on the asan lane, and they either abort the child while loading the addon (exception validator) or deliberately change the exit behavior under test (destruct-vm runs the skipped finalizers in the exit collection).

[robobun]

^{Updated 8:26 AM PT - Jul 8th, 2026}

❌ @robobun, your commit 3e68a1f has 2 failures in Build #70473 (All Failures):

• test/integration/next-pages/test/dev-server.test.ts - code 1 on 🍎 14 aarch64
• test/js/sql/postgres-binary-array-bounds.test.ts - code 1 on 🪟 2019 x64
• test/js/sql/postgres-binary-array-bounds.test.ts - code 1 on 🪟 2019 x64-baseline

---

🧪   To try this PR locally:

bunx bun-pr 32146

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

bun-32146 --bun

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Extracts cleanup-hook draining into a private VirtualMachine::run_cleanup_hooks(), calls it from both on_exit and global_exit (setting a “cleanup hooks only” FFI flag when needed), requests termination earlier in ZigGlobalObject destruction, adds N-API binding guards for hooks-only mode, and introduces spawned regression tests validating exit cleanup behavior.

Changes

N-API cleanup hooks on all exit paths

Layer / File(s)	Summary
VirtualMachine cleanup hook extraction and global exit integration src/jsc/VirtualMachine.rs	Introduce private run_cleanup_hooks(); update has_run_cleanup_hooks doc; add napi_internal_set_cleanup_hooks_only() FFI declaration; call helper from on_exit() and global_exit() so cleanup hooks are drained idempotently on all exit paths.
N-API bindings: 'hooks-only' flag and NapiEnv guard src/jsc/bindings/napi.cpp, src/jsc/bindings/napi.h	Add s_napiCleanupHooksOnly flag with napi_internal_set_cleanup_hooks_only() and napi_internal_cleanup_is_hooks_only() externs; make NapiEnv::cleanup() early-return when hooks-only mode is enabled to skip pending wrap finalizers on non-loop exits.
Zig global teardown requests termination before final GC src/jsc/bindings/ZigGlobalObject.cpp	Call VM termination/request APIs immediately before gcUnprotect() and collectNow() so N-API wrapper finalizers run during that final collection instead of being deferred.
Runtime comment update in NapiFinalizerTask::schedule src/runtime/napi/napi_body.rs	Update shutdown-path comment to reference run_cleanup_hooks() and the final collectNow() VM teardown context; no logic change.
Exit-path regression tests test/napi/napi-exit-cleanup.test.ts, test/no-validate-exceptions.txt	Add concurrent tests that conditionally build native addons, spawn isolated bun test runs to assert env cleanup hook reverse order and wrap-finalizer skip behavior on process exit; add test to ASAN no-validate-exceptions list.

• Suggested reviewers:
  • Jarred-Sumner
  • dylan-conway

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main objective: draining VM cleanup hooks in global_exit to enable napi at-exit cleanup execution during bun test teardown.
Linked Issues check	✅ Passed	The PR fully addresses issue #32144 by extracting cleanup hook drainage into run_cleanup_hooks(), implementing hooks-only mode in NapiEnv::cleanup() to prevent unsafe finalizer runs, and requesting VM termination in destructOnExit to ensure finalizers run immediately.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the cleanup hook drainage issue: VM cleanup refactoring, N-API cleanup mode flags, GC finalization timing, test file for verification, and exclusion list update.
Description check	✅ Passed	The PR description comprehensively covers the problem, root cause analysis, implemented fix, and verification steps with both deterministic tests and local debug-asan validation.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/napi/napi-exit-cleanup.test.ts`:
- Around line 48-55: Replace uses of tempDirWithFiles(...) with the disposable
tempDir(...) fixture so the temp directory is cleaned up if the spawned-process
test fails; create the temp dir via tempDir("napi-cleanup-hooks-bun-test") (or
similar), write the "hooks.test.js" file into the returned fixture directory
(using the fixture's write or path helpers) and pass the fixture.path (or
constructed file path) to the spawned process; update both occurrences that use
tempDirWithFiles (the one creating "hooks.test.js" around lines ~48-55 and the
other at ~74-81) to use the tempDir fixture API instead.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 650a5754-23fc-44b6-b4f1-d89fa5e6a192

📥 Commits

Reviewing files that changed from the base of the PR and between ddd1ff0 and a6f77bb.

📒 Files selected for processing (2)

• src/jsc/VirtualMachine.rs
• test/napi/napi-exit-cleanup.test.ts

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/jsc/bindings/napi.h`:
- Around line 206-217: The early return in NapiEnv::cleanup() when
napi_internal_cleanup_is_hooks_only() is enabled skips calling the
napi_set_instance_data() finalizer (instanceDataFinalizer.call(this,
instanceData, true)), which is part of env teardown; adjust cleanup() so that
even in hooks-only mode the instanceData finalizer is invoked before returning
(either move the instanceDataFinalizer.call(...) above the
napi_internal_cleanup_is_hooks_only() check or call it conditionally just before
the return), ensuring napi_set_instance_data() teardown runs while still
skipping other finalizer work.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 38e28cda-2d5b-4fae-b6e9-6037de9e5f6d

📥 Commits

Reviewing files that changed from the base of the PR and between 73f4ca4 and afac911.

📒 Files selected for processing (7)

• src/jsc/VirtualMachine.rs
• src/jsc/bindings/ZigGlobalObject.cpp
• src/jsc/bindings/napi.cpp
• src/jsc/bindings/napi.h
• src/runtime/napi/napi_body.rs
• test/napi/napi-exit-cleanup.test.ts
• test/no-validate-exceptions.txt

[claude]

Additional findings (outside current diff — PR may have been updated during review):

• 🔴 src/jsc/bindings/ZigGlobalObject.cpp:3964-3976 — Setting vm.setHasTerminationRequest() before collectNow() makes NapiEnv::mustDeferFinalizers() return false for non-experimental napi modules too, so swept-wrap / napi_add_finalizer callbacks now run inline mid-GC — a brand-new execution context that checkGC() only guards for experimental modules. This is the cause of the test_cannot_run_js/do.test.ts failure on x64-asan in the build report for 7e8a7b3 (the only lane with BUN_DESTRUCT_VM_ON_EXIT=1); 58c3373 doesn't touch this path. Either add the test to test/no-validate-leaksan.txt, move the termination request to after collectNow() (relying on ~VM's lastChanceToFinalize, which the comment already cites), or harden the inline-mid-GC path for non-experimental finalizers.

  Extended reasoning...

  What the bug is

  This PR adds vm.setHasTerminationRequest() immediately before gcUnprotect(globalObject) + collectNow() in Zig__GlobalObject__destructOnExit, so that NapiEnv::mustDeferFinalizers() (nm_version != NAPI_VERSION_EXPERIMENTAL && !isVMTerminating()) returns false during the final collection and swept wraps run their finalizers as raw callbacks instead of deferring NapiFinalizerTasks. That is the intended fix for the LSAN leak, but it also opens a new execution path for non-experimental napi modules: their napi_wrap / napi_add_finalizer callbacks now run inline while heap.collectNow() is on the stack, with the global already gcUnprotect'd and termination requested.

  Before this PR, only NAPI_VERSION_EXPERIMENTAL modules ever had mustDeferFinalizers()==false mid-GC, and they are guarded by checkGC() (napi.h:350-360), which aborts if a finalizer calls a JS-affecting napi function. Non-experimental modules have no such guard — checkGC() is a no-op for them — and their finalizers routinely call napi_get_global, napi_get_named_property, etc. that allocate, append to handle scopes on the now-unprotected global, and do property lookups while isCollectorBusyOnCurrentThread() is true.

  Concrete CI evidence

  The robobun build report for commit 7e8a7b3 lists test/napi/node-napi-tests/test/js-native-api/test_cannot_run_js/do.test.ts failing with code 1 on 🐧 13 x64-asan — the only lane that sets BUN_DESTRUCT_VM_ON_EXIT=1. The other failures in that report (3× musl LTO link errors, macOS init.test.ts) are unrelated infrastructure. The latest commit 58c3373 only adds instanceData = nullptr and the safe fn qualifier; neither touches this path, so the failure persists.

  The code path that triggers it

  1. Env propagation. test_cannot_run_js/do.test.ts is not in test/no-validate-leaksan.txt (only test_general and 6_object_wrap napi entries are), so scripts/runner.node.mjs sets BUN_DESTRUCT_VM_ON_EXIT=1 for it on the asan lane. The harness run() (test/napi/node-napi-tests/harness.ts:38-52) spawns bun run test.js with bunEnv, which spreads process.env — so the child inherits BUN_DESTRUCT_VM_ON_EXIT=1.
  2. Finalizer registration. test.js loads test_pending_exception (binding.gyp: NAPI_VERSION=8, non-experimental) and calls addon.createRef(mustNotCall()). createRef calls napi_add_finalizer(env, cb, ref, BasicFinalize, NULL, ref) with a non-null result pointer (test_cannot_run_js.c:59), which at napi.cpp:1160-1166 creates a NapiRef(refCount=0) with a weak handle via NapiRefWeakHandleOwner — not added to m_finalizers. So NapiEnv::cleanup()'s m_finalizers loop does not run it; it only fires when GC sweeps the mustNotCall() function.
  3. Exit path. On bun run exit, on_exit() drains cleanup hooks (has_run_cleanup_hooks=true), then global_exit() → destructOnExit. Because on_exit() already ran, the new hooks-only flag in global_exit() is not set (gated by !has_run_cleanup_hooks()). The only PR change affecting this child is the setHasTerminationRequest() before collectNow().
  4. Before this PR: during collectNow(), mustDeferFinalizers() = (8 != EXPERIMENTAL && !false) = true, so NapiRef::callFinalizer() passes immediate = !true || !true = false → napi_internal_enqueue_finalizer → NapiFinalizerTask::schedule() sees has_run_cleanup_hooks=true → drops. Finalize() never ran on this path.
  5. After this PR: isVMTerminating() is true → mustDeferFinalizers() = false → immediate = !false || … = true → NapiFinalizer::call invokes BasicFinalize inline (napi_finalizer.cpp:12-13). For the non-experimental build, BasicFinalize falls into the #else branch (test_cannot_run_js.c:44) and calls Finalize() directly — which calls napi_delete_reference, then napi_get_global (full NAPI_PREAMBLE with throw scope; toNapi() appends to globalObject->m_currentNapiHandleScopeImpl on the just-gcUnprotect'd global), then napi_get_named_property (allocates an Identifier, does getPropertySlot on the global) — all while isCollectorBusyOnCurrentThread() is true and termination has been requested.

  Why existing code doesn't prevent it

  checkGC() only enforces the "may not affect GC state from a finalizer" rule for NAPI_VERSION_EXPERIMENTAL modules (napi.h:350: if (m_napiModule.nm_version == NAPI_VERSION_EXPERIMENTAL)). Non-experimental finalizers were never expected to run mid-GC because mustDeferFinalizers() was always true for them outside ~VM's lastChanceToFinalize(). The PR comment says "same as during lastChanceToFinalize in ~VM", but lastChanceToFinalize runs after the heap stops collecting; collectNow(Sync, Full) is still mid-sweep when these callbacks fire. The test name is literally test_cannot_run_js — it exists to exercise exactly this scenario.

  Step-by-step proof

  1. asan runner: BUN_DESTRUCT_VM_ON_EXIT=1 set for outer do.test.ts (not in no-validate-leaksan.txt).
  2. run() spawns bun run test_cannot_run_js/test.js with bunEnv → child has BUN_DESTRUCT_VM_ON_EXIT=1.
  3. Child: createRef(mustNotCall()) for test_pending_exception (NAPI_VERSION=8) → NapiRef(refCount=0) with weak handle on the function.
  4. Child exits → on_exit() drains hooks → global_exit() → destructOnExit.
  5. vm.setHasTerminationRequest() (new) → gcUnprotect(globalObject) → collectNow(Sync, Full).
  6. mustNotCall() function swept → NapiRefWeakHandleOwner::finalize → callFinalizer() → mustDeferFinalizers()==false → BasicFinalize inline → #else → Finalize().
  7. Finalize() calls napi_get_global / napi_get_named_property mid-sweep on the unprotected global with termination requested → child exits non-zero → outer test's expect(result.exitCode).toBe(0) fails → code 1 on x64-asan, exactly as the build report shows.

  Impact

  Merge-blocking CI regression on the lane this PR is specifically fixing (x64-asan with BUN_DESTRUCT_VM_ON_EXIT=1), caused directly by a change in this PR. More broadly, any non-experimental addon whose napi_add_finalizer / weak-ref finalizer touches the JS heap is now exposed to running mid-GC under BUN_DESTRUCT_VM_ON_EXIT.

  How to fix

  Three options, roughly in order of effort:

  • Opt the test out: add test/napi/node-napi-tests/test/js-native-api/test_cannot_run_js/do.test.ts to test/no-validate-leaksan.txt so the asan runner doesn't set BUN_DESTRUCT_VM_ON_EXIT for it (same treatment as test_general / 6_object_wrap two lines above). This is the minimal unblock if the inline-mid-GC behavior is acceptable for the leak-detection use case.
  • Move the termination request after collectNow(): the PR comment already notes the equivalent path is ~VM's lastChanceToFinalize(), which runs after the explicit collection. Setting termination after collectNow() keeps non-experimental finalizers deferred (and dropped, since has_run_cleanup_hooks is true) during the explicit sweep, then ~VM finalizes whatever survives with termination set — same as today's behavior plus the LSAN fix.
  • Harden the inline path: make non-experimental finalizers running inline under termination tolerate the gcUnprotect'd global / mid-GC state (e.g. have napi_get_named_property etc. early-return napi_pending_exception when isVMTerminating() && inGC()). Most invasive, but matches what the test is actually asserting.

[robobun]

CI status on 58ce53c: all 20 debian x64-asan test shards pass, including test/regression/issue/30205.test.ts (the failure from the issue), test/napi/napi-exit-cleanup.test.ts, test_cannot_run_js, and the duckdb test that earlier revisions broke.

The one failed job is Windows 11 aarch64, in test/bake/dev/production.test.ts's harness setup (ensureReactCache/copyCachedReactDeps), before any bake logic runs. The hot.test.ts (Windows 2019) and fetch-abort-slow-connect (macOS 26) annotations passed on retry. None of these touch the napi/exit paths this PR changes.

[robobun]

Re the termination-request finding: that was exactly the cause of the test_cannot_run_js failure on the asan lane and was already reverted in 58ce53c (the review ran against 7e8a7b3). The current design defers the swept finalizers as before and drops the tasks in NapiFinalizerTask::schedule() once the cleanup hooks have run; all 20 x64-asan shards including test_cannot_run_js pass on the current head.

[claude]

All my earlier findings are addressed and the bug hunter found nothing new on f1d2264, but this changes VM exit sequencing and N-API finalizer-ordering semantics (with one approach already tried and reverted mid-PR), so it's worth a maintainer's look before merge.

Extended reasoning...

Overview

This PR makes global_exit() drain the VM cleanup-hook list (previously only on_exit() did), so napi env cleanup hooks fire when bun test exits. It also introduces a process-global "hooks-only" flag that makes NapiEnv::cleanup() skip the pending napi_wrap finalizer pass on exit paths that never drained the event loop, runs the napi_set_instance_data finalizer in that mode, and adds the originating regression test plus a new deterministic test to the leaksan/exception-validator opt-out lists. Files touched: src/jsc/VirtualMachine.rs, src/jsc/bindings/napi.{cpp,h}, src/runtime/napi/napi_body.rs (comment only), a new test file, and two CI opt-out lists.

Security risks

None identified. No auth, crypto, network, or untrusted-input surface is touched; the change is confined to process-exit teardown ordering.

Level of scrutiny

High. This is not a mechanical change:

• It alters VM shutdown sequencing on every bun test exit and introduces a user-observable behavior change (napi env cleanup hooks now run under bun test; wrap finalizers are explicitly skipped on that path).
• One design (setHasTerminationRequest() before the exit GC) was tried and reverted mid-PR after breaking test_cannot_run_js on the asan lane.
• Review surfaced several real issues that were fixed in follow-up commits: a finalizer-ordering inversion that could UAF instanceData under BUN_DESTRUCT_VM_ON_EXIT, a non-atomic process-global read by worker threads, an asan-lane env leak that would fail the new test, and a Windows CRLF assertion bug. All are addressed, but the volume of subtle follow-ups confirms this area is fragile.
• The hooks-only flag is process-global rather than per-VM, which the author justified but is a design call worth a maintainer's sign-off.

Other factors

All inline comments from me and CodeRabbit are resolved; the last commit (f1d2264) is a comment-only reword. CI on 58ce53c shows all 20 x64-asan shards green including the originating regression test, the duckdb test, and test_cannot_run_js; the remaining annotations (fetch-leak on macOS 26, bake production.test.ts harness on Windows aarch64, fetch-abort-slow-connect) are unrelated to the napi/exit paths. No human reviewer has looked at this yet; CodeRabbit suggested Jarred-Sumner / dylan-conway.

[robobun]

Heads up: #32180 (for #32176, the Expect/RefData half of the same asan lane flake) also touches the cleanup-hook bookkeeping. It runs a final ASAN-only collection on the test exit paths before global_exit and sets has_run_cleanup_hooks so finalizers deferred by that sweep take the drop branch in NapiFinalizerTask::schedule. If this PR lands first, #32180 rebases to call napi_internal_set_cleanup_hooks_only() + run_cleanup_hooks() there instead, so the hooks-only restriction and the env cleanup hooks still apply. If #32180 lands first, the !has_run_cleanup_hooks() guard here would skip napi_internal_set_cleanup_hooks_only() on asan test exits (the flag is already set by then), so the condition needs a different signal for "on_exit never ran".

[robobun]

Rebased onto main to resolve the conflict in src/jsc/bindings/napi.h. The conflict was in NapiEnv::cleanup(): main added clearExceptionsBetweenFinalizers() after the instance-data finalizer (#30286) and this PR added instanceData = nullptr in the same spot. Resolved by keeping both (null the pointer, then clear exceptions). Rebuilt and reran: napi-exit-cleanup.test.ts, test/regression/issue/30205.test.ts, and test_cannot_run_js all pass.

[robobun]

Post-rebase CI (build 70466): the x64-asan lane that exercises this change is green (18 shards passed, the rest still running, zero failures), including test/regression/issue/30205.test.ts. The red jobs are unrelated platform flakes on lanes this diff doesn't touch: update_interactive_install (aarch64 install), postgres-binary-array-bounds (Windows sql), zlib/leak (Windows RSS-threshold). Diff is ready for review.

[claude]

All my prior feedback is addressed and I found nothing new on 3e68a1f, but this touches VM teardown ordering and N-API finalizer lifecycle (with a design decision to skip wrap finalizers on the test-runner exit path, plus the noted interaction with #32180), so it warrants a maintainer sign-off.

Extended reasoning...

Overview

This PR changes process-exit behavior for bun test: it extracts the cleanup-hook drain loop from VirtualMachine::on_exit() into run_cleanup_hooks() and calls it from global_exit() so N-API env cleanup hooks fire on test-runner exit paths. It introduces a process-global s_napiCleanupHooksOnly flag (atomic bool, set by global_exit(), read in NapiEnv::cleanup()) that restricts cleanup on undrained-loop exits to explicit env cleanup hooks + the instance-data finalizer, deliberately skipping the pending napi_wrap finalizer pass. Files touched: src/jsc/VirtualMachine.rs, src/jsc/bindings/napi.{cpp,h}, src/runtime/napi/napi_body.rs (comment only), a new test file, and two CI opt-out lists.

Security risks

None identified. This is exit-path resource cleanup; no user-controlled input, auth, or network surface. The memory-safety concern (finalizer ordering / UAF via napi_get_instance_data) was raised in an earlier round and addressed by nulling instanceData after its finalizer runs.

Level of scrutiny

High. This is not a mechanical change:

• It alters VM teardown ordering on every bun test exit and introduces a new behavioral contract (wrap finalizers do not run at bun test exit, diverging from bun run). That is a deliberate design decision with a stated rationale (duckdb crash), but it is a design decision.
• The PR went through a mid-development pivot: an earlier revision requested VM termination before the exit collection so finalizers ran mid-sweep, which broke test_cannot_run_js and was reverted. The final approach (defer-then-drop) is sound but subtle.
• A process-global flag is read by worker threads concurrently with the main-thread write; it was made atomic after review, but the semantic leak (a worker mid-teardown may observe hooks-only mode intended for the main VM) is accepted as harmless-at-exit rather than fixed structurally.
• The author's own comment flags a rebase interaction with #32180 that changes how has_run_cleanup_hooks is used as a signal.

Other factors

I reviewed this PR across seven rounds; every issue I raised (Windows CRLF, stale docs ×3, BUN_DESTRUCT_VM_ON_EXIT env leak, non-atomic flag, instance-data ordering inversion, safe fn, missing clearExceptionsBetweenFinalizers(), missing canBuildNodeAddons() gate) was addressed. The current bug-hunt run found nothing on 3e68a1f. Test coverage is good (positive + negative contract), CI is green on the target asan lane per the author's status comments, and the PR description is thorough. But the scope — exit-path finalizer lifecycle in a memory-safety-critical subsystem, with a user-visible behavioral choice and a known cross-PR interaction — is exactly the kind of change a maintainer (Jarred-Sumner or dylan-conway per the suggested reviewers) should sign off on rather than a bot.