diff --git a/src/jsc/PosixSignalHandle.rs b/src/jsc/PosixSignalHandle.rs index 4a1390268b1f..c9dedc295c6e 100644 --- a/src/jsc/PosixSignalHandle.rs +++ b/src/jsc/PosixSignalHandle.rs @@ -1,4 +1,4 @@ -use core::sync::atomic::{AtomicU8, AtomicU16, Ordering}; +use core::sync::atomic::{AtomicBool, AtomicU8, AtomicU16, Ordering}; use crate::event_loop::EventLoop; use crate::{JSGlobalObject, Task, VirtualMachineRef as VirtualMachine}; @@ -17,6 +17,12 @@ pub struct PosixSignalHandle { tail: AtomicU16, /// Consumer index (main thread reads). head: AtomicU16, + + /// Set when a termination-class signal (SIGHUP/SIGINT/SIGQUIT/SIGTERM) has + /// been delivered to a user handler. The `--watch`/`--hot` run-loop reads + /// this to exit once the event loop drains instead of blocking in the + /// watcher keep-alive forever; a plain `bun run` already exits on drain. + termination_requested: AtomicBool, } impl Default for PosixSignalHandle { @@ -25,10 +31,24 @@ impl Default for PosixSignalHandle { signals: [const { AtomicU8::new(0) }; BUFFER_SIZE as usize], tail: AtomicU16::new(0), head: AtomicU16::new(0), + termination_requested: AtomicBool::new(false), } } } +/// Termination-class signals convey "shut down" (the default disposition of +/// each is to terminate the process). A `--watch`/`--hot` session exits once +/// the event loop drains after one of these is routed to a user handler, +/// matching a plain `bun run`. Signals like SIGWINCH or SIGUSR1/2 do not imply +/// shutdown and are deliberately excluded. +fn is_termination_signal(signal: u8) -> bool { + use bun_core::SignalCode; + signal == SignalCode::SIGHUP as u8 + || signal == SignalCode::SIGINT as u8 + || signal == SignalCode::SIGQUIT as u8 + || signal == SignalCode::SIGTERM as u8 +} + impl PosixSignalHandle { // `pub const new = bun.TrivialNew(@This());` #[allow(dead_code)] @@ -40,6 +60,12 @@ impl PosixSignalHandle { /// Returns `true` if enqueued successfully, or `false` if the ring is full. #[allow(dead_code)] pub(crate) fn enqueue(&self, signal: u8) -> bool { + // Record a shutdown request before anything can short-circuit (a full + // ring drops the signal but the intent to terminate still stands). + if is_termination_signal(signal) { + self.termination_requested.store(true, Ordering::Release); + } + // Read the current tail and head (Acquire to ensure we have up-to-date values). let old_tail = self.tail.load(Ordering::Acquire); let head_val = self.head.load(Ordering::Acquire); @@ -93,6 +119,13 @@ impl PosixSignalHandle { Some(signal) } + /// True once a termination-class signal has been delivered to a user + /// handler (see [`is_termination_signal`]). + #[allow(dead_code)] + pub(crate) fn termination_requested(&self) -> bool { + self.termination_requested.load(Ordering::Acquire) + } + /// Drain as many signals as possible and enqueue them as tasks in the event loop. /// Called by the main thread. #[allow(dead_code)] diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index 99b9524070f6..c5e7fb08095f 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -1056,6 +1056,14 @@ impl VirtualMachine { || !el.next_immediate_tasks.is_empty() } + /// True once a termination-class signal (SIGHUP/SIGINT/SIGQUIT/SIGTERM) has + /// been delivered to a user handler. The `--watch`/`--hot` run-loop exits + /// once the event loop drains after such a signal, matching a plain + /// `bun run`. + pub fn termination_signal_requested(&self) -> bool { + self.event_loop_shared().termination_signal_requested() + } + pub fn wakeup(&mut self) { self.event_loop_mut().wakeup(); } diff --git a/src/jsc/event_loop.rs b/src/jsc/event_loop.rs index eefced1e6374..4d8979a942b7 100644 --- a/src/jsc/event_loop.rs +++ b/src/jsc/event_loop.rs @@ -526,6 +526,22 @@ impl EventLoop { self.concurrent_ref.load(Ordering::SeqCst) > 0 } + /// True once a termination-class signal (SIGHUP/SIGINT/SIGQUIT/SIGTERM) has + /// been delivered to a user handler via the POSIX signal ring. The + /// `--watch`/`--hot` run-loop reads this to exit once the event loop drains. + pub fn termination_signal_requested(&self) -> bool { + #[cfg(unix)] + { + self.signal_handler + .map(|h| h.termination_requested()) + .unwrap_or(false) + } + #[cfg(not(unix))] + { + false + } + } + pub fn run_imminent_gc_timer(&mut self) { // The real `WTFTimer` lives in `bun_runtime` (cycle), so the body // dispatches through `__bun_run_wtf_timer` (link-time extern). diff --git a/src/runtime/cli/run_command.rs b/src/runtime/cli/run_command.rs index a98d7bce7f27..ec67f7f67abe 100644 --- a/src/runtime/cli/run_command.rs +++ b/src/runtime/cli/run_command.rs @@ -1571,6 +1571,16 @@ impl Run { } vm.on_before_exit(); vm.report_exception_in_hot_reloaded_module_if_needed(); + // A termination signal (SIGINT/SIGTERM/SIGHUP/SIGQUIT) was + // routed to a user handler and the event loop has since drained + // (the `while` above only exits when nothing keeps it alive). + // Exit like a plain `bun run` instead of blocking in the watcher + // keep-alive below. Otherwise a handler that cleans up (e.g. + // `server.stop()`) could never let the process exit under + // --watch/--hot. + if vm.termination_signal_requested() && !vm.is_event_loop_alive() { + break; + } // SAFETY: `event_loop` is a self-pointer into this VM; uniquely // accessed here. Watcher arm keeps the process alive across // reloads. diff --git a/src/runtime/cli/test_command.rs b/src/runtime/cli/test_command.rs index 150537464980..0a4438c34352 100644 --- a/src/runtime/cli/test_command.rs +++ b/src/runtime/cli/test_command.rs @@ -2963,14 +2963,22 @@ impl TestCommand { } fn run_event_loop_for_watch(vm: &mut VirtualMachine) { - vm.event_loop_ref().tick_possibly_forever(); - loop { while vm.is_event_loop_alive() { vm.tick(); vm.event_loop_ref().auto_tick_active(); } + // A termination signal (SIGINT/SIGTERM/SIGHUP/SIGQUIT) was routed to + // a user handler (e.g. from a preload) and the event loop has since + // drained. Exit through the normal post-watch path instead of + // blocking in the watcher keep-alive again. The guard precedes the + // only blocking call so a signal delivered mid-run (before this + // function is entered) can't park on the keep-alive timer. + if vm.termination_signal_requested() && !vm.is_event_loop_alive() { + return; + } + vm.event_loop_ref().tick_possibly_forever(); } } diff --git a/test/cli/watch/watch.test.ts b/test/cli/watch/watch.test.ts index 7674ea7b1d29..3004b9561a92 100644 --- a/test/cli/watch/watch.test.ts +++ b/test/cli/watch/watch.test.ts @@ -1,7 +1,7 @@ import type { Subprocess } from "bun"; import { spawn } from "bun"; -import { afterEach, expect, it } from "bun:test"; -import { bunEnv, bunExe, isBroken, isWindows, tmpdirSync } from "harness"; +import { afterEach, describe, expect, it } from "bun:test"; +import { bunEnv, bunExe, isBroken, isWindows, tempDir, tmpdirSync } from "harness"; import { rmSync } from "node:fs"; import { join } from "node:path"; @@ -46,3 +46,169 @@ for (const dir of ["dir", "©️"]) { afterEach(() => { watchee?.kill(); }); + +// https://github.com/oven-sh/bun/issues/32400 +// A custom SIGINT handler that cleans up a ref'd resource used to hang the +// --watch/--hot run-loop forever: the handler ran, the event loop drained, but +// the watcher kept the process alive. It should exit like a plain `bun run` +// (and like `node --watch`) once the loop drains after the signal. +describe.each(["--watch", "--hot"])("%s exits on SIGINT after the handler cleans up", flag => { + it.skipIf(isWindows)("issue #32400", async () => { + using dir = tempDir("watch-sigint", { + "serve.ts": ` + const server = Bun.serve({ port: 0, fetch() { return new Response("OK"); } }); + process.on("SIGINT", async () => { + await server.stop(); + console.log("CLEANED_UP"); + }); + console.log("READY"); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "run", flag, "serve.ts"], + cwd: String(dir), + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + // Drain stderr concurrently so the watch banner can't fill the pipe. + const stderrDone = proc.stderr.text(); + + // Wait until the server is up and the SIGINT handler is installed. + const reader = proc.stdout.getReader(); + const decoder = new TextDecoder(); + let stdout = ""; + let ready = false; + while (!ready) { + const { done, value } = await reader.read(); + if (done) break; + stdout += decoder.decode(value, { stream: true }); + if (stdout.includes("READY")) ready = true; + } + reader.releaseLock(); + expect(ready).toBe(true); + + process.kill(proc.pid, "SIGINT"); + + // On the fixed build the handler stops the server, the loop drains and the + // process exits. On the buggy build the --watch/--hot loop blocks forever, + // so this await hangs and the test times out (the fail-before state). The + // handler caught SIGINT, so the exit is clean (code 0, no signalCode), not + // a signal kill. + const exitCode = await proc.exited; + const stderr = await stderrDone; + + // Surface stderr (watch banner + any crash output) if the process didn't + // exit cleanly, so a regression shows the cause rather than just a code. + if (exitCode !== 0) { + expect(stderr).toBe(""); + } + expect(proc.signalCode).toBe(null); + expect(exitCode).toBe(0); + }); +}); + +// https://github.com/oven-sh/bun/issues/32400 (same class, `bun test --watch`) +// A preload that installs a custom SIGINT handler and cleans up a ref'd +// resource used to hang the test-watch keep-alive loop after Ctrl+C, the same +// way `bun run --watch` did. It should exit once the loop drains. +it.skipIf(isWindows)("bun test --watch exits on SIGINT after the handler cleans up", async () => { + using dir = tempDir("test-watch-sigint", { + "setup.ts": ` + const server = Bun.serve({ port: 0, fetch() { return new Response("ok"); } }); + process.on("SIGINT", async () => { + await server.stop(); + console.log("CLEANED_UP"); + }); + `, + "noop.test.ts": ` + import { test, expect } from "bun:test"; + test("noop", () => { expect(1).toBe(1); }); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "test", "--watch", "--preload", "./setup.ts", "./noop.test.ts"], + cwd: String(dir), + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + // `bun test` prints results to stderr; the first run finishing ("Ran ...") + // means the watcher is now idle. Drain both streams so neither pipe blocks, + // and resolve readiness when the marker appears (or the stream ends early). + const stdoutDone = proc.stdout.text(); + const decoder = new TextDecoder(); + let stderr = ""; + const { promise: ranReady, resolve: onRan } = Promise.withResolvers(); + const stderrDone = (async () => { + const reader = proc.stderr.getReader(); + try { + while (true) { + const { done, value } = await reader.read(); + if (done) break; + stderr += decoder.decode(value, { stream: true }); + if (stderr.includes("Ran ")) onRan(); + } + } finally { + reader.releaseLock(); + onRan(); + } + })(); + + await ranReady; + expect(stderr).toContain("Ran "); + + process.kill(proc.pid, "SIGINT"); + + // On the buggy build the test-watch loop blocks forever here; the handler + // caught SIGINT, so a clean exit is code 0 with no signalCode. + const exitCode = await proc.exited; + await Promise.all([stdoutDone, stderrDone]); + + if (exitCode !== 0) { + expect(stderr).toBe(""); + } + expect(proc.signalCode).toBe(null); + expect(exitCode).toBe(0); +}); + +// Same class, but for a SIGINT delivered *during* the test run (before the +// watcher loop is entered): the handler runs and the loop drains first, so the +// loop must not park on the keep-alive timer when it is finally entered. +// Sending the signal from inside a test makes the timing deterministic. +it.skipIf(isWindows)("bun test --watch exits on a SIGINT delivered during the run", async () => { + using dir = tempDir("test-watch-sigint-midrun", { + "setup.ts": `process.on("SIGINT", () => { console.log("GOT_SIGINT"); });`, + "sigint.test.ts": ` + import { test } from "bun:test"; + test("sends SIGINT to itself mid-run", () => { + process.kill(process.pid, "SIGINT"); + }); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "test", "--watch", "--preload", "./setup.ts", "./sigint.test.ts"], + cwd: String(dir), + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + // On the buggy build the leading keep-alive call parks here: the signal was + // already handled and the loop already drained, so nothing wakes it. A clean + // exit is code 0 with no signalCode. Drain both pipes alongside `exited` so a + // full pipe can't stall the child. + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + if (exitCode !== 0) { + expect(stderr).toBe(""); + } + expect(stdout).toContain("GOT_SIGINT"); + expect(proc.signalCode).toBe(null); + expect(exitCode).toBe(0); +}); diff --git a/test/internal/dead-code-escape-limits.json b/test/internal/dead-code-escape-limits.json index c07d361a97a0..1f8a6f4dd5d1 100644 --- a/test/internal/dead-code-escape-limits.json +++ b/test/internal/dead-code-escape-limits.json @@ -12,7 +12,7 @@ "src/install/lockfile/Package.rs": 1, "src/io/lib.rs": 2, "src/io/posix_event_loop.rs": 8, - "src/jsc/PosixSignalHandle.rs": 6, + "src/jsc/PosixSignalHandle.rs": 7, "src/jsc_macros/lib.rs": 2, "src/opaque/lib.rs": 5, "src/patch/lib.rs": 2,