diff --git a/src/js/internal/fs/streams.ts b/src/js/internal/fs/streams.ts index 8547718d143b..ff92864e64cb 100644 --- a/src/js/internal/fs/streams.ts +++ b/src/js/internal/fs/streams.ts @@ -567,6 +567,16 @@ function emitDrain(stream: FSStream) { } } +// The sink rejects for a failed write. Node reports one on a tick (Writable's onwriteError), and +// no tick runs once 'exit' is being emitted: a write that fails there has no callback and no 'error'. +function reportSinkError(cb: ((err: any) => void) | undefined, err: any, stream?: FSStream) { + if (process._exiting) return; + if (cb) cb(err); + // Node.js onwriteError: callback AND destroy are both invoked; the + // callback is additive, not a replacement for the 'error' event. + if (stream) require("internal/streams/destroy").errorOrDestroy(stream, err); +} + function _write(data, encoding, cb) { const fileSink = this[kWriteStreamFastPath]; @@ -578,7 +588,7 @@ function _write(data, encoding, cb) { emitDrain(this); cb(null); }) - .catch(cb); + .catch(err => reportSinkError(cb, err)); return false; // Indicate backpressure } else { cb(null); @@ -623,10 +633,7 @@ function underscoreWriteFast(this: FSStream, data: any, encoding: any, cb: any) if (cb) cb(null); emitDrain(this); }, - err => { - if (cb) cb(err); - require("internal/streams/destroy").errorOrDestroy(this, err); - }, + err => reportSinkError(cb, err, this), ); return false; } else { @@ -691,12 +698,7 @@ function writeFast(this: FSStream, data: any, encoding: any, cb: any) { emitDrain(this); cb(null); }, - err => { - cb(err); - // Node.js onwriteError: callback AND destroy are both invoked; the - // callback is additive, not a replacement for the 'error' event. - require("internal/streams/destroy").errorOrDestroy(this, err); - }, + err => reportSinkError(cb, err, this), ); return false; // Indicate backpressure } else { @@ -734,7 +736,7 @@ writeStreamPrototype._writev = function (data, cb) { emitDrain(this); cb(null); }) - .catch(cb); + .catch(err => reportSinkError(cb, err)); return false; } else { cb(null); diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index 4d3a0115880f..11420e0c9a2d 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -352,6 +352,9 @@ pub struct VirtualMachine { pub on_print_error_zig_exception: Option, pub on_print_error_zig_exception_ctx: *mut c_void, pub(crate) is_handling_uncaught_exception: bool, + /// Advances on each error `uncaught_exception` takes, handled or not. `on_exit` compares it + /// around the 'exit' listeners: an error they threw ends the event. + uncaught_exception_epoch: u32, pub(crate) exit_on_uncaught_exception: bool, /// An earlier `--hot` run ended with an unhandled error. Its `beforeExit` listeners are still registered. hot_run_failed: bool, @@ -500,6 +503,8 @@ unsafe extern "C" { safe fn Process__dispatchOnBeforeExit(global: &JSGlobalObject, code: u8); safe fn Process__dispatchOnExit(global: &JSGlobalObject, code: u8); + safe fn Bun__Process__hasExitCode(global: &JSGlobalObject) -> bool; + safe fn Bun__Process__discardNextTicks(global: &JSGlobalObject); safe fn Bun__closeAllSQLiteDatabasesForTermination(global: &JSGlobalObject); safe fn Bun__closeAllNodeSqliteDatabasesForTermination(global: &JSGlobalObject); safe fn Bun__WebView__closeAllForTermination(); @@ -776,6 +781,8 @@ pub struct ExitHandler { /// `process.exit()`, a fatal error or the end of a `bun test` run, as opposed to the event loop running dry. /// See `VirtualMachine::exit_tears_down_napi_envs`. pub requested: bool, + /// `on_before_exit()` returned with the loop dry and no unhandled error. + pub loop_ran_dry: bool, } impl ExitHandler { @@ -823,14 +830,22 @@ 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(crate) fn dispatch_on_exit(vm: &VirtualMachine) { - let exit_code = vm.exit_handler.exit_code; + /// + /// Returns whether the event was emitted and nothing stopped its listeners. + pub(crate) fn dispatch_on_exit(vm: &VirtualMachine) -> bool { // `process.on('exit')` handlers are user script (see `on_exit`). - if vm.script_allowed() && !vm.exit_handler.skip_exit_listeners { - Process__dispatchOnExit(vm.global(), exit_code); + if !vm.script_allowed() || vm.exit_handler.skip_exit_listeners { + return false; } - if vm.worker.is_none() { - Bun__WebView__closeAllForTermination(); + let exit_code = vm.exit_handler.exit_code; + let global = vm.global(); + match jsc::from_js_host_call_generic(global, || Process__dispatchOnExit(global, exit_code)) + { + Ok(()) => true, + Err(err) => { + let _ = crate::task::report_error_or_terminate(global, err); + false + } } } @@ -2145,6 +2160,7 @@ impl VirtualMachine { if self.is_shutting_down() || !self.script_allowed() || err.is_termination_exception() { return true; } + self.uncaught_exception_epoch = self.uncaught_exception_epoch.wrapping_add(1); // An exception thrown by a Bun.ModuleGraph's module code is that graph's to // handle, ahead of the test runner and the thread-wide path. (A rejection @@ -2227,6 +2243,7 @@ impl VirtualMachine { } pub fn on_before_exit(&mut self) { + self.exit_handler.loop_ran_dry = false; // Node only emits 'beforeExit' on a natural drain; a fatal uncaught // exception that brought us here is an implicit process.exit(1). Arm // the hard-exit flag ourselves since we're skipping the dispatch that @@ -2269,6 +2286,7 @@ impl VirtualMachine { break; } + self.exit_handler.loop_ran_dry = self.unhandled_error_counter == 0; } /// Writes each configured profile once. The sampling report is written first because @@ -2327,7 +2345,22 @@ impl VirtualMachine { // Write profiles before any shutdown begins. self.write_profiles(); - ExitHandler::dispatch_on_exit(self); + // The loop ran dry: node emits this 'exit' through MakeCallback, which then runs the + // listeners' promise jobs and reports what they rejected, unless a listener threw. + // `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; + let uncaught_exception_epoch = self.uncaught_exception_epoch; + { + let _entered = ran_dry.then(|| self.enter_event_loop_scope_without_checkpoint()); + if ExitHandler::dispatch_on_exit(self) && ran_dry { + self.run_exit_listeners_jobs(uncaught_exception_epoch); + } + } + if self.is_main_thread() { + Bun__WebView__closeAllForTermination(); + } // process.exit() never reaches drain_microtasks; flush AutoFlusher sinks here. if !self.is_inside_deferred_task_queue.get() { @@ -2360,6 +2393,34 @@ impl VirtualMachine { } } + /// What node's `InternalCallbackScope::Close` (src/api/callback.cc) runs after the 'exit' + /// listeners of a loop that ran dry, unless one threw: the microtask checkpoint, then the + /// rejection pass. `&self` and never inlined: the listeners and the jobs change the fields + /// it compares through other references. + #[cold] + #[inline(never)] + fn run_exit_listeners_jobs(&self, uncaught_exception_epoch: u32) { + if self.uncaught_exception_epoch != uncaught_exception_epoch { + return; + } + let global = self.global(); + Bun__Process__discardNextTicks(global); + if self.event_loop_mut().drain_microtasks().is_err() { + return; + } + let unhandled_error_counter = self.unhandled_error_counter; + let _ = global.handle_rejected_promises(); + 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; + } + } + } + /// Whether `on_exit()` runs `RareData::cleanup_hooks`, i.e. `NapiEnv::cleanup` for /// each addon. Node does this when it frees an environment: a worker's on any exit, /// the main thread's only once its loop ran dry (`process.exit()` and a fatal error diff --git a/src/jsc/bindings/BunProcess.cpp b/src/jsc/bindings/BunProcess.cpp index 00e342dac979..1512927c23e6 100644 --- a/src/jsc/bindings/BunProcess.cpp +++ b/src/jsc/bindings/BunProcess.cpp @@ -874,6 +874,19 @@ extern "C" void Process__dispatchOnExit(Zig::GlobalObject* globalObject, uint8_t dispatchExitInternal(globalObject, process, exitCode); } +// `process.exitCode` is not undefined. +extern "C" bool Bun__Process__hasExitCode(Zig::GlobalObject* globalObject) +{ + return globalObject->hasProcessObject() && globalObject->processObject()->m_isExitCodeObservable; +} + +// A tick that process.nextTick() queued before 'exit' does not run after it. +extern "C" void Bun__Process__discardNextTicks(Zig::GlobalObject* globalObject) +{ + if (auto* nextTickQueue = globalObject->m_nextTickQueue.get()) + nextTickQueue->discard(globalObject->vm()); +} + JSC_DEFINE_HOST_FUNCTION(Process_functionUptime, (JSC::JSGlobalObject * lexicalGlobalObject, JSC::CallFrame* callFrame)) { double now = static_cast(Bun__readOriginTimer(bunVM(lexicalGlobalObject))); diff --git a/test/js/bun/repl/repl.test.ts b/test/js/bun/repl/repl.test.ts index 6f3ecb50cd83..632733ccb536 100644 --- a/test/js/bun/repl/repl.test.ts +++ b/test/js/bun/repl/repl.test.ts @@ -382,6 +382,19 @@ describe.concurrent("Bun REPL", () => { expect(exitCode).toBe(0); }); + // .exit is a requested exit, as process.exit() is: nothing runs after the 'exit' listeners. + test(".exit runs process.on('exit') listeners and not their promise jobs", async () => { + const { stdout, stderr, exitCode } = await runRepl([ + "process.on('exit', async () => { console.log('exit listener'); await null; console.log('its job'); }), 0", + ".exit", + ]); + expect({ afterLastPrompt: stripAnsi(stdout).split("> \n").at(-1), stderr, exitCode }).toEqual({ + afterLastPrompt: "exit listener\n", + stderr: "", + exitCode: 0, + }); + }); + test(".help shows help message", async () => { const { outputs, stderr, exitCode } = await runRepl([".help", ".exit"]); expect(outputs).toHaveLength(1); @@ -961,6 +974,44 @@ describe.concurrent("Bun REPL", () => { expect(exitCode).toBe(0); }); + // The loop ran dry, as at the end of a script: the promise jobs of the listeners run. + test("-e runs the promise jobs of process.on('exit') listeners", async () => { + const { stdout, stderr, exitCode } = await runReplWith([ + "-e", + "process.on('exit', async () => { console.log('exit listener'); await null; console.log('its job'); })", + ]); + expect({ stdout, stderr, exitCode }).toEqual({ stdout: "exit listener\nits job\n", stderr: "", exitCode: 0 }); + }); + + // Node reports a rejection that an 'exit' listener leaves and exits with the assigned process.exitCode, or with 1. + test.each([ + ["", 1], + ["process.exitCode = 2;", 2], + ["process.exitCode = 0;", 0], + ])("-e %j then an 'exit' listener that rejects: exit code %d", async (assignment, code) => { + const { stdout, stderr, exitCode } = await runReplWith([ + "-e", + `${assignment} process.on('exit', async () => { throw new Error('cleanup failed'); })`, + ]); + expect({ stdout, reported: stderr.includes("error: cleanup failed"), exitCode }).toEqual({ + stdout: "", + reported: true, + exitCode: code, + }); + }); + + // The run ended with an unhandled error, not with a dry loop: nothing follows the 'exit' listeners. + test("-e runs no promise job of an 'exit' listener after a rejection that 'beforeExit' work made", async () => { + const { stdout } = await runReplWith([ + "-e", + `process.once('beforeExit', () => { + setTimeout(() => { Promise.reject(new Error('cleanup failed')); setTimeout(() => {}, 20); }, 1); + }); + process.on('exit', async () => { console.log('exit listener'); await null; console.log('its job'); })`, + ]); + expect(stdout).toBe("exit listener\n"); + }); + test("-e drains event loop (timers fire before exit)", async () => { const { stdout, stderr, exitCode } = await runReplWith(["-e", "setTimeout(() => console.log('from timer'), 50)"]); expect(stdout).toBe("from timer\n"); diff --git a/test/js/node/process/process.test.js b/test/js/node/process/process.test.js index 781c8c37606b..ca795968aa38 100644 --- a/test/js/node/process/process.test.js +++ b/test/js/node/process/process.test.js @@ -3,7 +3,19 @@ import { CString, dlopen, ptr } from "bun:ffi"; import { memoryUsage as jscMemoryUsage } from "bun:jsc"; import { describe, expect, it } from "bun:test"; import { familySync } from "detect-libc"; -import { bunEnv, bunExe, isASAN, isDebug, isLinux, isMacOS, isRoot, isWindows, tempDir, tmpdirSync } from "harness"; +import { + bunEnv, + bunExe, + isASAN, + isDebug, + isLinux, + isMacOS, + isRoot, + isWindows, + nodeExeAtLeast, + tempDir, + tmpdirSync, +} from "harness"; import { basename, join, resolve } from "path"; import { getHeapStatistics } from "v8"; @@ -1244,6 +1256,249 @@ describe.concurrent(() => { expect(stderr).not.toInclude("error: boom"); expect(exitCode).toBe(0); }); + + // Node emits the 'exit' of an event loop that ran dry through MakeCallback: when the listeners + // have returned, their promise jobs run, and then what they rejected is reported. + // process.exit() and a fatal error emit 'exit' with a plain call: nothing runs after the listeners. + const out = `const out = s => require("node:fs").writeSync(1, s + " ");\n`; + const failed = `new Error("cleanup failed")`; + const asyncListener = `process.on("exit", async () => { out("a"); await null; out("b"); throw ${failed}; });`; + + async function run(cmd, options = {}) { + await using proc = Bun.spawn({ cmd, env: bunEnv, stdio: ["ignore", "pipe", "pipe"], ...options }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout, stderr, exitCode }; + } + + // [name, script, [what it writes, whether the error is reported, exit code], flags] + const afterDryLoop = [ + [ + "an async listener continues after each await", + `process.on("exit", async () => { out("a"); await null; out("b"); await null; out("c"); });`, + ["a b c ", false, 0], + ], + [ + "the jobs run when every listener has returned", + `process.on("exit", async () => { out("a1"); await null; out("a2"); }); + process.on("exit", () => { out("b"); queueMicrotask(() => out("b2")); });`, + ["a1 b a2 b2 ", false, 0], + ], + [ + "a listener that 'beforeExit' added", + `process.once("beforeExit", () => { + process.once("exit", async () => { out("a"); await null; out("b"); }); + });`, + ["a b ", false, 0], + ], + [ + "a listener that process.emit('exit') called before", + `process.on("exit", async () => { out("a"); await null; out("b"); }); + process.emit("exit", 0); + out("main");`, + ["a main b a b ", false, 0], + ], + [ + "a job assigns process.exitCode", + `process.on("exit", async code => { await null; out("was " + code); process.exitCode = 5; });`, + ["was 0 ", false, 5], + ], + [ + "a job calls process.exit()", + `process.on("exit", async () => { await null; out("a"); process.exit(3); });`, + ["a ", false, 3], + ], + ["a job throws", `process.on("exit", () => { queueMicrotask(() => { throw ${failed}; }); });`, ["", true, 1]], + ["an async listener throws", `process.on("exit", async () => { throw ${failed}; });`, ["", true, 1]], + [ + "a listener calls an async function that rejects", + `const save = async () => { throw ${failed}; }; process.on("exit", () => { save(); });`, + ["", true, 1], + ], + [ + "a rejection is reported after the jobs", + `process.on("exit", () => { Promise.reject(${failed}); }); + process.on("exit", () => { Promise.resolve().then(() => out("job")); });`, + ["job ", true, 1], + ], + [ + "a rejection that a job handles is not reported", + `process.on("exit", () => { + const rejected = Promise.reject(${failed}); + Promise.resolve().then(() => rejected.catch(e => out("caught " + e.message))); + });`, + ["caught cleanup failed ", false, 0], + ], + [ + "a rejection reaches an 'unhandledRejection' listener", + `process.on("unhandledRejection", e => out("unhandledRejection " + e.message)); + process.on("exit", async () => { throw ${failed}; });`, + ["unhandledRejection cleanup failed ", false, 0], + ], + [ + "a rejection under --unhandled-rejections=strict", + `process.on("exit", async () => { out("a"); throw ${failed}; });`, + ["a ", true, 1], + "--unhandled-rejections=strict", + ], + [ + "a rejection under --unhandled-rejections=throw reaches an 'uncaughtException' listener", + `process.on("uncaughtException", (e, origin) => out(origin)); + process.on("exit", async () => { throw ${failed}; });`, + ["unhandledRejection ", false, 0], + "--unhandled-rejections=throw", + ], + [ + "a rejection under --unhandled-rejections=warn-with-error-code", + `process.on("exit", async () => { out("a"); throw ${failed}; });`, + ["a ", false, 1], + "--unhandled-rejections=warn-with-error-code", + ], + [ + "a rejection under --unhandled-rejections=warn", + `process.on("exit", async () => { out("a"); throw ${failed}; });`, + ["a ", false, 0], + "--unhandled-rejections=warn", + ], + [ + "a rejection under --unhandled-rejections=none", + `process.on("exit", async () => { out("a"); throw ${failed}; });`, + ["a ", false, 0], + "--unhandled-rejections=none", + ], + [ + "a tick, a timer or an immediate that a listener or a job queued does not run", + `process.on("exit", () => { + out("a"); + process.nextTick(() => out("tick")); + setTimeout(() => out("timer"), 0); + setImmediate(() => out("immediate")); + queueMicrotask(() => { out("job"); process.nextTick(() => out("tick")); }); + });`, + ["a job ", false, 0], + ], + [ + "with no 'exit' listener, the job that an 'unhandledRejection' listener left", + `process.on("unhandledRejection", async () => { out("a"); await null; out("b"); process.exit(1); }); + setTimeout(() => { Promise.reject(${failed}); }, 1);`, + ["a b ", false, 1], + ], + ]; + const nothingAfter = [ + ["process.exit()", `${asyncListener} process.exit(0);`, ["a ", false, 0]], + [ + "an uncaught error", + `process.on("exit", async () => { out("a"); await null; out("b"); }); + setTimeout(() => { throw ${failed}; }, 1);`, + ["a ", true, 1], + ], + [ + "a rejection that a 'beforeExit' listener made", + `process.on("beforeExit", () => { Promise.reject(${failed}); }); + process.on("exit", async () => { out("a"); await null; out("b"); });`, + ["a ", true, 1], + ], + [ + "a listener that threw", + `process.on("uncaughtException", (e, origin) => out(origin)); + process.on("exit", () => { out("a"); queueMicrotask(() => out("job")); throw ${failed}; });`, + ["a uncaughtException ", false, 0], + ], + ]; + + const expectRow = async (_, script, [stdout, error, exitCode], ...flags) => { + expect(await run([bunExe(), ...flags, "-e", out + script])).toEqual({ + stdout, + stderr: error ? expect.stringContaining("error: cleanup failed") : "", + exitCode, + }); + }; + it.each(afterDryLoop)("when the loop ran dry: %s", expectRow); + it.each(nothingAfter)("nothing runs after the listeners of %s", expectRow); + + const node = nodeExeAtLeast("24.0.0"); + it.skipIf(!node)("node gives the same output", async () => { + const rows = [...afterDryLoop, ...nothingAfter]; + const results = await Promise.all( + rows.map(([, script, , ...flags]) => run([node, ...flags, "-e", out + script])), + ); + expect( + results.map(({ stdout, stderr, exitCode }) => [stdout, stderr.includes("cleanup failed"), exitCode]), + ).toEqual(rows.map(([, , expected]) => expected)); + }); + + it("the jobs run in an ES module and under --print", async () => { + using dir = tempDir("process-exit-jobs", { + "entry.mjs": `import { writeSync } from "node:fs"; + process.on("exit", async () => { writeSync(1, "a "); await null; writeSync(1, "b "); });`, + }); + const listener = `process.on("exit", async () => { out("a"); await null; out("b"); })`; + expect( + await Promise.all([ + run([bunExe(), join(String(dir), "entry.mjs")]), + run([bunExe(), "--print", `${out}${listener}, 1`]), + ]), + ).toEqual([ + { stdout: "a b ", stderr: "", exitCode: 0 }, + { stdout: "1\na b ", stderr: "", exitCode: 0 }, + ]); + }); + + // What runs a vendored node test: the file ends when its loop runs dry. `node --test` fails this file too. + it("a listener that rejects fails a node:test file whose loop ran dry", async () => { + using dir = tempDir("process-exit-jobs-test", { + "exit.test.js": `const { test } = require("node:test"); + ${out} + process.on("exit", async () => { out("a"); await null; out("b"); throw ${failed}; }); + test("passes", () => {});`, + }); + expect( + await run([bunExe(), "test", "./exit.test.js"], { + cwd: String(dir), + env: { ...bunEnv, BUN_TEST_DRAIN_EVENT_LOOP: "1" }, + }), + ).toEqual({ + stdout: expect.stringContaining("a b "), + stderr: expect.stringContaining("error: cleanup failed"), + exitCode: 1, + }); + }); + + // A tick never runs once 'exit' is emitted. Node runs this one before the event. + it("a tick queued before 'exit' does not run after its listeners", async () => { + const { stdout, stderr, exitCode } = await run([ + bunExe(), + "-e", + `${out} + process.on("unhandledRejection", () => { out("unhandledRejection"); process.nextTick(() => out("tick")); }); + setTimeout(() => { Promise.reject(${failed}); }, 1); + process.on("exit", async () => { out("a"); await null; out("b"); });`, + ]); + expect({ end: stdout.slice(-4), stderr, exitCode }).toEqual({ end: "a b ", stderr: "", exitCode: 0 }); + }); + + // Node reports a failed write on a tick, which never runs once 'exit' is emitted. + it.each([`process.stdout.write("late\\n")`, `process.stdout.end("late\\n")`])( + "%s in a listener to a closed pipe is not an error", + async write => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `process.stdout.write("ready\\n"); + process.stdin.on("end", () => {}).resume(); + process.on("exit", () => { ${write}; });`, + ], + env: bunEnv, + stdio: ["pipe", "pipe", "pipe"], + }); + const reader = proc.stdout.getReader(); + await reader.read(); + await reader.cancel(); + proc.stdin.end(); + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + expect({ stderr, exitCode }).toEqual({ stderr: "", exitCode: 0 }); + }, + ); }); it("process.memoryUsage", () => {