fix(watch): support graceful shutdown via SIGTERM dispatch#32564
fix(watch): support graceful shutdown via SIGTERM dispatch#32564bartlomieju merged 7 commits intodenoland:mainfrom
Conversation
When watch mode restarts due to file changes or exits due to Ctrl+C, synthetically dispatch SIGTERM to registered signal handlers before cancelling the running operation. This gives JavaScript code a chance to perform async cleanup (e.g. closing DB connections, releasing resources, running finalizers). If no SIGTERM handlers are registered, the restart/exit happens immediately as before. If handlers are registered, a grace period of up to 10 seconds is given for the operation to finish. Closes denoland#30912 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
kajukitli
left a comment
There was a problem hiding this comment.
lgtm — nice feature for graceful shutdown in watch mode
implementation is clean:
deno_signals::raise(SIGTERM)dispatches to registered handlers- only waits if handlers actually exist (returns true)
- 10 second timeout prevents hanging forever
- works for both restart (file change) and exit (Ctrl+C)
the known limitation about process.exit() killing the watcher is fine — that's expected behavior and documented
one thought: might be worth a future enhancement to also dispatch SIGINT on Ctrl+C in addition to SIGTERM, since some libraries only listen for SIGINT. but SIGTERM is the standard "please shut down" signal so this is good as-is
- Dispatch both SIGINT and SIGTERM on Ctrl+C so libraries that only listen for SIGINT also get notified for graceful shutdown - Add test for SIGTERM dispatch on watch restart (file change) - Add test for SIGINT+SIGTERM dispatch on Ctrl+C in watch mode Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Resolve conflicts in tests/integration/watcher_tests.rs by keeping both the upstream unload/process-exit tests and adding back the PR's SIGTERM/SIGINT tests with #[cfg(unix)] to fix Windows CI hangs. Also reduce GRACEFUL_SHUTDOWN_TIMEOUT from 10s to 5s. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add SIGTERM to the Windows signal dictionary so that
Deno.addSignalListener("SIGTERM", ...) works on Windows. libuv already
defines SIGTERM on Windows (from the CRT), and Deno's signal init()
already maps CTRL_LOGOFF_EVENT and CTRL_SHUTDOWN_EVENT to SIGTERM.
This was the only missing piece preventing SIGTERM handlers from being
registered on Windows.
This also fixes the run_watch_sigterm_on_restart test on Windows CI —
the test no longer needs #[cfg(unix)] since SIGTERM handlers now work
cross-platform.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Update the signalsNotImplemented test to match the new error message that includes SIGTERM, and remove SIGTERM from the unsupported list. Add a test verifying SIGTERM can be registered on Windows. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
kajukitli
left a comment
There was a problem hiding this comment.
This PR implements synthetic signal raising for Deno's file watcher to ensure both SIGINT and SIGTERM handlers fire when Ctrl+C is pressed. The implementation has a medium-severity concern about cross-platform compatibility on Windows and a minor code clarity issue with the use of bitwise OR.
[MEDIUM] cli/util/file_watcher.rs:408: On Windows, synthetically raising SIGINT may behave differently than on Unix since Windows doesn't have native POSIX signal support. While SIGINT is listed in the Windows signal_dict, the behavior of the synthetic raise should be verified on Windows.
Suggestion: Verify that synthetically raising SIGINT works correctly on Windows, or consider platform-specific handling if issues arise.
[MEDIUM] ext/signals/lib.rs:254: The raise function calls handle_signal(signal) directly. Ensure that handle_signal is designed to be called from an async context (not just from actual signal handler callbacks) and properly synchronizes with any signal handler state.
Suggestion: Verify that
handle_signalis safe to call directly outside of a signal handler context.
[LOW] cli/util/file_watcher.rs:408: The bitwise OR (|) is used instead of logical OR (||) to ensure both raise(SIGINT) and raise(SIGTERM) execute without short-circuiting. While this is likely intentional, it may not be immediately clear to readers.
Suggestion: Consider adding a brief comment explaining that bitwise OR is used intentionally to ensure both signals are always raised, e.g.,
// Use bitwise OR to ensure both signals are raised regardless of return values
- Clarify that `raise()` works on all platforms (including Windows) because it invokes JS handlers directly rather than OS-level signals - Document that bitwise OR is intentional to avoid short-circuiting - Add doc comment on `raise()` explaining it's safe from any context Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ful-shutdown # Conflicts: # ext/signals/dict.rs # tests/unit/signal_test.ts
The 5-second grace period introduced in #32564 causes frustrating delays during development when a SIGTERM handler is registered (e.g. for production DB cleanup). 500ms is enough for most cleanup tasks while keeping the dev reload loop snappy. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The 5-second grace period introduced in #32564 causes frustrating reload delays during development when a SIGTERM handler is registered (e.g. for production graceful DB shutdown). 500ms is enough time for most cleanup tasks (closing connections, flushing logs) while keeping the dev reload loop snappy. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
deno_signals::raise()public API to synthetically trigger signal handlersCloses #30912
Example
On file change with
deno run --watch:Known limitation
process.exit()/Deno.exit()callsstd::process::exit()which terminates the entire Deno process, including the watcher. Scripts using SIGTERM handlers in watch mode should perform cleanup and let the event loop drain naturally (e.g. clear intervals/timeouts, close servers) rather than callingprocess.exit(). This is a pre-existing limitation unrelated to this change — it also affects the existingunloadevent behavior in watch mode.Test plan
🤖 Generated with Claude Code