process: drain microtasks after emitting 'exit' on natural termination by robobun · Pull Request #34910 · oven-sh/bun

robobun · 2026-07-21T09:09:40Z

Repro

process.on("exit", async () => {
  console.log("exit-listener");
  Promise.resolve().then(() => console.log("pt-in-exit"));
  queueMicrotask(() => console.log("qm-in-exit"));
  await Promise.resolve();
  console.log("after-await-in-exit");
});

$ node repro.js
exit-listener
pt-in-exit
qm-in-exit
after-await-in-exit
$ bun repro.js       # before
exit-listener

Cause

dispatchExitInternal in src/jsc/bindings/BunProcess.cpp emits 'exit' and returns; nothing runs a microtask checkpoint afterwards, so promise reactions, queueMicrotask callbacks, and the first-await continuation of an async listener are dropped.

Node performs a final microtask checkpoint after emitting 'exit', but only on a natural event-loop drain. process.exit() and fatal uncaught exceptions skip it, and process.nextTick callbacks are not drained (verified against node v26.3.0 for main thread and workers).

Fix

Thread a drainMicrotasks flag through Process__dispatchOnExit and call vm.drainMicrotasks() after the emit when set. Rust's on_exit() passes unhandled_error_counter == 0 so natural exit drains and fatal-error exit does not; process.exit() and the worker uncaught-exception path pass false. The existing m_isExiting guard ensures a later on_exit() reached via process.reallyExit never reaches the drain.

The sibling 'beforeExit' face of this (#32866) already landed; this is the 'exit' residue.

no test proof · iteration 2 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/node/process/process.test.js

Known residual gaps (follow-up)

These narrow cases still diverge from Node; all happen when something originates inside the 'exit' listener (or the rejection sweep it triggers), after on_exit(true) has already committed to the natural-drain path. None are regressions: before this PR the microtask/rejection was dropped entirely on every path.

An 'exit' listener throws and an uncaughtException handler swallows it: innerInvokeEventListeners clears the exception internally, so the emit returns cleanly and the drain runs. Fixing this needs emit() to surface that a listener threw.
In a worker on natural drain, the 'exit' listener calls process.exit(): WebWorker::exit() intentionally skips notify_need_termination() when self.vm has already been unpublished by shutdown(), so the !hasTerminationRequest() guard passes.
A microtask enqueued by an unhandledRejection handler fired from the post-drain sweep is dropped (single drainMicrotasks(); handleRejectedPromises(); pass vs. Node's processTicksAndRejections loop).
In a worker, the handleRejectedPromises() sweep is a no-op: shutdown() sets is_shutting_down = true before on_exit(), and unhandled_rejection() short-circuits on that flag. Moving the assignment would change the pre-existing worker 'exit'-listener-throws behavior.

coderabbitai · 2026-07-21T09:09:47Z

Walkthrough

Changes

Natural VM shutdowns now propagate a microtask-draining flag through Rust and C++ exit dispatch. Explicit exits and fatal paths disable draining. CLI, worker, and runtime callers pass explicit shutdown state, with subprocess tests covering natural, explicit, fatal, and worker exits.

Exit Microtask Draining

Layer / File(s)	Summary
Shutdown path selection `src/jsc/VirtualMachine.rs`, `src/jsc/web_worker.rs`, `src/runtime/...`, `src/runtime/node/node_process.rs`	VM shutdown APIs accept natural-exit state, and runtime, CLI, and worker paths pass values that distinguish normal completion from errors or requested termination.
Exit dispatch and validation `src/jsc/bindings/BunProcess.cpp`, `test/js/node/process/process.test.js`	C++ exit dispatch conditionally drains microtasks, while subprocess tests verify natural exits, explicit exits, fatal errors, `process.nextTick`, and workers.

Possibly related PRs

oven-sh/bun#34443: Adjusts the VM shutdown and exit-hook path used by the test runner.
oven-sh/bun#34444: Adds a test-runner path that invokes VM shutdown lifecycle hooks affected by this exit behavior.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title is concise and accurately describes the main behavioral change in natural exit handling.
Description check	✅ Passed	The description is substantive and includes repro, root cause, fix, and verification, but it doesn't use the required template headings.

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

robobun · 2026-07-21T09:09:50Z

^{Updated 6:50 AM PT - Jul 21st, 2026}

✅ @robobun, your commit bcbe14841bcc1a6b53208f94e6bf9a792369005f passed in Build #76845! 🎉

🧪 To try this PR locally:

bunx bun-pr 34910

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

bun-34910 --bun

claude

Both prior nits are addressed in c09dea6 (worker requested_terminate gate + combined-object test assertions) and no new issues turned up this pass. Deferring to a human for the final call since this threads a flag through the VM/process shutdown path across the Rust↔C++ FFI boundary — ordering-sensitive territory.

What was reviewed:

Traced all four dispatch_on_exit call sites (main-thread on_exit, Process_functionExit, worker on_unhandled_rejection, worker shutdown()→on_exit) — each reaches the new drain gate with the intended flag.
Confirmed worker_ref() reads the VM's back-pointer (not WebWorker.vm), so it's still valid after shutdown() unpublishes vm under vm_lock.
m_isExiting early-return still short-circuits the re-entrant process.exit()→reallyExit→on_exit() path before the drain is reached.

Extended reasoning...

Overview

Adds a post-'exit'-emit microtask checkpoint on natural event-loop termination (Node.js compat). Threads a drain_microtasks: bool through Process__dispatchOnExit (Rust extern + C++ impl), computes the flag in VirtualMachine::on_exit() as unhandled_error_counter == 0 && !worker.has_requested_terminate(), and calls vm.drainMicrotasks() after emitter.emit("exit", …) when set. process.exit() and the worker fatal-exception path pass false explicitly. Five new subprocess tests cover natural / process.exit() / fatal / nextTick-not-drained / worker-natural.

Security risks

None — no untrusted input parsing, no auth/crypto, no new resource acquisition. The added vm.drainMicrotasks() runs user JS the emit already ran; no new capability surface.

Level of scrutiny

Medium-high. The change itself is small and well-scoped, but it sits in the VM/process shutdown path where ordering is load-bearing (worker shutdown() clears the JSC termination request, unpublishes vm, runs on_exit(), then tears down JSC). A misplaced drain here could re-enter shutdown or run after teardown. I traced each call site and the guards (m_isExiting, !vm.hasTerminationRequest(), the new has_requested_terminate() check) and they compose correctly, but this is exactly the kind of change a maintainer familiar with the shutdown-ordering history should eyeball.

Other factors

My two prior nits (worker-terminate() gate imprecision; standalone expect(stderr).toBe("")) were both addressed in c09dea6 and the threads are resolved.
Test coverage is solid for the stated variant matrix; the terminate() variant is intentionally omitted per the author's reply (Node doesn't emit 'exit' there — pre-existing divergence).
FFI signature change is applied at every call site (grep confirms four Rust callers + one internal C++ caller, all updated).
CI build #76787 is still in progress; the PR description notes tests were deferred to CI.

Node performs a final microtask checkpoint after emitting the 'exit' event on a natural event-loop drain, so Promise.resolve().then(cb), queueMicrotask(cb), and the first-await continuation of an async exit listener all run before the process terminates. This checkpoint only happens for natural exit; process.exit() and fatal uncaught exceptions skip it, and process.nextTick callbacks are not drained. Bun was emitting 'exit' and terminating immediately without the checkpoint, so microtasks queued by an 'exit' listener were dropped. Thread a drain_microtasks flag through Process__dispatchOnExit. Rust's on_exit() passes (unhandled_error_counter == 0) so natural exit drains and fatal-error exit does not; process.exit() and the worker uncaught-exception path pass false. The m_isExiting guard in dispatchExitInternal ensures a later on_exit() reached via process.reallyExit never reaches the drain.

…st assertions Also gate the post-'exit' microtask drain on the worker's requested_terminate flag so worker.terminate() does not drain even though shutdown() clears the JSC termination request before on_exit(). Switch the four new stderr-empty assertions to the combined {stdout, stderr, exitCode} form per the file's existing convention.

coderabbitai

Actionable comments posted: 3

🤖 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/VirtualMachine.rs`:
- Around line 1516-1519: Condense the comments at src/jsc/VirtualMachine.rs
lines 1516-1519 and src/jsc/bindings/BunProcess.cpp lines 305-308 to three lines
or fewer each, preserving their natural-exit and Node microtask rationale
without changing code behavior.

In `@test/js/node/process/process.test.js`:
- Around line 803-826: Extend the worker shutdown tests near the existing
natural-exit case to cover uncaught worker errors and requested termination via
process.exit() and worker.terminate(). Assert each path emits the expected
shutdown output while queued microtasks do not run, using the existing Bun.spawn
test pattern and preserving the natural-exit expectation.
- Line 808: Replace the dynamic require of Worker in the -e child fixture with a
static ESM import from node:worker_threads, preserving the existing Worker usage
and behavior.

🪄 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: e0247b9c-38af-4758-a1a3-ad624afdb03c

📥 Commits

Reviewing files that changed from the base of the PR and between e550f2c and f92e65e.

📒 Files selected for processing (4)

src/jsc/VirtualMachine.rs
src/jsc/bindings/BunProcess.cpp
src/jsc/web_worker.rs
test/js/node/process/process.test.js

The previous heuristic (unhandled_error_counter == 0 plus worker requested_terminate) missed the exit_on_uncaught_exception fast-exit path: a beforeExit listener that throws with no uncaughtException handler reaches Bun__Process__exit -> on_exit() before the counter is bumped, so microtasks drained where Node does not. Give on_exit() an explicit 'natural: bool' so each caller states whether it is on the natural-drain path. Bun__Process__exit and all error paths pass false; the worker shutdown path passes !has_requested_terminate().

eval_script() reports its exception without routing through uncaught_exception(), so unhandled_error_counter stays 0 and the belt-and-suspenders check would not have suppressed the drain. Hoist had_error across both the eval and interactive branches and pass it.

vm.drainMicrotasks() flushes the JSC microtask queue but not m_aboutToBeNotifiedRejectedPromises, so a Promise.reject() created inside an 'exit' listener never reached unhandledRejection (and, with no handler, exited 0 where Node exits 1). Call handleRejectedPromises() after the drain so the rejection is reported.

github-actions Bot added the claude label Jul 21, 2026