Use sigwait for POSIX signal handling#5
Conversation
|
Review of the signal-handling change The core approach is correct and the right fix -- block the four signals before any worker thread spawns, inherit the mask, and let one dedicated Worth discussing
Minor
Tests Good addition -- |
|
Threading & WASM-signal interference (wire-sysio side) I checked whether moving to a dedicated The blocked-signal set is unchanged. sys-vm-oc memory faults (SIGSEGV) -- installed by the OC executor's signal initializer with Deadline timer (SIGRTMIN) -- OC compile monitor -- a forked subprocess that does its own Threading -- every thread-creation site in wire-sysio (the net/http/producer/chain/etc. plugins, The wire-sysio integration suite already covers the externally-visible behavior: SIGTERM-driven node shutdown is exercised throughout (e.g. |
7171626 to
aee7562
Compare
Summary
Replace appbase's Boost.Asio signal handler with a POSIX
sigwait-based signal thread and add regression coverage for SIGHUP/SIGTERM/SIGPIPE handling.What changed
sigwaitand dispatches appbase signal handling.io_contextparameter fromapplication_base::startup().Why
The old implementation relied on a fragile per-thread signal-mask invariant: appbase created one signal-catching thread before blocking
SIGHUP,SIGINT,SIGTERM, andSIGPIPEin the rest of the process, then assumed that thread would keep those signals unblocked for the lifetime of the process.The old sequence was:
SIGHUP,SIGINT,SIGTERM, andSIGPIPEin the rest of the app-managed threads so normal I/O would not be interrupted by those signals.sigactionhandlers for the registered signals.SIGTERMarrived, macOS had to deliver it to an unblocked thread.async_waitcallback, and appbase would callquit().The failure observed on macOS is that this invariant can be lost after startup. Temporary instrumentation showed:
SIGINT,SIGTERM,SIGPIPE, andSIGHUPunblocked.customfor the target signals.INT=1 TERM=1 PIPE=1 HUP=1.io_context/kqueue loop. It had not exited; it was simply no longer eligible to receive the shutdown/reload signals that appbase expected it to own.SIGTERM/SIGHUPleft the process alive. No Asio signal-handler entry was logged, no appbaseasync_waitcallback ran, and appbase never calledquit().async_waitcallback ->quit()-> clean exit.That is the directly observed root failure: the old design requires one special signal-catching thread to remain unblocked, and the failing macOS workload loses that property. Once appbase's signal owner has the target signals blocked, process-directed shutdown/reload signals can remain pending instead of reaching appbase's Asio signal callback.
The reachable in-process candidates were systematically checked and ruled out as the necessary writer:
invoke_with_signal_handlermask operations were fully ablated, including theSIG_UNBLOCKcall and bothSIG_SETMASKrestore paths; the invariant failure still reproduced.posix_signal_blockeractivity on the signal-catching thread was checked with a direct in-source probe; it produced zero entries before the abort-trap detection.signal_blockerwas ablated from the macOS build-local Asio headers; the invariant failure still reproduced.signal_set_servicefork-child masking requiresfork()and does not run in this node workload.select_reactorsignal-blocker paths are guarded byBOOST_ASIO_HAS_IOCPand are not compiled on macOS.The remaining writer is outside the reachable surface we have ablated so far. The fix does not depend on identifying that writer.
This PR changes appbase to the standard robust POSIX model:
sigwait.sigwaitsynchronously consume pending process-directed signals from that blocked set.That is why the fix works: with
sigwait, the signal thread does not need to remain unblocked. The blocked mask is the expected state. A process-directedSIGTERMbecomes pending for the blocked signal set, the signal waiter receives it synchronously, and appbase deterministically callsquit().SIGHUPis still posted back through appbase's dispatcher for reload callbacks, andSIGPIPEis explicitly ignored.The new design still depends on the waiter thread keeping the target signals blocked, but that is a much smaller invariant than the old one. The old signal-catching thread ran a full Asio
io_context/kqueue reactor loop, so reactor and completion code could run on the one thread that needed to preserve an unblocked mask. The new signal thread re-blocks the target set atsignal_loopentry and then runs only thesigwaitloop, leaving essentially no foreign code path on the waiter thread that could mutate its signal mask after startup.Notes
A follow-up investigation can use system-level tracing, such as DTrace on
pthread_sigmask/sigprocmaskif SIP and local permissions allow it, to identify the remaining mask writer. That follow-up is useful for closing the broader investigation, but it is not required for this fix because thesigwaitdesign removes appbase's dependency on an unblocked signal owner.Testing
cmake --build build/macos-arm64-ci-repro --target appbase_test custom_appbase_testbuild/macos-arm64-ci-repro/libraries/appbase/tests/appbase_test --run_test=posix_signal_handling --report_level=detailed --color_output=noappbase_testpassed before the final rebasecustom_appbase_testpassed before the final rebaseio_context, and then stopped receivingSIGTERM/SIGHUPposix_signal_blockerprobe logged zero signal-thread entries before the abort-trap detectioninvoke_with_signal_handlermask-operation ablation did not prevent the invariant failuresignal_blockerablation did not prevent the invariant failureappbase_testpassedcustom_appbase_testpassedposix_signal_handlingpassed