Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions src/jsc/VirtualMachine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ unsafe extern "C" {
safe fn Bun__emitHandledPromiseEvent(global: &JSGlobalObject, promise: JSValue) -> bool;

safe fn Process__dispatchOnBeforeExit(global: &JSGlobalObject, code: u8);
safe fn Process__dispatchOnExit(global: &JSGlobalObject, code: u8);
safe fn Process__dispatchOnExit(global: &JSGlobalObject, code: u8, drain_microtasks: bool);
safe fn Bun__closeAllSQLiteDatabasesForTermination();
safe fn Bun__closeAllNodeSqliteDatabasesForTermination(global: &JSGlobalObject);
safe fn Bun__WebView__closeAllForTermination();
Expand Down Expand Up @@ -503,9 +503,9 @@ impl ExitHandler {
/// parent via `container_of` would escape the provenance of `&mut self`
/// (which only covers the `ExitHandler` field). Callers pass the VM
/// reference instead; the body re-enters JS so no `&mut` is held.
pub fn dispatch_on_exit(vm: &VirtualMachine) {
pub fn dispatch_on_exit(vm: &VirtualMachine, drain_microtasks: bool) {
let exit_code = vm.exit_handler.exit_code;
Process__dispatchOnExit(vm.global(), exit_code);
Process__dispatchOnExit(vm.global(), exit_code, drain_microtasks);
if vm.worker.is_none() {
Bun__closeAllSQLiteDatabasesForTermination();
Bun__closeAllNodeSqliteDatabasesForTermination(vm.global());
Expand Down Expand Up @@ -1492,7 +1492,7 @@ impl VirtualMachine {
}
}

pub fn on_exit(&mut self) {
pub fn on_exit(&mut self, natural: bool) {
// Write CPU profile if profiling was enabled - do this FIRST before any
// shutdown begins. Grab the config and null it out to make this
// idempotent.
Expand All @@ -1513,7 +1513,9 @@ impl VirtualMachine {
}
}

ExitHandler::dispatch_on_exit(self);
// Node drains microtasks after 'exit' only on a natural event-loop
// drain: callers on explicit/fatal paths pass `natural = false`.
ExitHandler::dispatch_on_exit(self, natural && self.unhandled_error_counter == 0);
self.is_shutting_down = true;

// Make sure we run new cleanup hooks introduced by running cleanup
Expand Down
15 changes: 11 additions & 4 deletions src/jsc/bindings/BunProcess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@
return release;
}

static void dispatchExitInternal(JSC::JSGlobalObject* globalObject, Process* process, int exitCode)
static void dispatchExitInternal(JSC::JSGlobalObject* globalObject, Process* process, int exitCode, bool drainMicrotasks)
{
if (process->m_isExiting)
return;
Expand All @@ -301,6 +301,13 @@
MarkedArgumentBuffer arguments;
arguments.append(jsNumber(exitCode));
emitter.emit(event, arguments);

// Node performs a final microtask checkpoint after emitting 'exit' on a
// natural drain (not process.exit() or fatal exception); process.nextTick
// is not drained.
if (drainMicrotasks && !vm.hasTerminationRequest()) {
vm.drainMicrotasks();
}

Check warning on line 310 in src/jsc/bindings/BunProcess.cpp

View check run for this annotation

Claude / Claude Code Review

Worker: process.exit() inside natural-drain 'exit' listener still drains microtasks

Sibling to the listener-throws gap above: in a **worker** on natural drain, calling `process.exit()` from inside the `'exit'` listener still reaches `vm.drainMicrotasks()`. `WebWorker::shutdown()` nulls `self.vm` (step 1) *before* `vm.on_exit(true)` (step 2), so the re-entrant `process.exit()` → `worker.exit()` reads `self.vm_ptr()` as null and skips `notify_need_termination()` (its own comment documents this) — control returns here with the trap never armed. Node v26 skips the checkpoint on tha
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
}

JSC_DEFINE_CUSTOM_SETTER(Process_defaultSetter, (JSC::JSGlobalObject * globalObject, JSC::EncodedJSValue thisValue, JSC::EncodedJSValue value, JSC::PropertyName propertyName))
Expand Down Expand Up @@ -843,7 +850,7 @@
}
}

extern "C" void Process__dispatchOnExit(Zig::GlobalObject* globalObject, uint8_t exitCode)
extern "C" void Process__dispatchOnExit(Zig::GlobalObject* globalObject, uint8_t exitCode, bool drainMicrotasks)
{
if (!globalObject->hasProcessObject()) {
return;
Expand All @@ -852,7 +859,7 @@
auto* process = globalObject->processObject();
if (exitCode > 0)
process->m_isExitCodeObservable = true;
dispatchExitInternal(globalObject, process, exitCode);
dispatchExitInternal(globalObject, process, exitCode, drainMicrotasks);
}

JSC_DEFINE_HOST_FUNCTION(Process_functionUptime, (JSC::JSGlobalObject * lexicalGlobalObject, JSC::CallFrame* callFrame))
Expand All @@ -875,7 +882,7 @@
RETURN_IF_EXCEPTION(throwScope, {});

auto exitCode = Bun__getExitCode(bunVM(zigGlobal));
Process__dispatchOnExit(zigGlobal, exitCode);
Process__dispatchOnExit(zigGlobal, exitCode, false);

// process.reallyExit(exitCode);
auto reallyExitVal = process->get(globalObject, Identifier::fromString(vm, "reallyExit"_s));
Expand Down
4 changes: 2 additions & 2 deletions src/jsc/web_worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1258,7 +1258,7 @@ impl WebWorker {
// re-sets it for the JSC VM teardown.
vm.jsc_vm().clear_has_termination_request();
vm.is_shutting_down = true;
vm.on_exit();
vm.on_exit(!self.has_requested_terminate());
Comment thread
robobun marked this conversation as resolved.
if let Some(hooks) = runtime_hooks() {
(hooks.cron_clear_all_teardown)(vm);
// Drain `TimeoutObject`s from this worker's timer heap before
Expand Down Expand Up @@ -1587,7 +1587,7 @@ fn on_unhandled_rejection(
// they may change process.exitCode). Run them before arming termination — a pending
// termination exception makes dispatchExitInternal skip 'exit' (as terminate() should),
// and its processIsExiting guard stops shutdown() from running them twice.
virtual_machine::ExitHandler::dispatch_on_exit(vm);
virtual_machine::ExitHandler::dispatch_on_exit(vm, false);
let _ = worker.set_requested_terminate();
// Do NOT call `worker.shutdown()` here —
// `shutdown()` RETURNS, so calling it here would destroy
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/bake/production.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ pub fn build_command(ctx: Context) -> crate::Result<()> {
if vm.exit_handler.exit_code == 0 {
vm.exit_handler.exit_code = 1;
}
vm.on_exit();
vm.on_exit(false);
vm.global_exit();
}
Err(e) => return Err(e),
Expand Down
4 changes: 2 additions & 2 deletions src/runtime/cli/repl_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@
vm.print_error_like_object_to_console(exception);
}
vm.exit_handler.exit_code = 1;
vm.on_exit();
vm.on_exit(false);
vm.global_exit();
}

Expand All @@ -252,7 +252,7 @@
}

// Clean up
vm.on_exit();
vm.on_exit(true);

Check warning on line 255 in src/runtime/cli/repl_command.rs

View check run for this annotation

Claude / Claude Code Review

REPL eval error path passes natural=true to on_exit()

The `had_error` eval branch (and the interactive-error branch) fall through to `vm.on_exit(true)` here, but `repl.eval_script()` reports the exception via `print_js_error_to()` without going through `uncaught_exception()`, so `unhandled_error_counter` is still 0 and the `natural && unhandled_error_counter == 0` gate passes — microtasks drain after `'exit'` on an error exit. That's inconsistent with the sibling error-path callers this PR updated (line 229 above, `run_command.rs:1710`) and with de
Comment thread
robobun marked this conversation as resolved.
Outdated
vm.global_exit();
}

Expand Down
4 changes: 2 additions & 2 deletions src/runtime/cli/run_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1638,7 +1638,7 @@ impl Run {

vm.on_unhandled_rejection = Run::on_unhandled_rejection_before_close;
vm.global().handle_rejected_promises();
vm.on_exit();
vm.on_exit(true);

if ANY_UNHANDLED.load(Ordering::Relaxed) {
print_unhandled_version_note(vm);
Expand Down Expand Up @@ -1707,7 +1707,7 @@ fn dump_build_error(vm: &mut VirtualMachine) {
)]
fn exit_with_unhandled_note(vm: &mut VirtualMachine) -> ! {
vm.exit_handler.exit_code = 1;
vm.on_exit();
vm.on_exit(false);
if ANY_UNHANDLED.load(Ordering::Relaxed) {
bun_sourcemap::SavedSourceMap::MissingSourceMapNoteInfo::print();
pretty_errorln!("<r>\n<d>{}<r>", Global::unhandled_error_bun_version_string,);
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/node/node_process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ pub extern "C" fn exit(global_object: &JSGlobalObject, code: u8) {
// instead to terminate the worker sooner
worker.exit();
} else {
vm.on_exit();
vm.on_exit(false);
vm.global_exit();
}
}
Expand Down
133 changes: 133 additions & 0 deletions test/js/node/process/process.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -712,6 +712,139 @@ describe.concurrent(() => {
});

describe("process.onExit", () => {
it("drains microtasks queued by the listener on natural exit", async () => {
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`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");
});`,
],
env: bunEnv,
stdio: ["inherit", "pipe", "pipe"],
});
const [stderr, stdout, exitCode] = await Promise.all([proc.stderr.text(), proc.stdout.text(), proc.exited]);
expect({ stdout, stderr, exitCode }).toEqual({
stdout: "exit-listener\npt-in-exit\nqm-in-exit\nafter-await-in-exit\n",
stderr: "",
exitCode: 0,
});
});

it("does not drain microtasks queued by the listener on process.exit()", async () => {
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`process.on("exit", () => {
console.log("exit-listener");
queueMicrotask(() => console.log("qm-in-exit"));
});
process.exit(0);`,
],
env: bunEnv,
stdio: ["inherit", "pipe", "pipe"],
});
const [stderr, stdout, exitCode] = await Promise.all([proc.stderr.text(), proc.stdout.text(), proc.exited]);
expect({ stdout, stderr, exitCode }).toEqual({
stdout: "exit-listener\n",
stderr: "",
exitCode: 0,
});
});

it("does not drain microtasks queued by the listener after a fatal uncaught exception", async () => {
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`process.on("exit", () => {
console.log("exit-listener");
queueMicrotask(() => console.log("qm-in-exit"));
});
throw new Error("boom");`,
],
env: bunEnv,
stdio: ["inherit", "pipe", "pipe"],
});
const [stderr, stdout, exitCode] = await Promise.all([proc.stderr.text(), proc.stdout.text(), proc.exited]);
expect(stderr).toInclude("error: boom");
expect(stdout).toBe("exit-listener\n");
expect(exitCode).toBe(1);
});

it("does not drain microtasks queued by the listener when a beforeExit listener throws", async () => {
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`process.on("beforeExit", () => { throw new Error("boom"); });
process.on("exit", () => {
console.log("exit-listener");
queueMicrotask(() => console.log("qm-in-exit"));
});`,
],
env: bunEnv,
stdio: ["inherit", "pipe", "pipe"],
});
const [stderr, stdout, exitCode] = await Promise.all([proc.stderr.text(), proc.stdout.text(), proc.exited]);
expect(stderr).toInclude("error: boom");
expect(stdout).toBe("exit-listener\n");
expect(exitCode).toBe(1);
});

it("does not drain process.nextTick queued by the listener on natural exit", async () => {
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`process.on("exit", () => {
console.log("exit-listener");
process.nextTick(() => console.log("nt-in-exit"));
queueMicrotask(() => console.log("qm-in-exit"));
});`,
],
env: bunEnv,
stdio: ["inherit", "pipe", "pipe"],
});
const [stderr, stdout, exitCode] = await Promise.all([proc.stderr.text(), proc.stdout.text(), proc.exited]);
expect({ stdout, stderr, exitCode }).toEqual({
stdout: "exit-listener\nqm-in-exit\n",
stderr: "",
exitCode: 0,
});
});

it("drains microtasks queued by the listener on a worker's natural exit", async () => {
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`const { Worker } = require("worker_threads");
Comment thread
coderabbitai[bot] marked this conversation as resolved.
const w = new Worker(\`
process.on("exit", () => {
console.log("exit-listener");
queueMicrotask(() => console.log("qm-in-exit"));
});
\`, { eval: true });
w.on("exit", () => console.log("worker-done"));`,
],
env: bunEnv,
stdio: ["inherit", "pipe", "pipe"],
});
const [stderr, stdout, exitCode] = await Promise.all([proc.stderr.text(), proc.stdout.text(), proc.exited]);
expect({ stdout, stderr, exitCode }).toEqual({
stdout: "exit-listener\nqm-in-exit\nworker-done\n",
stderr: "",
exitCode: 0,
});
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.

it("throwing inside preserves exit code", async () => {
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", `process.on("exit", () => {throw new Error("boom")});`],
Expand Down
Loading