[robobun]

Repro

process.on("beforeExit", async () => {
  await Promise.resolve();
  console.log("cleanup ran");
});

$ node repro.js
cleanup ran
$ bun repro.js       # before this change
(nothing)

Anything after the first await in a beforeExit listener is silently dropped, so the common "flush logs / close connections on shutdown" pattern does nothing and the process exits 0. The same applies to Promise.resolve().then(...) and queueMicrotask(...) queued from the listener; nextTick / setImmediate / timers from the same listener work fine.

Cause

Process__dispatchOnBeforeExit (src/jsc/bindings/BunProcess.cpp) drains pending work after the listeners run, but only through m_nextTickQueue->drain():

auto fired = process->wrapped().emit(Identifier::fromString(vm, "beforeExit"_s), arguments);
if (fired) {
    if (globalObject->m_nextTickQueue) {
        auto nextTickQueue = globalObject->m_nextTickQueue.get();
        nextTickQueue->drain(vm, globalObject);
    }
}

m_nextTickQueue is lazily created the first time process.nextTick() is called. If the program never called it, the branch is skipped and no microtask checkpoint runs. VirtualMachine::on_before_exit then re-checks is_event_loop_alive(), which counts handles/tasks/immediates but not JSC's microtask queue, so the queued reaction is invisible and the process proceeds to exit.

Confirmed by inserting a dummy process.nextTick(() => {}) at the top of the repro: with the queue initialized, Bun already prints cleanup ran.

Fix

Call globalObject->drainMicrotasks() unconditionally after emit. Zig::GlobalObject::drainMicrotasks already drains the nextTick queue when it exists and vm.drainMicrotasks() otherwise, with the same exception handling as the main tick path.

The existing re-entry loop in VirtualMachine::on_before_exit then gives Node's re-fire behavior for free: if the drained microtask arms a timer/immediate, is_event_loop_alive() turns true, the loop ticks and beforeExit fires again. A bare microtask does not re-arm the loop (Node-identical; no infinite loop).

Verification

Three new tests in the process.onBeforeExit describe block of test/js/node/process/process.test.js:

• async listener: code after await Promise.resolve() runs
• queueMicrotask inside the listener runs
• a microtask that arms a setTimeout fires the timer and beforeExit fires a second time (Node: BE#1 MICRO TIMER BE#2)

# fail-before (main's src/)
bun bd test test/js/node/process/process.test.js -t "drains microtasks|re-fires after a microtask"
  0 pass, 3 fail

# pass-after
bun bd test test/js/node/process/process.test.js -t "drains microtasks|re-fires after a microtask"
  3 pass, 0 fail

The existing process.onBeforeExit tests and Node's test-process-beforeexit*.js / test-timers-unrefed-in-beforeexit.js still pass.

Related

Process__dispatchOnExit (microtasks from 'exit' listeners) is left untouched here; that is a separate face tracked independently. The process.exitCode divergence flagged in the original fuzz report is covered by #32814.

---

no test proof · iteration 8 · 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

[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: ee9ce807-918b-4fc5-a1be-90731e27d5d9

📥 Commits

Reviewing files that changed from the base of the PR and between 59242d6 and 6ddf079.

📒 Files selected for processing (2)

• src/jsc/bindings/BunProcess.cpp
• test/js/node/process/process.test.js

---

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

[robobun]

^{Updated 7:05 AM PT - Jul 29th, 2026}

❌ @robobun, your commit 6ddf079 has 1 failures in Build #85032 (All Failures):

• test/js/node/test/parallel/test-worker-message-port-transfer-terminate.js - SIGABRT on 🐧 13 x64-asan

---

🧪   To try this PR locally:

bunx bun-pr 32866

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

bun-32866 --bun

[claude]

LGTM — minimal, well-tested fix that mirrors the existing drain path.

Extended reasoning...

Overview

This PR adds a 2-line else branch to Process__dispatchOnBeforeExit in src/jsc/bindings/BunProcess.cpp: when m_nextTickQueue has not been lazily created (because process.nextTick was never called), it now calls vm.drainMicrotasks() directly. Previously, microtasks queued by an async beforeExit listener were silently dropped in that case. Three regression tests are added to test/js/node/process/process.test.js covering async listeners, queueMicrotask, and event-loop re-entry via setTimeout after an await.

Security risks

None. No user input parsing, auth, crypto, or filesystem changes — this only ensures already-queued JS microtasks get a chance to run before process shutdown, which matches Node.js semantics.

Level of scrutiny

Low. The runtime change is a single else { vm.drainMicrotasks(); } that is symmetric with the existing branch: JSNextTickQueue::drain already calls vm.drainMicrotasks() when the queue exists (verified at JSNextTickQueue.cpp:83), so this just covers the not-yet-initialized case with identical behavior. The change only executes when a beforeExit listener actually fired, and only at shutdown — no hot-path or steady-state impact.

Other factors

• The PR description includes a clear repro, root-cause analysis, and before/after test runs (0→3 pass).
• The new tests follow the exact pattern of adjacent tests in the same describe block (spawn bun -e, assert stdout/stderr/exitCode), and each guards against re-entrancy with a done flag.
• No CODEOWNERS apply to the touched files.
• The bug-hunting system found no issues, and there are no outstanding reviewer comments.

[claude]

LGTM — minimal, well-scoped fix with regression tests; CI failures are unrelated flaky leak/http tests on macOS.

Extended reasoning...

Overview

This PR adds a 2-line else branch to Process__dispatchOnBeforeExit in src/jsc/bindings/BunProcess.cpp: when m_nextTickQueue hasn't been lazily initialized (no process.nextTick calls yet), it now calls vm.drainMicrotasks() directly so that async beforeExit listeners and queueMicrotask callbacks aren't silently dropped. Three new subprocess-based regression tests are added to the existing process.onBeforeExit describe block in test/js/node/process/process.test.js, following the same spawn/assert pattern as neighboring tests.

Security risks

None. This is purely an event-loop draining fix on the process shutdown path — no auth, crypto, permissions, or untrusted-input parsing is involved. The added call (vm.drainMicrotasks()) is the same primitive that JSNextTickQueue::drain already invokes when the queue exists and is empty, so the two arms are now symmetric rather than introducing novel behavior.

Level of scrutiny

Low-to-moderate. While process lifecycle is a sensitive area in principle, this change only adds behavior to a code path that previously did nothing (the missing else), and what it adds is exactly what the sibling branch already does. The blast radius is limited to programs that (a) register a beforeExit listener, (b) never call process.nextTick, and (c) queue microtasks from that listener — precisely the bug being fixed. The PR description includes a clear repro, root-cause analysis, and before/after test verification.

Other factors

• No CODEOWNERS apply to the touched files.
• The bug-hunting system found no issues.
• The 3 CI failures (fetch-leak.test.ts, node-http-uaf.test.ts, serve-body-leak.test.ts on macOS aarch64) are unrelated leak/http tests, not the new tests or anything touching beforeExit.
• No outstanding reviewer comments; only bot comments on the timeline.

[robobun]

Rebased onto 59242d6c (after #33466). The fix was simplified during the rebase: instead of adding an else { vm.drainMicrotasks(); } arm, the whole conditional block is replaced with globalObject->drainMicrotasks(), which is the same helper the main event-loop tick uses (it drains m_nextTickQueue when present, then vm.drainMicrotasks(), with the same exception routing as the tick path).

# with main's BunProcess.cpp
bun bd test test/js/node/process/process.test.js -t "drains microtasks|re-fires after a microtask"
  0 pass, 3 fail
# with this PR's BunProcess.cpp
bun bd test test/js/node/process/process.test.js -t onBeforeExit
  11 pass, 0 fail

CI on the current head (build 85032): test/js/node/process/process.test.js is green on every lane. The only error-level annotation so far is test/js/node/test/parallel/test-worker-message-port-transfer-terminate.js on debian-13-x64-asan (ASSERTION FAILED: !exception() in JSC's ExceptionScope.h:61, no stack trace). That test calls w.terminate(), which per src/jsc/web_worker.rs:1184 skips beforeExit entirely, and registers no beforeExit listener anywhere, so the if (fired) block this PR touches never runs for it. 60 local debug+ASAN runs (30 with this PR's BunProcess.cpp, 30 with main's) all pass.

Earlier build 83688 had only test/js/bun/http/serve.test.ts red (the request-body-backpressure EPIPE case), which is also red on main builds 83683 and 83238.

The diff is green and ready for a maintainer to merge.

[claude]

LGTM — minimal, well-scoped fix with matching tests.

Extended reasoning...

Overview

Two-line C++ change in Process__dispatchOnBeforeExit (src/jsc/bindings/BunProcess.cpp): when m_nextTickQueue has not been lazily created (i.e. process.nextTick was never called), fall through to vm.drainMicrotasks() so async continuations and queueMicrotask callbacks scheduled inside a beforeExit listener actually run. Three new subprocess tests in test/js/node/process/process.test.js cover async listeners, queueMicrotask, and event-loop re-entry via a post-await setTimeout.

Security risks

None. No user-controlled input parsing, no auth/crypto/permissions surface — this only adds a microtask drain call on a shutdown path.

Level of scrutiny

Low-to-medium. The change is tiny and mechanically consistent: JSNextTickQueue::drain already calls vm.drainMicrotasks() when the queue exists and is empty, so the new else arm just brings the uninitialized case to parity. The root cause is well explained and empirically confirmed (inserting a dummy process.nextTick made the repro pass on the old code).

Other factors

Tests follow the exact idiom of neighbors in the same describe block (spawn with -e, bunEnv, Promise.all on stdout/stderr/exited, assert combined object). PR description shows the tests fail on main and pass with the fix. CI was green aside from unrelated infra flakes (Docker Hub 429 on autobahn, artifact-download timeout). No CODEOWNERS coverage on the touched paths, no outstanding reviewer comments, and the bug-hunting system found nothing.

[claude]

LGTM — replacing the conditional nextTickQueue->drain() with globalObject->drainMicrotasks() is a strict superset of the old behavior and fixes the dropped-microtask case.

What was reviewed:

• Confirmed Zig::GlobalObject::drainMicrotasks() (ZigGlobalObject.cpp:3225) still drains m_nextTickQueue when present, then vm.drainMicrotasks(), with the same exception routing as the main tick path — so the existing nextTick-initialized case is preserved.
• Checked that the throwing-listener tests in the same describe block still exercise the exitDuringUncaughtException arming that happens before emit(); the if (fired) guard is unchanged.
• New tests follow the neighboring subprocess pattern (bunEnv, concurrent pipe drain, combined {stdout, stderr, exitCode} assertion) and cover async/await, queueMicrotask, and the re-entry loop via a microtask-armed timer.

Extended reasoning...

Overview

One-line change in Process__dispatchOnBeforeExit (src/jsc/bindings/BunProcess.cpp): the post-emit drain switches from a conditional m_nextTickQueue->drain() (only reached if process.nextTick had ever been called) to an unconditional globalObject->drainMicrotasks(). Three new subprocess tests are appended to the existing process.onBeforeExit describe block in test/js/node/process/process.test.js.

Security risks

None. This is process-shutdown microtask ordering; no untrusted input, auth, crypto, or resource-limit surfaces are touched.

Level of scrutiny

Low-to-moderate. The native change is tiny and delegates to an existing, widely-used helper. I read GlobalObject::drainMicrotasks() at ZigGlobalObject.cpp:3225–3271 to confirm it (a) still drains the nextTick queue when initialized, (b) always drains vm.drainMicrotasks() afterward, and (c) handles pending/termination exceptions via the same reportUncaughtExceptionAtEventLoop path as the main loop — so behavior for the previously-working nextTick case is preserved and the microtask case is now covered. The infinite-loop concern (microtask that queues another microtask keeping the process alive forever) is addressed in the PR description and matches Node semantics: bare microtasks don't set is_event_loop_alive(), so the outer re-entry loop doesn't spin.

Other factors

• Fail-before/pass-after was demonstrated for all three new tests, and the full onBeforeExit block (11 tests including the throwing-listener cases from #33466) passes on the fixed build.
• Tests use the file's existing conventions: Bun.spawn with bunExe()/-e, bunEnv, await using, Promise.all over stdout/stderr/exited, and a single combined-object equality assertion.
• CI red on the rebased head is the darwin-aarch64 artifact-download infra timeout also present on recent main builds; process.test.js is green on every lane that ran.
• No prior reviewer comments to address; Process__dispatchOnExit is intentionally left for a separate PR as noted in the description.

[claude]

LGTM — replaces the nextTick-only drain with globalObject->drainMicrotasks(), the same helper the main tick path uses.

What was reviewed:

• Confirmed Zig::GlobalObject::drainMicrotasks drains m_nextTickQueue when present and vm.drainMicrotasks(), with the same exception routing as before — the existing throwing-in-beforeExit tests still cover that path.
• Checked the re-fire test guards with if (n === 1) so a bare microtask can't infinite-loop, and asserts exact Node-matching output.
• Tests follow the sibling pattern in the same describe block (subprocess spawn, concurrent stdout/stderr/exited drain, combined-object assertion).

Extended reasoning...

Overview

Fixes microtasks queued from process.on('beforeExit', ...) listeners being dropped when process.nextTick() was never called. The 4-line conditional if (m_nextTickQueue) { ...->drain() } in Process__dispatchOnBeforeExit is replaced with a single globalObject->drainMicrotasks() call. Three new subprocess tests are added to test/js/node/process/process.test.js inside the existing process.onBeforeExit describe block.

Security risks

None. This is a microtask-checkpoint fix in the process shutdown path; no untrusted input is parsed and no new surface is exposed.

Level of scrutiny

Low-to-medium. The native change is one line and swaps in a strictly-more-comprehensive helper that is already the canonical drain path for the main event-loop tick (JSC__JSGlobalObject__drainMicrotasks). I read GlobalObject::drainMicrotasks in ZigGlobalObject.cpp:3225-3271 and confirmed it (a) drains the nextTick queue when present — preserving the old behavior — then (b) calls vm.drainMicrotasks(), and (c) routes any exception through reportUncaughtExceptionAtEventLoop, so the existing 'throwing inside beforeExit' tests remain covered. The Rust caller (VirtualMachine.rs:518-524) already wraps the dispatch in from_js_host_call_generic, so exception plumbing is unchanged.

Other factors

• Tests are placed in the correct existing file, mirror the sibling pattern exactly (await using proc, Promise.all([stdout.text(), stderr.text(), proc.exited]), single-object expect(...).toEqual({stdout, stderr, exitCode})), and cover three distinct entry points (async/await, queueMicrotask, Promise.then → timer re-arm).
• Verified fail-before/pass-after evidence in the PR body and rebase comment; CI build 83688 green on process.test.js across all lanes; the one darwin-x64 failure is a known pre-existing serve.test.ts flake unrelated to beforeExit.
• The if (fired) guard is retained, so no drain happens when there are no listeners — matches the original semantics.
• Process__dispatchOnExit is intentionally left untouched (called out in the PR body as separately tracked).

[robobun]

Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-07-29, it conflicts with main, and its last CI run failed. This is not a judgment on the fix itself. If the problem still reproduces on a current build, reopen this PR after a rebase or open a new one against main.