test(napi): cover worker.terminate() with napi_async_work execute in flight

[robobun]

Problem

• worker.terminate() while a napi_async_work execute callback was still running on the thread pool freed the worker's VM under it.
• The pool thread then posted into the freed VM: heap-use-after-free in EventLoop::vm_ref (freed by WebWorker::shutdown). execute also kept writing an ArrayBuffer teardown had released.
• Worker / worker_threads: WebCore-shaped lifetimes, joined threads, one ordered VM teardown #37075 fixed this but added no napi test. This carries over the test from napi: block worker shutdown on in-flight napi_async_work (UAF) #36855, which Worker / worker_threads: WebCore-shaped lifetimes, joined threads, one ordered VM teardown #37075 superseded. No runtime changes.

Fix

• Adds a small addon whose execute sleeps, then memsets a JS-owned ArrayBuffer on the pool thread.
• Adds a test that terminates a worker only after it reports four such works queued, so terminate always lands while execute is asleep.
• Verified: at 52bf09c (parent of the Worker / worker_threads: WebCore-shaped lifetimes, joined threads, one ordered VM teardown #37075 merge) it fails 3/3 under ASAN with the UAF above; on current main it passes 3/3.
• LSan is off in the subprocess: eval workers leak-report their source Blobs at exit on main regardless of the addon, so the test asserts on the crash only.

Background

• node-api (napi) is the C ABI native Node addons compile against; bun implements it. The addons under test/napi/napi-app are built by node-gyp and driven from napi.test.ts.
• napi_async_work runs execute on a thread pool, then posts to the owning event loop so complete runs on the JS thread.
• Each worker_threads worker owns a VM and event loop; terminate() frees both and finalizes heap objects such as ArrayBuffer stores.
• ASAN aborts as soon as freed memory is touched, which makes this race a deterministic failure.

---

no test proof · iteration 4 · Platform-specific test-only change; deferring to CI.

Original description

Problem

• worker.terminate() while a napi_async_work execute callback was still running on the thread pool freed the worker's VM under it; the pool thread's completion post then hit a heap-use-after-free (EventLoop::vm_ref <- enqueue_task_concurrent <- napi_async_work::run, freed by WebWorker::shutdown), and the addon's execute kept writing an ArrayBuffer store that VM teardown had already released.
• Worker / worker_threads: WebCore-shaped lifetimes, joined threads, one ordered VM teardown #37075 fixed this class (napi async work now reaches the VM through the per-VM handle that teardown counts and closes). It did not add a napi-specific test; this PR carries over the one from napi: block worker shutdown on in-flight napi_async_work (UAF) #36855, which that PR superseded.

Test

• test/napi/napi.test.ts, napi_async_work > worker.terminate() with execute callbacks in flight ...: a subprocess repeatedly starts a worker that queues four works (300-450 ms execute, each memsetting a 16 MiB ArrayBuffer on the pool thread) and terminates it while they are in flight.
• Addon: test/napi/napi-app/test_async_work_worker_terminate.c, public node-api only (napi_get_arraybuffer_info + napi_ref pin + napi_create/queue_async_work).
• At 52bf09c (parent of the Worker / worker_threads: WebCore-shaped lifetimes, joined threads, one ordered VM teardown #37075 merge) with only this test applied: fails 3/3 under bun bd (ASAN), heap-use-after-free in EventLoop::vm_ref as above.
• On current main (9a543cc): passes 3/3.
• LSan is disabled for the subprocess: on main a -e script that creates eval workers reports their source Blobs at exit with or without the addon, so the test asserts on the crash only (the napi-side state is freed at teardown; nothing napi-related shows up with LSan on).

fail-before at 52bf09c

+ ==15816==ERROR: AddressSanitizer: heap-use-after-free on address 0x716f37f4a628
+ SUMMARY: AddressSanitizer: heap-use-after-free src/jsc/event_loop.rs:1072:18 in <bun_jsc::event_loop::EventLoop>::vm_ref
(fail) napi > napi_async_work > worker.terminate() with execute callbacks in flight waits for them and does not UAF

(3/3 runs)

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 2 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: add95e22-9a20-4cf1-91bc-05f63bd97064

📥 Commits

Reviewing files that changed from the base of the PR and between 9a543cc and 62f7ed6.

📒 Files selected for processing (3)

• test/napi/napi-app/binding.gyp
• test/napi/napi-app/test_async_work_worker_terminate.c
• test/napi/napi.test.ts

---

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

[robobun]

^{Updated 5:05 PM PT - Aug 12th, 2026}

❌ @robobun, your commit 62f7ed6 has 1 failures in Build #93517 (All Failures):

• test/cli/install/bun-install-registry.test.ts - code 1 on 🪟 11 aarch64

---

🧪   To try this PR locally:

bunx bun-pr 37918

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

bun-37918 --bun

[claude]

LGTM — test-only PR adding a regression test for the #37075 fix; follows established napi test patterns.

What was reviewed:

• The C addon uses public node-api only, matches the CHECK/free pattern of sibling .c addons, and the binding.gyp target is a byte-for-byte copy of neighbors.
• The test awaits the observable postMessage("up") before terminating, wires error/exit to reject, drains stdout/stderr/exited concurrently, and asserts a combined object — matching REVIEW.md subprocess conventions and neighboring tests in this file.
• Checked that the fixed sleeps live in the pool-thread execute callback (creating the race), not on the JS side waiting for a condition; iteration count branches on isASAN; needsInstall() picks up the new .c source.

Extended reasoning...

Overview

This PR adds a single regression test for a heap-use-after-free that #37075 already fixed (worker.terminate() while napi_async_work execute callbacks are in flight on the thread pool). It touches three files, all under test/napi/: a new ~110-line C addon using only public node-api, a new binding.gyp target copied verbatim from sibling targets, and one new it() block inside the existing napi_async_work describe in napi.test.ts. No production code changes.

Security risks

None. This is test-only code that compiles a small napi addon and spawns a Bun subprocess with bunExe()/bunEnv. No network access, no auth, no untrusted input parsing.

Level of scrutiny

Low-to-medium. Test-only, self-contained, and the PR description documents fail-before (3/3 ASAN heap-use-after-free at the pre-#37075 commit) and pass-after (3/3 on main). The main risks for a test PR of this shape are flakiness and CI cost, both of which I checked.

Other factors

• Test conventions: The test is placed in the existing napi_async_work describe (not a new file), spawns via bunExe() + bunEnv, drains all pipes with Promise.all, and asserts {stdout, stderr, exitCode} as one object. The worker "up" handshake wires message/error/exit so a premature exit rejects rather than hangs. The stderr: "" assertion matches several neighboring tests in this file that also spawn bunExe() -e under bunEnv.
• Sleeps: The 300-450ms sleeps are inside the addon's thread-pool execute callback — they are the in-flight work the test terminates around, not a JS-side wait for a condition. The JS side awaits postMessage("up") before w.terminate(). This is the correct shape for reproducing the race and is documented in the test comment.
• Timing/CI cost: 3 (ASAN) or 5 iterations, each bounded by the longest execute (~450ms) plus worker startup; the 30s per-test timeout has headroom for debug+ASAN. The suite is describe.concurrent, so this runs alongside siblings.
• LSan disable: The comment justifies detect_leaks=0 (pre-existing eval-worker source-Blob leak unrelated to napi), matching REVIEW.md's rule that skipping a safety net needs a stated reason.
• Addon: The CHECK macro leaks the calloc'd work_t on a mid-setup napi failure, but that's the established idiom across the sibling test addons and only reachable on programmer error in fixture code.
• Build integration: needsInstall() in this file scans *.c sources, so the new addon triggers a rebuild on first run.

[robobun]

CI at 62f7ed6 (build 93517): test/napi/napi.test.ts, including the new test, passes on every lane.

The one non-flaky red is test/cli/install/bun-install-registry.test.ts "hoisting > peers > it should hoist 1.0.1 when peer *" on windows aarch64 (expects a-dep 1.0.1, gets 1.0.9). Same failure was red on an unrelated branch in build 88406, so it is a main break in the installer, not this diff; it has been reported for triage. The remaining entries passed on retry.

Ready for review.

[robobun]

Looked into the LSan report behind the detect_leaks=0 override in this test. It is not a leak of the eval source Blobs, so the override should not be needed.

• The objects LSan lists (Blob::new called from the Blob constructor, plus the Store and source bytes hanging off them) are the native state of the new Blob([src]) wrappers worker_threads creates for eval: true. Those wrappers are ordinary JS garbage: adding Bun.gc(true) before the script ends makes the report go away, and the object URLs are revoked (the Stores are only reachable from the reported boxes).
• They only get reported when the process exits without sweeping the JS heap, i.e. without BUN_DESTRUCT_VM_ON_EXIT=1. In that mode a throwaway new Blob(["x"]) in a plain .mjs file produces the identical report. With the env the runner sets for LSan-validated files (BUN_DESTRUCT_VM_ON_EXIT=1, detect_leaks=1, test/leaksan.supp), the -e script from this test (eval workers, both terminate() and natural exit) exits 0 with no report, 3/3 runs on a debug ASAN build of current main. bunEnv inherits that env, so the subprocess here gets it in CI.
• The -e vs file difference is GC timing only: -e/-p invocations start JSC with numberOfGCMarkers = 1 (JSCInitialize, one-shot startup), so on a debug ASAN build the eden collection that would have swept the last two wrappers had not finished by exit. BUN_JSC_numberOfGCMarkers=8 bun -e ... is clean and BUN_JSC_numberOfGCMarkers=1 bun file.mjs shows the same two Blobs.

So the ASAN_OPTIONS override in the spawn can be dropped (worth a quick re-run with the addon to confirm nothing addon-specific shows up), and the "eval workers leak-report their source Blobs" line in the description does not hold.