worker: wake the loop when a worker stops itself from an immediate (process.exit() / uncaught error)

[robobun]

Problem

• In a worker_threads Worker (or Web Worker) whose loop is kept alive by something like a parentPort 'message' listener, process.exit() called from a setImmediate callback (or from a nextTick/microtask that callback queued), and likewise an uncaught exception thrown from one, does not end the worker promptly. The parent's 'exit' event arrives about 680ms later on 1.4.0 release (about 900ms on a debug build); the same exit from a setTimeout callback or a message listener takes ~3ms.
• The delay is "until something unrelated wakes the worker's loop": by default that is the idle GC timer (1s period, 30s once it has backed off after 30 idle fires). With BUN_GC_TIMER_DISABLE=1 the worker never exits at all; the parent process hangs.
• Cause: the worker's two self-stop sites, WebWorker::exit() (src/jsc/web_worker.rs:1088, reached from Bun__Process__exit) and the uncaught-error hook on_unhandled_rejection (src/jsc/web_worker.rs:1282), set requested_terminate, stop the VM handle and arm the JSC termination trap, but do not wake the loop. A parent's worker.terminate() goes through VmHandle::request_termination() (src/jsc/VmHandle.rs:387), which does the same three things plus wakeup().
• Why only immediates: spin() (web_worker.rs:933) checks requested_terminate after tick() and after auto_tick_active(). Immediates run at the top of auto_tick_active() (src/runtime/jsc_hooks.rs:1134), which then computes the poll timeout from the timer heaps and blocks in tick_with_timeout(); the flag is only re-read once that poll returns. Timer and I/O callbacks run after the poll, so their exits are seen right away. With a keep-alive and an empty timer heap, get_timeout() asks for an unbounded wait.

Fix

• Both self-stop sites call handle_ref().request_termination(), the same request a parent's terminate() makes: stop the handle, arm the trap, wake the loop. The wake is a write to the loop's wakeup fd (eventfd / mach port / uv_async_send), so the poll that follows returns at once and spin() sees the flag. Nothing else about the stop changes: stop() and notify_need_termination() are what these sites already did, in the same order.
• Correct because the wake goes through the mechanism the parent-side stop has always used (the worker's loop is already woken from another thread on terminate()); waking the loop from its own thread is the same eventfd write, and a wake that turns out to be unnecessary (no keep-alive, so the poll was non-blocking anyway) is harmless, exactly as it is today for a terminate() that lands while the worker is idle.
• Debug build, BUN_GC_TIMER_DISABLE=1: exit from an immediate went from never exiting to ~165ms (the same as an exit from a timer callback, i.e. plain teardown cost); with the GC timer on, ~900ms to ~165ms. Release numbers in the details below.
• Verified:
  • test/js/node/worker_threads/worker_threads.test.ts, new describe "a worker that stops itself from an immediate exits right away" (3 rows: process.exit(), process.exit() from a nextTick the immediate queued, an uncaught exception). Each runs with BUN_GC_TIMER_DISABLE=1, so without the src/ change the worker never exits and all three rows time out (confirmed on a build without it); with it they pass. Whole file: 132 pass.
  • test/js/web/workers/worker.test.ts, worker-terminate-lifetime.test.ts, worker-terminate-funnels.test.ts, worker-late-completion.test.ts, worker_destruction.test.ts, worker-top-level-await.test.ts, worker-async-dispose.test.ts, worker-shutdown-post-leak.test.ts: the only failures are identical on a build without this change (the fs Binding LSan report that node:fs: mark the per-VM Binding box as LSan-ignored (fixes worker-terminate-lifetime.test.ts on main) #35159 addresses, and 5s timeouts of tests that boot many workers on a debug/ASAN build).
  • 24 upstream test-worker-* exit / uncaught / terminate tests under test/js/node/test/parallel: all pass.
• Not covered here: a promise rejected (not thrown) inside an immediate is still only reported after the next loop wakeup, on the main thread as well. That has a different cause (rejections are not examined between the immediate phase and the poll) and is tracked separately; event loop: report promise rejections left by the turn that let the loop go idle #37981 is adjacent to it.
• worker: discard what was queued before process.exit(), an uncaught error or terminate(); first process.exit() decides the exit code #38016 edits the same two sites for a different bug (discarding queued work after the stop); the two changes compose, whichever lands second just calls both.

Background

• Worker run loop: WebWorker::spin() loops tick() (runs queued tasks and microtasks) then auto_tick_active() (runs the immediates queued for this turn, computes how long the loop may sleep from the timer heaps, blocks in the uSockets/libuv poll, then runs due timers). It breaks out and tears the VM down when requested_terminate is set; that flag is checked between those two calls, never inside them.
• requested_terminate and VmHandle: a worker is stopped by setting the WebWorker's requested_terminate flag (what spin() polls) and making the VM stop running script: VmHandle::stop() closes the gate every native-to-JS entry consults, and notify_need_termination() arms JSC's trap so script already on the stack unwinds with a TerminationException. VmHandle::request_termination() bundles those two with a loop wakeup, and is what the parent thread's terminate() calls; this PR makes the worker's own exit paths call it too.
• Loop wakeup: every uSockets loop has an async handle (an eventfd on Linux, a mach port on macOS, a uv_async_t on Windows) registered in its poll set. wakeup() signals it, so a blocked poll returns immediately and a poll entered afterwards returns without blocking. It works the same whether signalled from another thread or from the loop's own thread before it polls.
• Idle GC timer (GarbageCollectionController): a per-VM repeating timer that runs a GC every 1s (30s after the heap has been stable for 30 fires); BUN_GC_TIMER_DISABLE=1 turns it off. It is normally what bounded this bug at about a second, which is why the new tests disable it: without it a worker stuck in the poll stays stuck, so the tests hang rather than pass slowly on a build without the fix.

Measurements (time from the worker's postMessage right before the stop to the parent's 'exit' event)

Release 1.4.0, linux x64, worker kept alive by a parentPort listener, before this change:

shape	default	BUN_GC_TIMER_DISABLE=1
process.exit() from an immediate scheduled 300ms in	679ms	never exits
throw from that immediate	679ms (code 1)	never exits
process.exit() from a nextTick queued by that immediate	680ms	never exits
process.exit() from a microtask queued by that immediate	682ms	never exits
process.exit() from a setTimeout callback	4ms	3ms
throw from a setTimeout callback	4ms	5ms
process.exit() from a parentPort message listener	4ms	4ms
immediate scheduled synchronously at startup	~104ms	~103ms (a JSC startup timer happens to fire then)

Scanning the immediate's scheduling delay with the GC timer disabled (release): 0ms to 100ms after startup the exit piggybacks on a one-shot startup timer (106ms, 79ms, 54ms, 32ms, 4ms), from 150ms on it never exits. The debug build behaves the same (last startup timer at about 150ms), which is what the 300ms in the new tests is sized against.

Debug build after this change, all immediate shapes: 162ms to 195ms, the same as the timer shapes (161ms to 168ms) on that build. The Promise.reject shape is unchanged (910ms / never), see the last Fix bullet.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 40 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: a910b690-4aab-41d7-9916-3880e83e4b7f

📥 Commits

Reviewing files that changed from the base of the PR and between 7cf6296 and fbcebf4.

📒 Files selected for processing (2)

• src/jsc/web_worker.rs
• test/js/node/worker_threads/worker_threads.test.ts

---

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

[robobun]

^{Updated 5:03 AM PT - Aug 14th, 2026}

@robobun, your commit 6aa6e7d is building: #95963

[robobun]

Status: reproduced and fixed.

• Reproduced on 1.4.0 release with a worker_threads worker kept alive by a parentPort listener: process.exit() (or a throw) from a setImmediate callback reaches the parent's 'exit' event ~680ms later, and never with BUN_GC_TIMER_DISABLE=1; the same from a setTimeout callback takes ~4ms.
• Cause and fix are in the PR description; the new tests in test/js/node/worker_threads/worker_threads.test.ts hang (time out) on a build without the src/ change and pass with it.

[claude]

I reviewed this PR and didn't find any bugs. The change is small and well-reasoned — it routes the two worker self-stop sites through the existing VmHandle::request_termination() helper (already used by the parent's terminate()), which adds only a loop wakeup() on top of what these sites already did. Because this is worker VM teardown / termination lifecycle code, and the PR notes it composes with #38016 which edits the same two sites, a human look would still be worthwhile.

What was reviewed:

• Confirmed request_termination() = stop() + notify_need_termination() + wakeup(), identical to the inline code being replaced plus the wake; same call order.
• Checked the enter() gate is safe on the worker's own thread — state cannot be Closed while vm_ptr is non-null (Closed is set inside close_and_wait, which runs later in shutdown()), and the active counter inc/dec doesn't interact with anything at this point.
• Checked re-entrant process.exit() from an exit handler: shutdown() nulls self.vm before on_exit(), so exit()'s vm_ptr.is_null() guard still short-circuits.
• Test uses BUN_GC_TIMER_DISABLE=1 so it hangs (rather than passes slowly) on an unfixed build; the 300ms delay before the immediate is commented with why no observable signal exists.

Extended reasoning...

Overview

The PR fixes a hang where a worker that stops itself (process.exit() or an uncaught error) from a setImmediate callback does not exit promptly — it waits until something unrelated wakes the loop (the ~1s idle GC timer by default; forever with BUN_GC_TIMER_DISABLE=1). Three files touched: a doc-comment expansion in src/jsc/VmHandle.rs, two ~1-line behavioral changes in src/jsc/web_worker.rs (WebWorker::exit() and on_unhandled_rejection), and a new 3-row test.concurrent.each in test/js/node/worker_threads/worker_threads.test.ts.

The behavioral change replaces the inline handle_ref().stop(); jsc_vm().notify_need_termination(); at each site with handle_ref().request_termination(), an existing helper that does exactly those two calls in the same order and then wakeup()s the loop. The parent's worker.terminate() already goes through this helper, so the wake path is already exercised cross-thread; this PR adds the same-thread callers.

Security risks

None. No user-input parsing, no auth/crypto, no new unsafe surface (one unsafe block in exit() is now smaller). The wakeup is a write to the loop's own async handle — an established mechanism.

Level of scrutiny

High. This is worker VM termination / teardown lifecycle code — the module doc and surrounding comments in web_worker.rs describe UAF hazards, ordering constraints between shutdown() / spin() / JSC frames, and cross-thread handle publication. However, the actual diff is a strict superset of prior behavior via an existing shared helper, and the PR description demonstrates a thorough root-cause analysis (why only immediates: they run at the top of auto_tick_active() before the poll, and spin() re-checks the flag only after the poll returns).

I verified:

• request_termination()'s enter() gate cannot refuse here: state is at most Stopping while the worker's VM is live and spin() hasn't returned; Closed is only set inside close_and_wait, called from shutdown() via VirtualMachine::teardown.
• The active atomic inc/dec added by enter() is harmless on the worker thread (no waiter on it until close_and_wait).
• Re-entrant exit() from an exit handler is still guarded by the vm_ptr.is_null() check (shutdown nulls self.vm before running on_exit()).

Other factors

• The PR explicitly notes composition with #38016 (which edits the same two sites for a different bug); a maintainer should be aware of the merge order.
• Test quality is good: subprocess-spawned with pipes drained via Promise.all, asserts a combined {stdout, stderr, exitCode} object, uses BUN_GC_TIMER_DISABLE=1 so an unfixed build hangs (fails via timeout) rather than passing slowly. The 300ms setTimeout before scheduling the immediate is commented (per REVIEW.md's ">50ms sleep needs a named reason" rule) and backed by measurements in the PR description.
• The bug hunting system found no issues.

Given this is lifecycle-critical native code with an adjacent in-flight PR touching the same lines, deferring to a human reviewer for sign-off rather than auto-approving.

[claude]

LGTM — routes both worker self-stop sites through the existing VmHandle::request_termination() helper, adding the loop wakeup that parent-side terminate() already does.

What was reviewed:

• Confirmed request_termination() = stop() + notify_need_termination() + wakeup(); the removed code was the first two, so the only net change is the wake. enter() gates on != Closed, which holds whenever self.vm is non-null (both are cleared together in shutdown() step 1).
• New tests: BUN_GC_TIMER_DISABLE=1 so the unfixed build hangs rather than passing slowly; the 300ms setTimeout is commented with why no observable signal exists; pipes drained concurrently; combined {stdout, stderr, exitCode} assertion; test.concurrent.each over the three stop shapes.
• comment-cop feedback was addressed in fbcebf4 (VmHandle.rs is no longer in the diff; the two web_worker.rs sites are one-line comments).

Extended reasoning...

Overview

Two-line native fix in src/jsc/web_worker.rs: at the two places a worker stops itself (WebWorker::exit() reached from process.exit(), and on_unhandled_rejection for uncaught errors), replace inline handle_ref().stop() + jsc_vm().notify_need_termination() with handle_ref().request_termination(). Cross-referencing src/jsc/VmHandle.rs:387, request_termination() performs exactly stop() → (gated on enter()) notify_need_termination() → loop_of(Regular).wakeup(). The gate check is state != Closed, which is always true here: self.vm is non-null (checked at the exit() site) and the handle is closed only in teardown(), after shutdown() has already nulled self.vm. So the only behavioral change is the added wakeup(), which is the fix — an immediate runs before auto_tick_active() blocks in the poll, and without a wake the flag isn't seen until something else ends that poll.

Plus a test.concurrent.each block in test/js/node/worker_threads/worker_threads.test.ts covering three shapes (process.exit from immediate, from a nextTick the immediate queued, uncaught throw from immediate).

Security risks

None. No user input parsing, no auth/crypto/permissions. The wakeup writes to the loop's own eventfd/mach-port/uv_async; it's the same call path parent-side worker.terminate() has always used from another thread, now called from the worker's own thread before its poll.

Level of scrutiny

Worker termination is memory-safety-sensitive, but this change is a consolidation onto an existing, already-exercised helper rather than new logic. The helper is what every parent terminate() already goes through; calling it from the worker's own thread is strictly a subset of the concurrency it's built for (writing your own loop's wakeup fd before polling it, vs. from another thread while it's polling). The 12-line comment removed from on_unhandled_rejection was explaining what the old inline code did; the one-line replacement points at exit()'s identical call and spin()'s shutdown, which is where that reasoning still lives.

Other factors

• Test design follows the repo review guide: BUN_GC_TIMER_DISABLE=1 makes the unfixed build hang (verified by author) rather than pass ~680ms slower, so the test can't accidentally certify a regression; the 300ms setTimeout is commented per the ≥50ms-sleep rule; pipes drained via Promise.all; asserts on a combined object with exact stdout JSON, empty stderr, and exit 0; uses {...bunEnv, ...}; test.concurrent for the three independent subprocesses.
• The PR description enumerates the sibling paths (setTimeout callbacks, message listeners) and explains why they don't need the fix (they run after the poll returns), and calls out the one adjacent shape not covered (Promise.reject inside an immediate) as a separate root cause tracked elsewhere.
• comment-cop bot flagged long comments on earlier commits; the author trimmed them in fbcebf4 and VmHandle.rs is no longer touched — final diff is 2 files.