Skip to content

fix(core): register signal handler before startup work and handshake - #328

Merged
bug-ops merged 1 commit into
mainfrom
fix/318-sigterm-handler-race
Aug 5, 2026
Merged

fix(core): register signal handler before startup work and handshake#328
bug-ops merged 1 commit into
mainfrom
fix/318-sigterm-handler-race

Conversation

@bug-ops

@bug-ops bug-ops commented Aug 5, 2026

Copy link
Copy Markdown
Owner

Summary

  • run_stdio registered its SIGTERM/SIGINT handler only after mcp_server.serve(..)'s MCP initialize handshake resolved, and serve_with ran config validation, workspace-root heuristics, and spawn_lsp_servers_background before run_stdio ever got a chance to register one. A signal delivered anywhere in that window fell through to the OS's default disposition (immediate kill), skipping Translator::shutdown_servers and risking an orphaned LSP child process mid-spawn.
  • ShutdownSignal is now constructed once, as the first statement in serve_with, before any startup work runs, and moved by value into whichever transport runs; run_stdio races it against the handshake itself, then reuses the same instance in its existing post-handshake select!.
  • Also fixes a related gap found during review: SIGINT was previously re-registered via a fresh one-shot tokio::signal::ctrl_c() on every wait, which could silently lose a signal delivered while a different select! branch was being polled. ShutdownSignal now holds a persistent listener per signal kind for its entire lifetime.

Fixes #318.

Test plan

  • cargo +nightly fmt --all -- --check
  • cargo clippy --all-targets --all-features --workspace -- -D warnings
  • cargo nextest run --workspace --all-features --lib --bins (643 passed)
  • RUSTFLAGS="-D warnings" RUSTDOCFLAGS="--deny rustdoc::broken_intra_doc_links" cargo doc --no-deps --workspace --all-features
  • cargo test --doc --workspace --all-features
  • New regression test test_e2e_sigterm_exits_promptly_during_handshake_wait, targeting the actual pre-fix window (SIGTERM sent before initialize() is ever called): 5/5 failures against pre-fix code, 10/10 passes against the fix
  • Existing test_e2e_sigterm_exits_promptly_while_client_stdin_open (mcpls process does not exit on SIGTERM/SIGINT while a stdio client is still connected #308 regression) still passes with its artificial delay removed
  • Empirically verified 0 orphaned LSP child processes across 8 kill-delay timings (0.001s-0.300s) against a stand-in long-running LSP server

@github-actions github-actions Bot added documentation Improvements or additions to documentation rust Rust code changes testing Test-related changes mcpls-core mcpls-core crate changes labels Aug 5, 2026
serve_with previously ran config validation, workspace-root heuristics,
and spawn_lsp_servers_background before run_stdio ever registered a
SIGTERM/SIGINT handler, and run_stdio itself only registered one after
mcp_server.serve(..)'s MCP initialize handshake resolved. rmcp's
serve(..) awaits the client's first message internally, so a signal
arriving at any point up to and including that wait fell through to
the OS's default disposition: immediate termination, skipping
Translator::shutdown_servers and risking an orphaned LSP child process
mid-spawn.

ShutdownSignal is now constructed once, as the first statement in
serve_with, before any startup work, and moved by value into whichever
transport runs; run_stdio races it against the handshake itself, then
reuses the same instance in its existing post-handshake select!.

Also fixes a related gap found while designing the reused handle:
SIGINT was re-registered via tokio::signal::ctrl_c() on every wait (a
fresh one-shot listener each time), silently losing a signal delivered
while a different select! branch was being polled. ShutdownSignal now
holds a persistent listener per signal kind for its entire lifetime
instead.
@bug-ops
bug-ops force-pushed the fix/318-sigterm-handler-race branch from ba7638c to f463712 Compare August 5, 2026 02:58
@bug-ops
bug-ops enabled auto-merge (squash) August 5, 2026 02:58
@bug-ops
bug-ops merged commit 9642857 into main Aug 5, 2026
28 checks passed
@bug-ops
bug-ops deleted the fix/318-sigterm-handler-race branch August 5, 2026 03:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation mcpls-core mcpls-core crate changes rust Rust code changes testing Test-related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SIGTERM/SIGINT delivered before the stdio handler registers bypasses graceful shutdown

1 participant