watch: exit on SIGINT when a custom signal handler drains the event loop by robobun · Pull Request #32405 · oven-sh/bun

robobun · 2026-06-16T15:16:44Z

Repro

// serve.ts
const server = Bun.serve({ port: 0, fetch() { return new Response("OK"); } });
process.on("SIGINT", async () => {
  await server.stop();
  console.log("cleaned up");
});

bun run --watch serve.ts
# press Ctrl+C -> handler runs, server stops, but the process never exits

Cause

On POSIX, bun run --watch/--hot run the script in-process with an infinite run-loop in src/runtime/cli/run_command.rs so the watcher can keep the process alive between reloads:

loop {
    while vm.is_event_loop_alive() { vm.tick(); vm.auto_tick_active(); }
    vm.on_before_exit();
    vm.event_loop_ref().tick_possibly_forever(); // blocks, then loops again
}

When a script calls process.on("SIGINT", ...), Bun installs its own sigaction that routes the signal to the JS event loop instead of terminating. The handler runs and drains the loop (e.g. server.stop()), but the outer loop just goes back to tick_possibly_forever(). The only escape was an explicit process.exit(). Without a custom handler the default disposition terminates the process, so this never came up.

A plain bun run exits in the same scenario, and so does node --watch, so the watcher hanging is a compatibility gap.

Fix

Record when a termination-class signal (SIGHUP/SIGINT/SIGQUIT/SIGTERM) has been delivered to a handler (an AtomicBool on PosixSignalHandle, set in enqueue). After the inner event loop drains (so the handler and any async cleanup it started have run), the watch loop checks the flag and breaks to the normal exit path instead of blocking again:

if vm.termination_signal_requested() && !vm.is_event_loop_alive() {
    break;
}

The process then exits via the usual path with process.exitCode (0 by default), matching bun run and node --watch.

bun test --watch has the same keep-alive loop (run_event_loop_for_watch in test_command.rs), so a preload that installs a SIGINT handler and cleans up would hang it the same way. The identical guard is applied there too, returning through the normal post-watch exit path.

The flag is intentionally sticky: once a termination signal has been delivered, the process exits the next time the event loop drains. That matches the intent of the signal and is strictly more conservative than a plain bun run, which exits on any drain. A handler that keeps the loop alive (doesn't clean up) is unaffected, since the loop never drains and the break is never taken. Non-termination signals (SIGWINCH, SIGUSR1/2, and so on) are excluded. The flag lives behind #[cfg(unix)]; on Windows the accessor returns false, so behavior there is unchanged.

Relation to #30597

#30597 targeted the same root cause (for #13539/#9871) but was opened the day before #30412 ("Rewrite Bun in Rust") landed, so its edits are in the .zig files that are now reference-only and no longer compiled. This PR implements the fix on the live Rust path. It also differs on exit code: #30597 exits 128+signal, whereas a caught-and-cleaned-up signal should exit 0 (what bun run and node --watch do), which is what this PR produces.

Verification

New tests in test/cli/watch/watch.test.ts cover bun run --watch, bun run --hot, and bun test --watch (preload) with a custom SIGINT handler that stops a server. Each hangs until timeout on the baked bun and exits with code 0 on this change.

(pass) should watch files [7385ms]
(pass) should watch files (non-ascii path) [7518ms]
(pass) --watch exits on SIGINT after the handler cleans up > issue #32400 [650ms]
(pass) --hot exits on SIGINT after the handler cleans up > issue #32400 [676ms]
(pass) bun test --watch exits on SIGINT after the handler cleans up [720ms]
(pass) bun test --watch exits on a SIGINT delivered during the run [621ms]

I also reproduced #13539 (Bun.spawn children + --watch + a SIGINT handler that kills them): it hangs on the baked bun and exits cleanly (code 0) on this change, so it is fixed by the same mechanism. #5624 was suggested by the issue bot, but its repro has no custom SIGINT handler, so the default disposition already terminates it (exit 130) both before and after this change; it is a different scenario and is not closed here.

Under `bun run --watch` and `--hot`, the run-loop blocks in tick_possibly_forever() after the event loop drains so the watcher can keep the process alive for reloads. When a script installs a custom SIGINT handler, the signal is routed to JS instead of terminating the process; the handler runs, cleans up its resources (e.g. server.stop()), the loop drains, and then the process hangs forever. Only an explicit process.exit() escaped it. A plain `bun run` exits in this scenario, and so does `node --watch`. Record when a termination-class signal (SIGHUP/SIGINT/SIGQUIT/SIGTERM) has been delivered to a handler, and break out of the watch loop once the event loop drains after such a signal. The process then takes the normal exit path (process.exitCode, 0 by default), matching `bun run` and Node.

robobun · 2026-06-16T15:16:56Z

^{Updated 11:01 AM PT - Jun 16th, 2026}

✅ @autofix-ci[bot], your commit 09c20b091856b96b5144b00f3c3e13e46c5c6bec passed in Build #62831! 🎉

🧪 To try this PR locally:

bunx bun-pr 32405

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

bun-32405 --bun

github-actions · 2026-06-16T15:20:14Z

Found 2 issues this PR may fix:

Bun does not terminate the process when Bun.spawn and --watch mode are running #13539 - --watch mode with Bun.spawn and a custom SIGINT handler fails to terminate on Ctrl+C, same root cause of the watch loop not breaking after signal handling
Unexpected behavior with Bun.serve with --watch mode when exiting #5624 - Bun.serve with --watch mode hangs on exit when SIGINT is received and the exit handler calls server.stop()

If this is helpful, copy the block below into the PR description to auto-close these issues on merge.

Fixes #13539
Fixes #5624

🤖 Generated with Claude Code

coderabbitai · 2026-06-16T15:22:22Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

Walkthrough

PosixSignalHandle gains an AtomicBool field set on termination-class signals (SIGHUP, SIGINT, SIGQUIT, SIGTERM) during enqueue. EventLoop and VirtualMachine expose termination_signal_requested() accessors that thread the flag upward. Both the run-command and test-command watcher run-loops break out when that flag is true and the event loop is drained. Tests validate clean exit for --watch, --hot, and bun test --watch.

Changes

SIGINT Watch-Mode Clean Exit

Layer / File(s)	Summary
Termination flag in `PosixSignalHandle` `src/jsc/PosixSignalHandle.rs`, `test/internal/dead-code-escape-limits.json`	Adds `AtomicBool` import, `termination_requested` field, `is_termination_signal` helper covering `SIGHUP`/`SIGINT`/`SIGQUIT`/`SIGTERM`, sets the flag with `Release` ordering in `enqueue`, and exposes it via a `pub(crate)` accessor with `Acquire` ordering. Updates escape limit configuration for the module.
`EventLoop` and `VirtualMachine` accessors `src/jsc/event_loop.rs`, `src/jsc/VirtualMachine.rs`	`EventLoop::termination_signal_requested()` delegates to `signal_handler.termination_requested()` on Unix and returns `false` on non-Unix; `VirtualMachine::termination_signal_requested()` forwards to `event_loop_shared()`.
Watcher loop exit condition and tests `src/runtime/cli/run_command.rs`, `src/runtime/cli/test_command.rs`, `test/cli/watch/watch.test.ts`	Both `Run::start` and `TestCommand::run_event_loop_for_watch` break their watcher keep-alive loops when `vm.termination_signal_requested()` is true and the event loop is no longer alive. Tests for `--watch` and `--hot` spawn a server with custom `SIGINT` cleanup, send `SIGINT`, and assert exit code `0` with no `signalCode`. Separate tests validate `bun test --watch` behaves identically both with preload-based cleanup and with SIGINT triggered inside the test run.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: detecting termination signals and exiting the watch loop when a custom handler drains the event loop.
Description check	✅ Passed	The description provides thorough coverage of what the PR does, the root cause, the fix mechanism, relation to prior work, and verification. It follows the template structure with detailed explanations.
Linked Issues check	✅ Passed	The PR fully addresses both linked issues: `#32400` (--watch hanging on SIGINT with ref'd resources and custom handler) and `#13539` (--watch hanging with Bun.spawn children and SIGINT handler) through the termination signal tracking and watch loop exit mechanism.
Out of Scope Changes check	✅ Passed	All changes are within scope: signal handling infrastructure (PosixSignalHandle, EventLoop), event loop coordination (VirtualMachine), watch loop exit logic (run_command, test_command), test configuration, and comprehensive test coverage.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/cli/watch/watch.test.ts`:
- Around line 108-109: Remove the explicit per-test timeout configuration that
sets the timeout to isDebug ? 30_000 : 15_000. Delete this timeout parameter
entirely and rely on Bun's built-in timeout behavior instead, as per the coding
guidelines that prohibit per-test timeout settings.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: b0c07fe9-93d6-4108-a5bb-e562a59e2a4d

📥 Commits

Reviewing files that changed from the base of the PR and between 78f0fff and a402c2e.

📒 Files selected for processing (5)

src/jsc/PosixSignalHandle.rs
src/jsc/VirtualMachine.rs
src/jsc/event_loop.rs
src/runtime/cli/run_command.rs
test/cli/watch/watch.test.ts

github-actions · 2026-06-16T15:22:51Z

This PR may be a duplicate of:

--watch/--hot: exit on SIGINT/SIGTERM when a JS handler is installed #30597 - Both fix the same watch/hot mode hang when a custom SIGINT/SIGTERM handler drains the event loop; --watch/--hot: exit on SIGINT/SIGTERM when a JS handler is installed #30597 targets the old Zig path while this PR implements the fix on the live Rust path, and both reference issues Bun does not terminate the process when Bun.spawn and --watch mode are running #13539 and Spawned processes become zombies when using --watch #9871

🤖 Generated with Claude Code

claude

I didn't find any bugs, but this changes signal-handling semantics in the --watch/--hot run-loop and takes a different exit-code stance than #30597, so it's worth a human look.

Extended reasoning...

Overview

This PR adds an AtomicBool to PosixSignalHandle that is set (in async-signal context, via enqueue) when a termination-class signal (SIGHUP/SIGINT/SIGQUIT/SIGTERM) is delivered to a user-installed handler. New accessors on EventLoop and VirtualMachine expose the flag, and the --watch/--hot run-loop in src/runtime/cli/run_command.rs now breaks out (instead of blocking in tick_possibly_forever()) once the flag is set and the event loop has drained. Two new tests in test/cli/watch/watch.test.ts cover both --watch and --hot.

Security risks

None apparent. The change adds a single atomic-bool store in the existing signal-handler path (async-signal-safe) and a read on the main thread. No new inputs, parsing, or privilege boundaries.

Level of scrutiny

Moderate-to-high. The diff is small and well-commented, but it touches the POSIX signal ring (code that runs in async-signal context) and alters the lifecycle of the watcher run-loop — a core, user-visible runtime path. The flag is sticky (never cleared), which seems intentional but is worth a maintainer's eye for edge cases like: signal arrives → handler keeps loop alive → later the loop drains for an unrelated reason → process exits. The placement after on_before_exit() and the re-check of !vm.is_event_loop_alive() look correct to me.

Other factors

The PR explicitly diverges from #30597 on exit-code semantics (clean 0 vs 128+signal). The argument (matching plain bun run and node --watch) is reasonable, but it's a product/behavior decision a maintainer should sign off on rather than a purely mechanical fix. Test coverage is good (fail-before/pass-after on both flags), and the Windows path is a no-op via #[cfg]. Given the runtime-critical surface and the design choice involved, I'm deferring rather than approving.

robobun · 2026-06-16T15:38:05Z

Re: the issue-bot suggestions. I added Fixes #13539 after checking it against this build: its repro (Bun.spawn children + --watch + a SIGINT handler that kills them) hangs on the baked bun and exits cleanly (code 0) here, so the same mechanism fixes it.

I left out #5624 on purpose. Its repro has no custom SIGINT handler, only process.on("exit", ...), so the default disposition already terminates it (exit 130) both before and after this change. That scenario is unaffected, so this PR doesn't close it.

claude

I didn't find any bugs and the implementation looks sound (the atomic store in the signal-context enqueue is async-signal-safe), but this changes user-facing exit semantics for --watch/--hot and bakes in a design choice (exit 0 vs 128+signal, flag never reset across reloads) that's worth a human sign-off.

Extended reasoning...

Overview

This PR threads a one-way AtomicBool through PosixSignalHandle → EventLoop → VirtualMachine that records when a termination-class signal (SIGHUP/SIGINT/SIGQUIT/SIGTERM) has been routed to a user handler, then has the --watch/--hot keep-alive loop in run_command.rs break out (instead of re-entering tick_possibly_forever()) once the JS event loop has drained after such a signal. New tests in watch.test.ts cover both flags. CodeRabbit's per-test-timeout nit was already addressed in commit a71eecd.

Security risks

None identified. The only addition to the async-signal handler path is an AtomicBool::store(true, Release) plus integer compares against compile-time #[repr(u8)] enum constants — all async-signal-safe. No new external input parsing, auth, or privilege boundaries.

Level of scrutiny

Medium-high. The diff is small and mechanically clean, but it sits in three sensitive layers: the POSIX signal ring (async-signal context), the per-tick EventLoop accessor surface, and the --watch/--hot outer run-loop that governs process lifetime. It also changes user-observable behavior — a previously-hanging process will now exit — and the PR explicitly diverges from #30597 on exit-code semantics (clean 0 vs 128+signal). That's a defensible choice (matches plain bun run and node --watch) but it's a product decision a maintainer should ratify.

Other factors

The flag is monotonic and never reset, so once any termination signal is caught during a watch session, any subsequent point where the event loop fully drains will exit the process (e.g., after a later reload whose script doesn't re-ref anything). That's probably the desired semantics, but combined with the exit-code choice it's enough behavior surface that I'd rather a human confirm than auto-approve. No CODEOWNERS cover these paths.

The new termination_requested() accessor is used on unix (from the watch run-loop) and dead on non-unix, so it carries #[allow(dead_code)] like the sibling signal-ring methods. Bump the escape count for the file from 6 to 7.

run_event_loop_for_watch had the same infinite keep-alive loop as the bun run --watch path, so a preload (or test file) that installs a custom SIGINT handler and cleans up would hang bun test --watch after Ctrl+C. Apply the same termination-signal guard so it exits through the normal post-watch path once the event loop drains.

run_event_loop_for_watch had a leading tick_possibly_forever() outside the loop, so a SIGINT delivered mid-run (handled, loop drained) before the function was entered would still park on the 4-minute keep-alive timer. Drop the leading call so the single termination-signal guard precedes the only blocking call, matching the bun run --watch loop. Add a regression test that delivers SIGINT from inside a test.

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/cli/watch/watch.test.ts`:
- Around line 205-212: The test is waiting for proc.exited before draining the
stdout and stderr pipes, which can cause a deadlock if the output fills the pipe
buffers. Combine the three awaits (proc.exited, proc.stdout.text(), and
proc.stderr.text()) into a single Promise.all() call so they all execute
concurrently. Currently proc.exited is awaited separately before the
stdout/stderr reads; move the proc.exited promise into the existing
Promise.all() array and adjust the destructuring to capture the exit code from
the third position in the results array.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6e9bac82-d252-4eab-a592-abc675ae6654

📥 Commits

Reviewing files that changed from the base of the PR and between 26208e8 and e1326b1.

📒 Files selected for processing (2)

src/runtime/cli/test_command.rs
test/cli/watch/watch.test.ts

Await stdout/stderr alongside proc.exited so a full pipe can't stall the child, and assert the handler ran (GOT_SIGINT on stdout).

robobun mentioned this pull request Jun 16, 2026

--watch does not exit on sigint when ref'd ressources exist #32400

Closed

github-actions Bot added the claude label Jun 16, 2026

coderabbitai Bot reviewed Jun 16, 2026

View reviewed changes

Comment thread test/cli/watch/watch.test.ts Outdated

claude Bot reviewed Jun 16, 2026

View reviewed changes

test: rely on the default test timeout in the watch SIGINT test

a71eecd

claude Bot reviewed Jun 16, 2026

View reviewed changes

Comment thread test/cli/watch/watch.test.ts Outdated

test: surface stderr on unclean exit in the watch SIGINT test

1161be0

claude Bot reviewed Jun 16, 2026

View reviewed changes

Comment thread src/runtime/cli/run_command.rs

claude Bot reviewed Jun 16, 2026

View reviewed changes

Comment thread src/runtime/cli/test_command.rs

coderabbitai Bot reviewed Jun 16, 2026

View reviewed changes

Comment thread test/cli/watch/watch.test.ts Outdated

robobun and others added 2 commits June 16, 2026 17:21

test: drain pipes concurrently in the mid-run watch SIGINT test

c3ca32b

Await stdout/stderr alongside proc.exited so a full pipe can't stall the child, and assert the handler ran (GOT_SIGINT on stdout).

[autofix.ci] apply automated fixes

09c20b0

coderabbitai Bot mentioned this pull request Jul 23, 2026

Add persistent worker execution plane rajohan/Mira-Dashboard#314

Merged

coderabbitai Bot mentioned this pull request Aug 15, 2026

refactor(canary): inject a typed subprocess seam myagentdojo/agent-plugin-template#48

Merged

Navigation Menu

Uh oh!

watch: exit on SIGINT when a custom signal handler drains the event loop - #32405

watch: exit on SIGINT when a custom signal handler drains the event loop#32405
robobun wants to merge 8 commits into
mainfrom
farm/caed0ad3/watch-sigint-exit

robobun commented Jun 16, 2026 •

edited

Loading

Uh oh!

robobun commented Jun 16, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jun 16, 2026

Uh oh!

coderabbitai Bot commented Jun 16, 2026 •

edited

Loading

Reviews paused

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

github-actions Bot commented Jun 16, 2026

Uh oh!

claude Bot left a comment

Uh oh!

robobun commented Jun 16, 2026

Uh oh!

claude Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

robobun commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Repro

Cause

Fix

Relation to #30597

Verification

Uh oh!

robobun commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jun 16, 2026

Uh oh!

coderabbitai Bot commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Jun 16, 2026

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

robobun commented Jun 16, 2026

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

robobun commented Jun 16, 2026 •

edited

Loading

robobun commented Jun 16, 2026 •

edited

Loading

coderabbitai Bot commented Jun 16, 2026 •

edited

Loading