Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
16 changes: 12 additions & 4 deletions src/jsc/VirtualMachine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@
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 @@
/// 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 @@ -1513,7 +1513,15 @@
}
}

ExitHandler::dispatch_on_exit(self);
// Drain microtasks queued by the 'exit' listener only on a natural
// event-loop drain: not after a fatal error, and not when a worker
// reached here via terminate()/process.exit() (both set
// requested_terminate before shutdown() calls us).
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
let natural = self.unhandled_error_counter == 0
&& !self
.worker_ref()
.is_some_and(|w| w.has_requested_terminate());
ExitHandler::dispatch_on_exit(self, natural);

Check warning on line 1524 in src/jsc/VirtualMachine.rs

View check run for this annotation

Claude / Claude Code Review

natural gate misses exit_on_uncaught_exception fast-exit path

The `natural` gate misses the `exit_on_uncaught_exception` fast-exit path: when a `beforeExit` listener throws with no `uncaughtException` handler, `uncaught_exception()` at line 1434 calls `(hooks.process_exit)(global, 1)` → `vm.on_exit()` *before* line 1438's `unhandled_error_counter += 1` is reached, so `natural` computes `true` and microtasks drain — Node emits `'exit'` but does not drain on that path (verified v22.22.0). Same shape for the `process_exit(7)` branch at line 1412 and the `Bun_
Comment thread
robobun marked this conversation as resolved.
Outdated
self.is_shutting_down = true;

// Make sure we run new cleanup hooks introduced by running cleanup
Expand Down
16 changes: 12 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 @@ -300,7 +300,15 @@

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), so promise
// reactions and queueMicrotask callbacks queued by the listener run
// before termination. process.nextTick does not.
if (drainMicrotasks && !vm.hasTerminationRequest()) {
vm.drainMicrotasks();
}

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

View check run for this annotation

Claude / Claude Code Review

drainMicrotasks runs even after an 'exit' listener threw (handled by uncaughtException)

Edge case: when an `'exit'` listener throws *and* an `uncaughtException` handler is present, Bun's `EventEmitter::innerInvokeEventListeners` swallows the throw (routes it through `Bun__reportUnhandledError` → the handler → returns normally), so `emitter.emit("exit")` returns cleanly and the new `vm.drainMicrotasks()` runs — Node skips the checkpoint on that path (verified against v22/v26). Very narrow (needs an exit listener that both queues a microtask and throws, plus an uncaughtException hand
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 +851,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 +860,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 +883,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
2 changes: 1 addition & 1 deletion src/jsc/web_worker.rs
Original file line number Diff line number Diff line change
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
113 changes: 113 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,119 @@ 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 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