Skip to content

client: restore signal dispositions on teardown and escalate repeated signals#1560

Merged
eric846 merged 1 commit into
mainfrom
shutdown-signal-handling
Jul 9, 2026
Merged

client: restore signal dispositions on teardown and escalate repeated signals#1560
eric846 merged 1 commit into
mainfrom
shutdown-signal-handling

Conversation

@oschaaf

@oschaaf oschaaf commented Jul 9, 2026

Copy link
Copy Markdown
Member

Part 3/3 of fixing nighthawk failing to shut down when a target can't keep up.

Three related defects left nighthawk unresponsive to Ctrl-C exactly when shutdown hangs:

  • ~SignalHandler() left the process-wide handler installed, delegating to a destroyed instance (undefined behavior; in practice a silent no-op).
  • client.cc destroyed the SignalHandler before process->shutdown(), so the phase most likely to hang had no live signal handling.
  • After the first handled signal the notification pipe is closed, so a second Ctrl-C was silently swallowed, leaving no force-quit path.

Fix: SignalHandler saves the dispositions it replaces and restores them on destruction (then clears the process-wide delegate); the handler's scope in client.cc now covers process->shutdown(); and the saved dispositions are restored as soon as the first signal is consumed — deliberately before the cancellation callback runs, since that callback may itself block on the hung shutdown (it contends on the lock held across the worker drain loop). A restore placed after the callback would never execute in exactly the flagship hang scenario. Destructor ordering (set flag, restore dispositions, join, clear delegate) is documented in-code.

Behavior change: a second Ctrl-C now hard-kills the process (no output written) — intended, it's the escape hatch. nighthawk_service uses the same SignalHandler and inherits the escalation.

Testing: disposition-restore test, a threadsafe-style death test (post-destruction SIGTERM must kill the process; previously "failed to die"), and an escalation test that observes the disposition fall back to SIG_DFL via read-only sigaction probing (a std::signal round-trip probe could race and clobber the restore). All three fail against the previous implementation.

signals

Three related defects left nighthawk unresponsive to Ctrl-C exactly
when shutdown hangs:
- ~SignalHandler() left the process-wide handler installed, delegating
  to a destroyed instance (undefined behavior; in practice a silent
  no-op).
- client.cc destroyed the SignalHandler before process->shutdown(), so
  the phase most likely to hang had no live signal handling.
- After the first handled signal the notification pipe is closed, so a
  second Ctrl-C was silently swallowed, leaving no force-quit path.

SignalHandler now saves the dispositions it replaces and restores them
on destruction (then clears the process-wide delegate), the handler's
scope in client.cc covers process->shutdown(), and the saved
dispositions are restored as soon as the first signal is consumed -
before the cancellation callback runs, since that callback may itself
block on the hung shutdown - so any further signal gets the OS default
action and terminates the process.

Note: nighthawk_service uses the same SignalHandler and inherits the
escalation; a second SIGTERM/SIGINT now terminates it after the first
has initiated graceful shutdown.

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
@oschaaf oschaaf force-pushed the shutdown-signal-handling branch from aab0956 to 4804e77 Compare July 9, 2026 11:28
@eric846 eric846 merged commit b2b77ad into main Jul 9, 2026
9 checks passed
@eric846

eric846 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Thank you Otto! This will be a huge improvement to have a fast and clean return even when the server was slow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants