Internal-trigger shutdown arms force-quit without having consumed a signal, so a single routine SIGTERM aborts the graceful drain (asymmetric with the OS-signal path)

[erskingardner]

Summary

wait_for_signal_or_trigger has two exit paths that are asymmetric with respect to when force-quit-on-next-signal is armed. On the OS-signal path, force-quit arms only after the first signal is consumed, so it takes two signals to force-exit (the intended "first = graceful, second = force" contract). On the internal-trigger path, spawn_force_quit_on_second_signal() is called without any signal having been consumed, so the very first OS signal force-exits — aborting the graceful drain that an internal-trigger shutdown just started.

Location

• src/shutdown.rs:86-99 — wait_for_signal_or_trigger; the internal-trigger arm calls spawn_force_quit_on_second_signal() at line 95
• Contrast src/shutdown.rs:101-116 — handle_first_shutdown_signal arms force-quit (line 115) only after signal.await (line 109)
• Driven from src/app.rs:723-744 (run-loop wait → graceful_shutdown(staged_teardown, ...))

Details

Concrete failure path:

1. A supervised critical task dies (event-loop ChannelClosed, or a health-server runtime error/panic) → fires the ShutdownTrigger.
2. wait_for_signal_or_trigger returns InternalTrigger and, on the way out, calls spawn_force_quit_on_second_signal() — but no signal has been consumed, so the spawned listener is now armed to std::process::exit(1) on the first OS signal.
3. main enters graceful_shutdown(staged_teardown, shutdown_timeout_secs) and begins draining in-flight event-unwrap tasks and the push dispatcher (up to the full timeout, e.g. 30s).
4. During that drain the orchestrator very plausibly sends a routine SIGTERM — e.g. Kubernetes terminating a pod that just flipped /ready to 503, or an operator reacting to the crash. That single SIGTERM immediately exit(1)s, cutting staged_teardown short mid-drain. In-flight notifications and mid-unwrap gift wraps are dropped — exactly the loss staged_teardown ([MEDIUM] In-flight event-processing tasks are detached and never awaited on shutdown — gift wraps mid-unwrap are aborted and lost on every restart #84 / [MEDIUM] Shutdown drains the push dispatcher (setting shutting_down) before the event loop is quiesced, so events that finish unwrap during the drain have their pushes rejected and lost #173) exists to prevent.

The asymmetry means identical external input (one SIGTERM) yields a graceful drain when it initiates shutdown, but an immediate force-kill when shutdown was already initiated internally — a difference invisible to the operator/orchestrator sending the signal.

The comment at shutdown.rs:92-95 documents the arming as intentional ("a signal arriving after an internal trigger forces the process out instead of being ignored during teardown"), so this is a design-judgment call rather than an obvious oversight — but the concrete consequence (a single expected SIGTERM aborting an internal-trigger drain, vs. two signals required on the signal path) is worth an explicit decision.

Why this is not a duplicate

• wait_for_signal_or_trigger select is not biased, so an OS signal racing an internal trigger can misclassify the shutdown reason and flip the process exit code #311 concerns only the biased-select! / exit-code misclassification of the same function — not the force-quit arming asymmetry or the premature-drain-abort consequence.
• [LOW] Shutdown robustness: timeout bounds only the push drain, no force-quit on a repeated signal, and signal-handler install uses .expect() #104 is the signal-path "force-quit on repeated signal" contract and does not address the internal-trigger path arming without a consumed signal.

Recommendation

Either (a) do not arm force-quit on the internal-trigger path until a first signal is actually observed (treating the first post-trigger signal as the graceful request, symmetric with the signal path), or (b) if the current behavior is deliberate, document the operator-facing contract that a SIGTERM during an internal-trigger drain force-kills.

Impact

Loss of in-flight notifications when a routine SIGTERM coincides with an internal-trigger-initiated drain. LOW (requires the internal-trigger drain window to overlap an external signal).