Add stdio-mode SIGTERM observability handler (#156)#157
Merged
Conversation
Investigation of #156 found that the daemon already exits promptly on SIGTERM in every measured condition (median 50-60ms across 30 local SIGTERM cycles in the full MacOSMCPTests suite, 0 SIGKILL escalations). The 1200s wedge tracked by #146's polling band-aid was inside Foundation's `Process.waitUntilExit()` Mach-port path on the test-host side, not the daemon. Stdio mode previously had no SIGTERM handler — `runStdio()` skipped `DaemonLifecycle.installSignalHandlers()`, leaving the kernel default disposition (terminate immediately). That's already fast, but yields no observability: if a future shutdown wedge recurs, there's no way to distinguish "daemon never received SIGTERM" from "Foundation Process wedged on the test side." This adds a minimal `signal()` handler that writes a stderr breadcrumb via signal-safe `write(2)` and exits via `_exit(0)`. Deliberately not the daemon-mode `DispatchSource(... queue: .main)` pattern: that path is coupled to NSApplication's runloop, and queue starvation under load was the only remaining live hypothesis for the original wedge. Stdio mode has no on-disk state to clean up before exit (no PID file, no socket), so signal-thread `_exit(0)` is correct. Local verification: - `swift test --filter MacOSMCPTests`: 7/7 pass, exit latency unchanged - Bare-spawn SIGTERM: 4ms exit (vs 5ms before — handler is faster than the kernel default path because it skips dyld destructors) - Breadcrumb appears in per-instance daemon stderr log immediately before `[stop ...] process exited after SIGTERM` from the test harness
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the question raised in #156 by establishing — through local repro — that the daemon already exits promptly on SIGTERM. Adds a small observability hook so any future recurrence is self-diagnosing.
Investigation findings (issue #156)
PR #146 landed a 5s-poll + SIGKILL band-aid in
MCPTestServer.stop(). CI then went 8/8 green with zero SIGKILL escalations, meaning the daemon really does exit within the poll window — so the 1200s wedge wasn't the daemon being slow to die. Local repro confirms this:previewsmcp serve(no MCP traffic), 10×initialize, 10×Process+Pipe()mirroringMCPTestServer, 10×MacOSMCPTests.hotReloadStructural(full preview state)MacOSMCPTests× 5 iterations (30 SIGTERM cycles)The three issue-listed hypotheses:
_NSGetExecutablePathchange — ruled out by code analysis. Only on the daemon-mode spawn path and thepreview_build_infotool. Not on the stdio shutdown path.DaemonClient, used only by CLI clients, notMCPTestServer.The 1200s wedge was inside Foundation's
Process.waitUntilExit()Mach-port path on the test-host side. The polling band-aid sidesteps it. The daemon side is healthy.Change
Add a signal-safe SIGTERM handler in
runStdio(). Behavior:previewsmcp stdio: received SIGTERM, exitingvia signal-safewrite(2)from aStaticStringbuffer._exit(0)— no AppKit teardown, no libdispatch dependency.Deliberately not the daemon-mode
DispatchSource(... queue: .main)pattern. That path is coupled to NSApplication's runloop, and queue starvation under load was the only remaining live hypothesis for the original wedge. Stdio mode has no on-disk state to clean up before exit (no PID file, no socket), so signal-thread_exit(0)is correct.The single observable side effect is the stderr breadcrumb. If a future shutdown wedge recurs, the presence/absence of this line in the per-instance log partitions "daemon ignored signal" from "Foundation Process wedged on the test-host side."
Test plan
swift buildcleanswift test --filter MacOSMCPTests— 7/7 pass, exit latency unchanged (50-60ms per stop)[stop ...] process exited after SIGTERM