test: give worker-shutdown-post-leak an explicit 90s timeout

[robobun]

Problem

• test/js/node/worker_threads/worker-shutdown-post-leak.test.ts (ASAN-only, added in worker: mark the context terminating before the final concurrent-queue drain #34278) fails with this test timed out after 5000ms. when run with bun bd test on a debug+ASAN build on a busy machine.
• The scenario it spawns is healthy: run by hand with the test's env it exits 0 with empty output, but one worker boot plus BUN_DESTRUCT_VM_ON_EXIT=1 teardown takes 4.7 to 7.1s under debug+ASAN here (load average 120 to 170 on a 12 CPU cgroup), so the file's 5s default per-test timeout has no headroom. Same binary, empty -e 0: 0.3 to 0.5s.
• The failure mode is cut off by the same budget: when LSan does find a leak, it symbolizes the leaked stacks against the ~1GB debug binary before exiting. Measured on this binary with an induced one-stack leak, that adds about 4.8s (0.5 to 0.6s with detect_leaks=0, 5.3 to 5.5s with the report). The test is watching for two allocations (ConcurrentTask + EventLoopTask), so a real regression would also be reported as a timeout, with the subprocess killed before the LSan report reaches the failure output.
• CI is not affected either way: the runner passes --timeout=270000 on the asan lane, where this file's median is 442ms (test/expected-durations.json). The 5s default only applies to direct bun bd test runs, which is where this flaked.

Fix

• Pass an explicit 90_000 per-test timeout, with a comment saying why.
• Why this is the right fix: the slow part is worker VM boot and teardown under debug+ASAN plus LSan symbolization, neither of which the test can make cheaper (it is already a single worker doing two writes), so the budget is what was wrong. This is the per-test-timeout-for-an-outlier case the root CLAUDE.md allows (test/CLAUDE.md's "no timeouts" rule is the default for ordinary tests), and 90s is the budget the other LSan guard tests of the same shape already use (test/js/bun/shell/shell-worker-terminate-leak.test.ts, test/js/bun/spawn/spawn-stdin-pipe-fd-leak.test.ts), for the same reason.
• An explicit per-test timeout overrides --timeout (verified), so on CI's asan lane the budget goes from 270s to 90s, still about 200x the measured median, and a real leak regression still fails the test there, now with the LSan report attached.
• Verified: bun bd test test/js/node/worker_threads/worker-shutdown-post-leak.test.ts timed out at 5008ms before the change on this machine; after it, three runs pass in 6.2s, 6.8s and 7.1s. Test-only change, no src/ changes.

Background

• LSan guard test: the test runs a scenario in a subprocess with ASAN_OPTIONS=detect_leaks=1 and BUN_DESTRUCT_VM_ON_EXIT=1 (tear the VM down fully at exit instead of calling _exit), so anything still allocated at exit is reported by LeakSanitizer as a leak and turns into non-empty stderr and exit code 1. The leak under guard here is the one fixed in worker: mark the context terminating before the final concurrent-queue drain #34278: a cross-thread task posted to a worker during its shutdown was enqueued onto a queue that is never drained again.
• Per-test timeout: test(name, fn, ms) sets the budget for that one test and takes precedence over the file default (5s) and over bun test --timeout, which CI uses to set the default per lane.

Timing probes (debug+ASAN build of b7a0431, linux-x64, load average 120 to 170)

Scenario cost, three runs each, same env as the test:

empty -e, no leak env              : 0.42s, 0.31s, 0.29s
empty -e, BUN_DESTRUCT_VM_ON_EXIT+LSan: 0.52s, 0.54s, 0.48s
worker scenario, no leak env       : 5.44s, 5.18s, 5.94s
worker scenario, test env          : 5.94s, 6.54s, 5.31s

Where the time goes (stderr timestamps relative to new Worker): require("worker_threads") alone is ~1.3s, the worker's online event fires at +2.5 to +3.0s, exit at +3.1 to +3.8s. Release build of the same scenario: 0.08 to 0.10s total.

Failure-mode cost: a C function compiled with bun:ffi cc that mallocs three blocks and drops them, called from a setTimeout so the stack is not covered by test/leaksan.supp, run with the test's env:

detect_leaks=0 : 0.64s, 0.52s
leak reported  : 5.34s, 5.54s, 5.41s   (29-frame stack, SUMMARY: 195 byte(s) leaked in 3 allocation(s))

bun bd test on the unmodified file: (fail) ... [5008.10ms] this test timed out after 5000ms. With this change: (pass) ... [6249.80ms], [6762.09ms], [7056.25ms].

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 12 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: 3448881d-f739-4cc7-8660-46e9e8691161

📥 Commits

Reviewing files that changed from the base of the PR and between b7a0431 and 4d0e56f.

📒 Files selected for processing (1)

• test/js/node/worker_threads/worker-shutdown-post-leak.test.ts

---

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

[robobun]

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

❌ @robobun, your commit 4d0e56f has some failures in Build #94600 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 38157

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

bun-38157 --bun

[robobun]

Status: ready for review.

• Reproduced on a debug+ASAN build (bun bd test test/js/node/worker_threads/worker-shutdown-post-leak.test.ts) on a busy machine: this test timed out after 5000ms. The spawned scenario itself exits 0 in 4.7 to 7.1s there; empty -e 0 on the same binary is 0.3 to 0.5s.
• With this change the same command passes three times in a row (6.2s, 6.8s, 7.1s). Test-only change; CI's asan lane is unaffected in practice (its own per-test budget was already larger, and the file's median there is 442ms).
• Overlap: worker: free the thread's event name table when the worker thread exits #38164 (a fix for a different worker leak, opened after this PR) applies the same 90s budget to this test while adding its own cases to the file. If worker: free the thread's event name table when the worker thread exits #38164 lands first, close this PR; otherwise worker: free the thread's event name table when the worker thread exits #38164 rebases that one hunk.

[claude]

LGTM — test-only timeout bump matching the established 90s budget used by the other LSan guard tests.

What was reviewed:

• Confirmed shell-worker-terminate-leak.test.ts and spawn-stdin-pipe-fd-leak.test.ts both already use 90_000 for the same reason, so this follows the existing pattern rather than inventing one.
• Checked against the "don't raise timeouts to make a slow test pass" rule — the workload is already minimal (one worker, two writes); the cost is debug+ASAN worker boot and LSan symbolization, which the test cannot shrink.
• The added comment is concise and explains both the pass-path and fail-path timing, which the code alone cannot convey.

Extended reasoning...

Overview

Single-file, test-only change to test/js/node/worker_threads/worker-shutdown-post-leak.test.ts: adds an explicit 90_000 per-test timeout as the third argument to test.skipIf(...)(...), plus a four-line comment explaining why. No src/ changes, no assertion changes, no test logic changes.

Security risks

None. This is a test timeout value in an ASAN-only guard test. No runtime code, no user-facing surface, no inputs handled.

Level of scrutiny

Low. The diff is +5 lines (a numeric literal and a comment). The main review question is whether raising a timeout is papering over a real problem — REVIEW.md flags "don't raise per-test timeouts to make a slow test pass; shrink the workload." The PR description addresses this directly with timing probes: the scenario is already the minimum reproducer (one worker, two stdout writes), and the ~5-7s cost under debug+ASAN comes from worker VM boot/teardown plus LSan symbolizing against a ~1GB debug binary. Neither can be reduced by the test. The 5s default was the defect, not the test.

Other factors

• Verified the two sibling LSan guard tests cited in the PR (shell-worker-terminate-leak.test.ts line 47, spawn-stdin-pipe-fd-leak.test.ts line 137) both use exactly 90_000 with near-identical justification comments, so this is convergence on an established pattern.
• The PR notes the explicit timeout overrides CI's --timeout=270000, dropping the CI budget from 270s to 90s — still ~200x the 442ms measured median, so this tightens rather than loosens the CI-side bound while fixing local bun bd test runs.
• The comment satisfies REVIEW.md's "a literal sleep/setTimeout of 50ms or more … needs a comment naming why" spirit for timeout choices — it names both the pass-path cost (worker boot + VM teardown) and the fail-path cost (LSan symbolization), which is exactly what a future reader would otherwise spend time re-measuring.
• No prior reviews on the PR; nothing outstanding to address.

[github-actions]

This PR may be a duplicate of:

1. worker: free the thread's event name table when the worker thread exits #38164 - Modifies the same file and applies the same 90_000 per-test timeout to the same existing worker-shutdown-post-leak test (as LEAK_TEST_TIMEOUT), for the same debug+ASAN reason, alongside the underlying event-name-table leak fix.

🤖 Generated with Claude Code

[robobun]

Not a duplicate in purpose, but the overlap is real: #38164 fixes a different leak (the worker thread's EventNames table) and, while adding its own LSan cases to this file, also gives the existing test the same 90s budget this PR adds. That one hunk is the whole of this PR.

This PR predates #38164 and is test-only, so it can land on its own. If #38164 lands first, this PR becomes unnecessary and should be closed; if this one lands first, #38164 only needs to rebase that hunk.