Skip to content

Run the promise jobs of 'exit' listeners and report their rejections when the event loop runs dry - #46758

Draft
robobun wants to merge 1 commit into
mainfrom
robobun/005a3c01/exit-listener-microtasks
Draft

robobun wants to merge 1 commit into
mainfrom
robobun/005a3c01/exit-listener-microtasks

Conversation

@robobun

@robobun robobun commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • When the event loop runs dry, Bun runs nothing after the 'exit' listeners. An async listener stops at its first await.
  • process.on("exit", async () => { throw new Error("x") }) prints nothing and exits 0. Node prints the error and exits 1.
  • VirtualMachine::on_exit runs no microtask checkpoint and no rejection pass after the 'exit' dispatch. A differential run against Node found it. No user reported it.

Fix

Background

  • Node emits this 'exit' through MakeCallback. Its scope close (InternalCallbackScope::Close) runs the promise jobs, then reports rejections. process.exit() emits with a plain call.
  • A promise job is the code after an await.
  • Considered: a drain in Run::start (misses bun repl -e) and a flag on the C++ dispatch (closed process: drain microtasks after emitting 'exit' on natural termination #34910).

Downsides

  • An 'exit' listener that rejects, or leaves a job that throws, now prints and exits 1 (before: silent, 0). Node exits 0 in two such cases (Notes).
  • An 'exit' listener whose promise jobs never end (for (;;) await 0) now never exits, as in Node. Before: exit 0.
  • Every natural exit costs +263 instructions (gdb single-step, release) and no syscall. .text grows by 1,024 bytes.
Notes

Report. Found by a differential run against node v26.3.0. There is no user issue for it. Each line is a whole CommonJS program (out = s => require("fs").writeSync(1, s + " ")).

program node v26.3.0 main this PR
process.on("exit", async () => { out("a"); await null; out("b"); }) a b, exit 0 a, exit 0 a b, exit 0
process.on("exit", async () => { throw new Error("cleanup failed") }) error, exit 1 silent, exit 0 error, exit 1
const save = async () => { throw new Error("cleanup failed") }; process.on("exit", () => { save() }) error, exit 1 silent, exit 0 error, exit 1
the first program followed by process.exit(0) a, exit 0 a, exit 0 a, exit 0

What the change does, in order. All of it is in VirtualMachine::on_exit and its cold helper run_exit_listeners_jobs.

  1. on_before_exit records in ExitHandler.loop_ran_dry that the loop ran dry with no unhandled error. The gate is main thread && loop_ran_dry && !requested. requested is already set by process.exit(), a fatal error and the end of a bun test run.
  2. The 'exit' dispatch and the jobs run inside enter_event_loop_scope_without_checkpoint(). A native call that a listener or a job makes is then not the outermost event-loop entry, so it does not run the next job before it returns.
  3. If a listener threw (uncaught_exception ran during the dispatch), the event ends there. Node does the same (if (failed_) return; in InternalCallbackScope::Close).
  4. The helper discards the tick queue, runs the checkpoint, then runs handle_rejected_promises() once.
  5. If the pass reported an error that nothing handled, the exit is a failed one: requested is set (no Node-API environment teardown, as after any fatal error) and the exit code is 1 unless process.exitCode was assigned.
  6. src/js/internal/fs/streams.ts: the four places that report a failed FileSink write do nothing once process._exiting is set. Node's Writable reports a failed write on a tick, and a tick never runs after 'exit'.

Node source (v26.3.0). src/api/hooks.cc:49-81 (EmitProcessExitInternal: set_exiting(true), ProcessEmit), src/node_process_events.cc:48-62 (ProcessEmit is a MakeCallback), src/api/callback.cc:133-208 (InternalCallbackScope::Close), src/api/embed_helpers.cc:23-78 (SpinEventLoopInternal), lib/internal/process/task_queues.js:113-117 (nextTick returns when process._exiting), lib/internal/process/per_thread.js:234-241 (process.exit() emits with a plain call). Node's documentation for 'exit' says that listeners must only do synchronous work. The table above is the observed behavior of v26.3.0.

Differential against node v26.3.0 (release builds of the merge base and of this PR, 62 main-thread programs, order of the output tokens ignored):

  • 42 programs differ on main, 10 with this PR.
  • The programs that end with process.exit() or with a fatal error print the same on both builds.
  • 24 worker programs print the same on both builds.

Differences from Node that remain.

Relation to open pull requests. Landing order: #45575, #45597 and #45582 first, then this PR, rebased. A trial merge of the pushed commit with the heads of #45582 and #45597 is clean. #45575 conflicts with main in src/jsc/web_worker.rs, a file this PR does not touch. This PR does not change the lines of run_command.rs that #45575 changes. #43741 holds the event loop entered across the 'beforeExit', 'exit' and error listeners. This PR does the same for the 'exit' of a dry loop only. #42032 owns the exit code of an error during 'exit'. #32228 owns a replaced process.emit. #41588 owns an 'exit' listener that throws under bun test.

Earlier attempts. #34910 added the drain behind a flag on Process__dispatchOnExit. A stale sweep closed it with no maintainer comment. #30601 carried the drain as one of three parts and closed as a duplicate. #34354 moved the stdio write completion to a tick. Its failure arm is what the guard in streams.ts covers at exit.

Self-review. 18 concerns were raised against the first version.

  • Addressed in code or tests: the gate no longer asks whether a listener was called, ticks are discarded before the checkpoint, the event loop scope, requested and the exit code of a reported error, the failed stdio write, the missing --unhandled-rejections rows, a bun test row, a run of every row under real Node, the comments, and the loop_ran_dry condition.
  • Addressed by leaving workers unchanged: terminate() and a worker's 'exit' jobs, Node-API finalizers queued during a worker's 'exit', a worker's pending ESM entry, the close event of a Web Worker, the bun lint engine VM, and two worker test rows with a racing order.
  • Kept as known differences: jobs queued before 'exit', await import(), 'rejectionHandled'.
  • Not followed at that time: the review suggested to stack this on Report unhandled rejections made in the event loop's last turn #45582 and to land it after process: 'exit' listeners see exit code 1 after an unhandled rejection #45575. The second self-review showed that this order is needed (next section). It also suggested to deliver a failed stdio write on a tick. That changes the error from an unhandled rejection to an uncaught exception in ordinary turns, so it is not part of this PR.

Second self-review (2026-10-24): defects in the pushed commit, and the plan. A code review on this PR raised 8 findings. A second self-review of the answer raised 35 concerns. Its result: the 'exit' close is right, but this PR must be cut to that one concern and must land after the three open pull requests that change the same exit sequence. Every row below is from release builds and from node v26.3.0. "error" means that the error is printed.

program node v26.3.0 main pushed commit
process.exitCode = 2, an 'exit' listener rejects (bun run) error, 2 silent, 2 error, 1
process.exitCode = 2, an 'exit' listener leaves a job that throws error, 2 silent, 2 error, 1
a replaced process.emit that assigns process.exitCode on 'exit' (signal-exit does this), a listener rejects error, 0 silent, 0 error, 1
an 'uncaughtException' listener, an 'exit' listener rejects the listener takes it, 0 silent, 0 error, 1
--print of a pending promise that an 'exit' listener settles before it awaits, logs job and throws Promise { <pending> }, job, error, 1 Promise { <pending> }, 0 Promise { <pending> }, 42, 0
bun test with BUN_TEST_DRAIN_EVENT_LOOP=1, an 'exit' listener leaves a job that throws the job runs, silent, 0 error, 0

The plan:

  1. Rebase on main when process: 'exit' listeners see exit code 1 after an unhandled rejection #45575, Run the ticks and microtasks that 'unhandledRejection' listeners queue #45597 and Report unhandled rejections made in the event loop's last turn #45582 have merged. process: 'exit' listeners see exit code 1 after an unhandled rejection #45575 stores the exit code of a reported rejection before on_exit, so row 1 is then right with no change to run_command.rs here.
  2. The rule for row 2 (a fatal error during 'exit' keeps the assigned process.exitCode) is not part of this PR. uncaught_exception has three stores of the literal 1. Report a loop turn's rejections before the loop-alive check, and keep the exit code when an 'exit' listener throws #42032 changes all three, and fix(process): allow clearing exitCode #42607 makes process.exitCode = undefined clear the code under it. I built the rule at one store, and it regressed another program: process.exitCode = 0, an earlier unhandled rejection and a throwing 'exit' listener exited 0 (Node and main: 1). The rule is only right when the rejection's exit code is stored before 'exit'.
  3. Row 3 needs process: emit 'beforeExit' and 'exit' through a replaced process.emit #32228, which calls a replaced process.emit. The order of the two pull requests must be agreed, and the row goes into the table.
  4. Row 5: the two --print reactions (Bun__onResolveEntryPointResult, Bun__onRejectEntryPointResult) call Global::exit. The new checkpoint runs them inside on_exit. They must not print or exit once the pending promise is printed.
  5. Row 6: the helper takes its mark of unhandled_error_counter before the checkpoint. The gate in on_exit also requires unhandled_error_counter == 0, for an error that bun test reports after on_before_exit. Both are built and tested locally.
  6. New rows: row 4 (pinned as a known difference), a listener that calls socket.destroy() (the event loop scope), two worker baselines, a Node-API row for requested, and a check in the child that the pipe is closed in the two closed-pipe tests.
  7. dispatch_on_exit returns nothing again: its result is always true where on_exit reads it. Bun__Process__discardNextTicks goes if nothing needs it after Run the ticks and microtasks that 'unhandledRejection' listeners queue #45597.

Measurements. Release builds of the merge base (179664e) and of this PR. bloaty, valgrind, pahole, strace and perf are not available on the machine. I used size, nm, objdump, gdb single-stepping with scheduler locking, and qemu-x86_64 -strace.

what main this PR
.text 82,164,617 82,165,641 (+1,024)
.rodata, .data, .bss, file size unchanged
new symbols 1 (run_exit_listeners_jobs, 557 bytes, cold)
exit sequence, no process object (on_before_exit to global_exit) 1,002 instructions 1,265 (+263)
exit sequence, process object, no listener 2,333 2,596 (+263)
process.exit(0) (on_exit to global_exit) 956 980 (+24)
the helper alone, nothing queued 227 instructions, no allocator function entered
10,000 chained setImmediate: checkpoints / JSC drains / rejection passes 20000 / 20002 / 10002 20001 / 20003 / 10003
the same, ending in process.exit(0) 19999 / 20001 / 10001 19999 / 20001 / 10001
syscalls, globalThis.x = 1 286 286
syscalls, process.on("exit", () => {}) 289 289
size_of::<VirtualMachine>() 32,320 32,320
uncaught_exception 133 instructions 134

Instruction counts are identical in three runs each. Heap blocks are not measured (no dhat).

Release builds differ from debug builds here. A first version of the helper took &mut self and compared unhandled_error_counter around the rejection pass. The optimizer folded the comparison, because the pass changes the field through another reference. bun repl -e then exited 0 in release and 1 in debug. The helper takes &self, is never inlined, and does both comparisons itself.

Suites.

  • Debug and ASAN build: the new block in process.test.js (31 pass, five runs), repl.test.ts (135 pass), worker_threads.test.ts (161 pass), filesink.test.ts (83 pass), process-stdio.test.ts (18 pass), tty.test.ts (8 pass). With BUN_JSC_validateExceptionChecks=1: the exit tests of process.test.js and repl.test.ts pass with no unchecked exception.
  • The full process.test.js on the debug build: should be the node version on the host that we expect fails (the host has node v26.3.0). Two signal tests time out at 5 s under load and pass alone. They do the same with the test file of main.
  • Release builds, main against this PR: 3,662 vendored Node tests under bun run and 111 node:test files under bun test end with the same exit code. 262 Bun test files show stable differences only in the two files with new tests. Five other files differed once and flip on both builds.
  • The new rows also run under the host's Node when it is 24 or later (node gives the same output).

…se jobs and a rejection pass

When the main thread's event loop ran dry, VirtualMachine::on_exit ends
the 'exit' event the way node's MakeCallback does: after the listeners
return it runs the microtask checkpoint and one rejection pass. An error
that the pass reports ends the process with the assigned process.exitCode
or with 1. process.exit(), a fatal error and a listener that threw still
run nothing after the listeners.

A tick queued before 'exit' is discarded first: process.nextTick() queues
nothing once the event is emitted, and no tick runs after it.

A FileSink write that fails once 'exit' is emitted is not reported: node
reports a failed write on a tick.
@robobun

robobun commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 11:21 AM PT - Oct 24th, 2026

✅ @robobun, your commit d7afdd6654519df74ff2204b7c5c6d025f462e45 passed in Build #132080! 🎉


🧪   To try this PR locally:

bunx bun-pr 46758

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

bun-46758 --bun

@robobun

robobun commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status

Reproduction, with node v26.3.0, main at 179664e and this branch:

// p.cjs
const out = s => require("node:fs").writeSync(1, s + " ");
process.on("exit", async () => { out("a"); await null; out("b"); throw new Error("cleanup failed"); });
  • node p.cjs prints a b and the error. The exit code is 1.
  • bun p.cjs on main prints a. The exit code is 0.
  • bun p.cjs on this branch prints a b and the error. The exit code is 1.

The new tests are in test/js/node/process/process.test.js and test/js/bun/repl/repl.test.ts. On a release build of main, 20 of the 29 new cases in the first file and 4 of the 6 in the second file fail.

Reviewed: this PR stays open as a draft. It must land after #45575, #45597 and #45582. A second self-review found defects in the pushed commit. The description lists them with the plan.

Pull request: #46758

@coderabbitai

coderabbitai Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 179664e and d7afdd6.

Files selected for processing: 5
  • src/js/internal/fs/streams.ts
  • src/jsc/VirtualMachine.rs
  • src/jsc/bindings/BunProcess.cpp
  • test/js/bun/repl/repl.test.ts
  • test/js/node/process/process.test.js

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

Walkthrough

Exit handling now records whether the event loop drained without unhandled errors and can process eligible jobs after exit listeners. File sink rejection handling skips error reporting during process exit. Tests cover exit-listener jobs, errors, exit codes, and stream writes during exit.

Changes

Natural exit processing

Layer / File(s) Summary
Dry-loop state and exit dispatch
src/jsc/VirtualMachine.rs
ExitHandler records whether the pre-exit drain completed without unhandled errors. Exit dispatch returns whether listeners completed and reports caught listener errors.
Conditional exit-listener jobs
src/jsc/VirtualMachine.rs, src/jsc/bindings/BunProcess.cpp
After a natural dry loop, the main-thread exit path can discard queued next ticks, drain microtasks, and check rejected promises. An uncaught-exception epoch guards the job pass.
Sink rejection handling and exit tests
src/js/internal/fs/streams.ts, test/js/bun/repl/repl.test.ts, test/js/node/process/process.test.js
File sink rejection handlers use a shared helper that skips error reporting while the process is exiting. Tests cover listener jobs, rejection handling, exit codes, and writes to a closed pipe.

Suggested reviewers: alii, cirospaciari

Priority: Normal

Severity of issue fixed: Low

Merge Risk: Minimal · up to d7afd

The exit-behavior changes are ready for normal validation; no identified issue requires a fix before merging.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly states the primary change: running promise jobs for exit listeners and reporting their rejections after the event loop runs dry.
Linked Issues check Passed No active directly linked issue remains. Issue #34910 is closed and provides historical context only. Therefore, this pull request has no linked-issue coding requirements to assess.
Out of Scope Changes check Passed The changes stay within the stated scope. VirtualMachine::on_exit implements the natural main-thread exit job and rejection pass. BunProcess.cpp provides the required queue and exit-code helpers. …
Description check Passed The description is complete and relevant. It explains the problem, the fix, scope, known differences, risks, and verification results. Although it does not use the exact template headings, it provides…
Full details: Out of Scope Changes check

Explanation

The changes stay within the stated scope. VirtualMachine::on_exit implements the natural main-thread exit job and rejection pass. BunProcess.cpp provides the required queue and exit-code helpers. streams.ts preserves silent failed FileSink writes during exit. The added tests cover these behaviors. No unrelated change is shown.

Full details: Description check

Explanation

The description is complete and relevant. It explains the problem, the fix, scope, known differences, risks, and verification results. Although it does not use the exact template headings, it provides the required information for what the PR does and how it was verified.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings marked 🟡 are optional suggestions and need no follow-up push.

Comment thread src/jsc/VirtualMachine.rs
Comment on lines +2416 to +2420
let exit_handler = &mut self.as_mut().exit_handler;
exit_handler.requested = true;
if !Bun__Process__hasExitCode(global) {
exit_handler.exit_code = 1;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Under bun run, a script that assigns process.exitCode = N and has an 'exit' listener whose promise rejects now exits 1 instead of N (base: silent, exit N; Node: error printed, exit N). The new pass at src/jsc/VirtualMachine.rs:2412 reports the rejection through Run::on_unhandled_rejection_before_close, which sets ANY_UNHANDLED, and print_unhandled_version_note at src/runtime/cli/run_command.rs:1821 then stores exit_code = 1 after on_exit, so the Bun__Process__hasExitCode branch at :2418 is overridden for this entry point. Fix: keep the assigned code on every entry point that runs the new pass, e.g. have the post-on_exit store in run_command.rs respect Bun__Process__hasExitCode.

Why this was flagged

A CommonJS or ESM script run with bun file.js does process.exitCode = 2; process.on("exit", async () => { throw new Error("cleanup failed"); }) and lets its loop run dry. Run::start (src/runtime/cli/run_command.rs:1708) installs Run::on_unhandled_rejection_before_close and calls vm.on_exit() at :1714. on_exit computes ran_dry true (src/jsc/VirtualMachine.rs:2352) and run_exit_listeners_jobs drains the listener's job and runs handle_rejected_promises() at :2412; unhandled_rejection_owned bumps unhandled_error_counter and calls the handler at :4641-4642, which sets ANY_UNHANDLED at run_command.rs:1379. Back in run_exit_listeners_jobs, Bun__Process__hasExitCode is true, so exit_code stays 2 (:2418-2420). Then Run::start at :1716 sees ANY_UNHANDLED and print_unhandled_version_note at :1821 stores exit_code = 1 before global_exit(). The user gets exit status 1; on the base branch the rejection was never reported and the process exited 2; Node v26 prints the error and exits 2.

Verification: Then Run::start :1716-1717 sees ANY_UNHANDLED and calls print_unhandled_version_note, whose first line is vm.exit_handler.exit_code = 1; (:1821), and global_exit exits with self.exit_handler.exit_code (VirtualMachine.rs:2472) = 1. So the hasExitCode branch at :2418-2420 is dead under the bun run entry point, and the assigned exit code regresses from N to 1 for this case.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed. process.exitCode = 2 with an 'exit' listener that rejects exits 1 under bun run (Node v26.3.0: 2, main: 2 and silent).

The store that replaces the code is the one in print_unhandled_version_note, after on_exit. #45575 moves that store before on_exit. I built a local change that kept the store and passed a flag for an error that is reported during on_exit. A self-review showed two problems with it. It conflicts with every line that #45575 changes there. And the rule behind it is only right when the exit code of a rejection is stored before 'exit'.

So this PR does not change run_command.rs. It is a draft now. It will be rebased on main after #45575 merges, and this row goes into the test table then. The description has the row and the plan.

Comment thread src/jsc/VirtualMachine.rs
Comment on lines +2413 to +2420
if self.unhandled_error_counter != unhandled_error_counter {
// A fatal error in node: it exits with the assigned `process.exitCode` or with 1,
// and frees no environment.
let exit_handler = &mut self.as_mut().exit_handler;
exit_handler.requested = true;
if !Bun__Process__hasExitCode(global) {
exit_handler.exit_code = 1;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Users who assign process.exitCode and whose 'exit' listener leaves a job that throws now exit 1 instead of the assigned code (base: assigned code, silent; Node: assigned code, error printed). The "assigned process.exitCode, or 1" rule at src/jsc/VirtualMachine.rs:2418 only covers the rejection pass. A throwing microtask in drain_microtasks (2408), or a rejection under --unhandled-rejections=throw or strict, goes through uncaught_exception, which calls process_exit(global_object, 1) at 2219 and overwrites exit_code with 1 at src/runtime/node/node_process.rs:71. Fix: once 'exit' is being emitted, every fatal error reported after the listeners must keep the assigned exitCode, e.g. have the fatal path at 2219 pass exit_handler.exit_code when Bun__Process__hasExitCode is true.

Why this was flagged

Trigger: bun -e 'process.exitCode = 2; process.on("exit", () => { queueMicrotask(() => { throw new Error("x") }) })', entered through Run::start -> on_exit -> run_exit_listeners_jobs. At src/jsc/VirtualMachine.rs:2408 drain_microtasks runs the job; its throw reaches ZigGlobalObject::drainMicrotasks's endedByException -> reportUncaughtExceptionAtEventLoop (src/jsc/bindings/ZigGlobalObject.cpp:1236) -> uncaught_exception. No 'uncaughtException' listener, exit_on_uncaught_exception is already true (set by Process__dispatchOnBeforeExit at src/jsc/bindings/BunProcess.cpp:855), so src/jsc/VirtualMachine.rs:2219 calls process_exit(global_object, 1); Bun__Process__exit at src/runtime/node/node_process.rs:71 stores exit_code = 1 and never returns, so the Bun__Process__hasExitCode check at 2418 is never reached. Under --unhandled-rejections=throw or strict the rejection pass takes the same uncaught_exception route (4615, 4629) with the same result. Result: error printed, exit code 1. On the base branch no job runs and the process exits with the assigned 2.

Verification: src/jsc/VirtualMachine.rs:2408 drain_microtasks() runs the job; its throw reaches uncaught_exception. With no 'uncaughtException' listener and exit_on_uncaught_exception already true, :2219 calls process_exit(global_object, 1), which stores exit_code = 1 (src/runtime/node/node_process.rs:71) and never returns, so the Bun__Process__hasExitCode rule at :2419-2421 is never reached. On base the job never ran, so the process exited with the assigned code.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed. The exit code is 1 where Node exits 2, for a job that throws and for a rejection under --unhandled-rejections=throw or strict.

I built the fix at the fatal arm of uncaught_exception: pass the assigned code once 'exit' is being emitted. These rows then matched Node, and another program regressed. process.exitCode = 0, an earlier unhandled rejection and an 'exit' listener that throws exited 0 (Node and main: 1). The cause is that bun run stores the exit code of a reported rejection only after on_exit. uncaught_exception also has two more stores of the literal 1 that the change did not cover, so the code depended on how the exit was reached.

#42032 carries this rule for all three stores, and it needs #45575 first. The rule is not part of this PR anymore. Until it lands, a job that throws exits 1, as a listener that throws does on main. The description lists the row.

Comment thread src/jsc/VirtualMachine.rs
Comment on lines +2408 to +2411
if self.event_loop_mut().drain_microtasks().is_err() {
return;
}
let unhandled_error_counter = self.unhandled_error_counter;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 (optional) Under bun test with BUN_TEST_DRAIN_EVENT_LOOP=1, an 'exit' listener job that throws is printed but the file still exits 0, while a job that rejects exits 1. run_exit_listeners_jobs snapshots unhandled_error_counter at src/jsc/VirtualMachine.rs:2411 only after drain_microtasks, so an error the checkpoint reported through the bun:test branch of uncaught_exception (counter bumped, no exit) never sets requested or the exit code. Fix: take the counter snapshot before Bun__Process__discardNextTicks and the drain, so every error reported by the checkpoint or the rejection pass marks the exit failed and sets code 1 unless process.exitCode is assigned. In bun run the same throw hard-exits 1 via exit_on_uncaught_exception, so only the bun:test path is affected.

Why this was flagged

A node:test file run as bun test ./x.test.js with BUN_TEST_DRAIN_EVENT_LOOP=1 containing process.on("exit", () => { queueMicrotask(() => { throw new Error("x") }) }); test("passes", () => {}). src/runtime/cli/test_command.rs:2739 leaves exit_handler.requested false in drain mode and on_exit (src/jsc/VirtualMachine.rs:2357) calls run_exit_listeners_jobs. drain_microtasks at :2408 runs the job; JSC reports the throw through uncaught_exception, where the isBunTest branch at :2172 bumps unhandled_error_counter, prints via jest::on_unhandled_rejection, and returns true without exiting. The snapshot at :2411 is taken after that, so the compare at :2413 sees no change: requested stays false and exit_code keeps the value computed at test_command.rs:2736. Base never runs the job (nothing printed, exit 0); this PR prints error: x and still exits 0, while the sibling rejection case exits 1 through the same function. The exit_on_uncaught_exception safeguard at :2209 does not apply because the isBunTest check at :2172 returns first.

Verification: nit — triggered when a node:test file run under bun test with BUN_TEST_DRAIN_EVENT_LOOP=1 has an 'exit' listener whose promise job throws. The isBunTest branch at src/jsc/VirtualMachine.rs:2172-2176 does unhandled_error_counter += 1 and returns without touching the exit code; the snapshot at :2411 is taken AFTER that bump, so neither requested nor exit_code = 1 is set. The error prints but the file exits 0; base already exits 0.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. The helper will take its mark of unhandled_error_counter before it discards the ticks and runs the checkpoint. I built and tested it locally: a node:test file whose 'exit' listener leaves a job that throws then exits 1, as with a rejection.

It is not pushed yet. The PR is a draft until it can be rebased on #45575, #45597 and #45582, and this change comes with that push.

Comment thread src/jsc/VirtualMachine.rs
Comment thread src/js/internal/fs/streams.ts
Comment thread src/jsc/VirtualMachine.rs
// `process.exit()` and a fatal error emit it with a plain call. Not in a worker:
// `WebWorker::shutdown` has unpublished its VM, so `terminate()` could not stop a job.
let ran_dry =
self.is_main_thread() && self.exit_handler.loop_ran_dry && !self.exit_handler.requested;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 (optional) Under bun test with BUN_TEST_DRAIN_EVENT_LOOP=1, a node:test file whose last loop turn left an unhandled rejection still gets the post-'exit' jobs drain and rejection pass, and if that pass reports anything the Node-API cleanup hooks are skipped. src/jsc/VirtualMachine.rs:2353 computes ran_dry from loop_ran_dry, which on_before_exit set at src/runtime/cli/test_command.rs:3070 before the rejection pass at test_command.rs:3075 bumped unhandled_error_counter, and requested stays false via exit_is_requested(). Fix: treat a loop that ended with unhandled_error_counter > 0 as not dry at on_exit time (re-check the counter at 2353 or set requested when the counter is nonzero, as run_command.rs:1711 does), so every entry point agrees.

Why this was flagged

Trigger: bun test on a node:test file with BUN_TEST_DRAIN_EVENT_LOOP=1, where the file's final turn rejects a promise nobody handles. test_command.rs:3070 calls on_before_exit, which stores loop_ran_dry = (unhandled_error_counter == 0) at src/jsc/VirtualMachine.rs:2289 while the counter is still 0. test_command.rs:3075 then runs handle_rejected_promises, which under isBunTest increments the counter at VirtualMachine.rs:4558. test_command.rs:2739 sets requested = exit_is_requested() = false in this mode. on_exit at VirtualMachine.rs:2353 therefore sees ran_dry = true although the loop ended with a reported error, which the run_command.rs:1711 path explicitly treats as a fatal (non-dry) exit. Lines 2407-2412 then run the 'exit' listeners' jobs and a second rejection pass; a new report sets requested = true at 2417, which makes exit_tears_down_napi_envs() false so run_cleanup_hooks is skipped — on the base branch this file ran the addon cleanup hooks.

Verification: Trigger: bun test with BUN_TEST_DRAIN_EVENT_LOOP=1 on a file whose final turn leaves an unhandled rejection. test_command.rs:3070 calls vm.on_before_exit(), which stores loop_ran_dry while the counter is still 0; :3075 then bumps the counter. So on_exit computes ran_dry = true (VirtualMachine.rs:2353) and runs run_exit_listeners_jobs. Contrast run_command.rs:1709-1713, which sets requested = true.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. on_exit will also require unhandled_error_counter == 0, so that a loop that ended with a reported error is not dry on any entry point. I built and tested it locally. It comes with the rebase push (the PR is a draft until then).

One observation for the test. Under bun test, main already runs the jobs of the 'exit' listeners after on_exit returns, and their errors are silent. So the test asserts what is reported, not what the job prints.

Comment thread src/jsc/VirtualMachine.rs
Comment thread src/jsc/bindings/BunProcess.cpp
@robobun

robobun commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

An earlier cut of this change is on robobun/3533b2ca/exit-listener-microtasks (7964cac, never opened as a PR). It ran the jobs only when a listener was called, and it repeated the drain and the rejection pass until both were empty. This PR supersedes it.

One result from the review of that cut applies to the suggestion to repeat the drain after the rejection pass (the thread on src/jsc/VirtualMachine.rs:2412). With that loop, this program never exits. It exits 0 on Node v26.3.0, on main, and on this PR's head (d7afdd6, debug build):

let closed = false;
process.on("unhandledRejection", async () => {
  await null;
  if (closed) throw new Error("resource used after the exit listener closed it");
  console.log("continuation ran before exit");
});
process.on("exit", () => { closed = true; });
Promise.reject(new Error("boom"));

The continuation of the 'unhandledRejection' listener is still queued when 'exit' is emitted. It runs after the 'exit' listener and throws, and the rejection goes back to the same listener. A loop repeats that with no end (several million listener calls in 10 s on a release build of the earlier cut). The single pass in this PR ends it.

I also added one line to Downsides here: a listener whose promise jobs never end (for (;;) await 0) now never exits, as in Node. I ran it on a debug build of d7afdd6 (3 of 3 runs killed at 8 s), on Node v26.3.0 (never exits) and on Bun 1.4.3 (exit 0).

@robobun
robobun marked this pull request as draft October 25, 2026 00:06
@robobun

robobun commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

A second self-review changes the state of this PR. I converted it to a draft.

@dylan-conway @Jarred-Sumner two questions:

  1. No user reported this. Do you want the change?
  2. If yes, is this order right: process: 'exit' listeners see exit code 1 after an unhandled rejection #45575, Run the ticks and microtasks that 'unhandledRejection' listeners queue #45597 and Report unhandled rejections made in the event loop's last turn #45582 first, then this PR rebased on main?

I will do the rework that the description lists when the order is settled.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants