Skip to content

Use sigwait for POSIX signal handling#5

Merged
huangminghuang merged 2 commits into
masterfrom
feature/posix-sigwait-signal-handling
Jun 3, 2026
Merged

Use sigwait for POSIX signal handling#5
huangminghuang merged 2 commits into
masterfrom
feature/posix-sigwait-signal-handling

Conversation

@huangminghuang

@huangminghuang huangminghuang commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

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

  • Block SIGINT, SIGTERM, SIGPIPE, and SIGHUP before worker threads inherit signal masks.
  • Add a dedicated POSIX signal thread that waits with sigwait and dispatches appbase signal handling.
  • Preserve SIGHUP reload behavior by posting callback/plugin handling through appbase's dispatcher.
  • Treat SIGINT and SIGTERM as graceful quit signals.
  • Treat SIGPIPE as an explicit no-op so a broken pipe does not shut down the process.
  • Remove the unused io_context parameter from application_base::startup().
  • Add a POSIX appbase unit test that verifies SIGHUP reloads without quitting, SIGPIPE does not quit, and SIGTERM quits cleanly.

Why

The old implementation relied on a fragile per-thread signal-mask invariant: appbase created one signal-catching thread before blocking SIGHUP, SIGINT, SIGTERM, and SIGPIPE in the rest of the process, then assumed that thread would keep those signals unblocked for the lifetime of the process.

The old sequence was:

  1. appbase created a dedicated signal-catching thread while the target signals were still unblocked.
  2. appbase then blocked SIGHUP, SIGINT, SIGTERM, and SIGPIPE in the rest of the app-managed threads so normal I/O would not be interrupted by those signals.
  3. Boost.Asio installed process-wide sigaction handlers for the registered signals.
  4. When a process-directed signal such as SIGTERM arrived, macOS had to deliver it to an unblocked thread.
  5. Asio's handler would write the signal number into its internal pipe, the Asio reactor would dispatch the async_wait callback, and appbase would call quit().

The failure observed on macOS is that this invariant can be lost after startup. Temporary instrumentation showed:

  • The signal setup initially worked correctly. The appbase signal thread started with SIGINT, SIGTERM, SIGPIPE, and SIGHUP unblocked.
  • Asio's process-wide handlers remained installed; signal dispositions stayed custom for the target signals.
  • Later in the same run, the appbase signal-catching thread reported those same signals as blocked: INT=1 TERM=1 PIPE=1 HUP=1.
  • The signal-catching thread was still alive inside its dedicated Asio 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.
  • After that point, process-directed SIGTERM/SIGHUP left the process alive. No Asio signal-handler entry was logged, no appbase async_wait callback ran, and appbase never called quit().
  • A healthy node in the same instrumented run showed the expected path: Asio handler entry -> appbase async_wait callback -> 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:

  • sys-vm invoke_with_signal_handler mask operations were fully ablated, including the SIG_UNBLOCK call and both SIG_SETMASK restore paths; the invariant failure still reproduced.
  • Boost.Asio posix_signal_blocker activity on the signal-catching thread was checked with a direct in-source probe; it produced zero entries before the abort-trap detection.
  • Boost.Asio scheduler own-thread signal_blocker was ablated from the macOS build-local Asio headers; the invariant failure still reproduced.
  • Boost.Asio signal_set_service fork-child masking requires fork() and does not run in this node workload.
  • The remaining Boost.Asio select_reactor signal-blocker paths are guarded by BOOST_ASIO_HAS_IOCP and 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:

  1. Block the target signals before application worker threads are created.
  2. Let worker threads inherit the blocked signal mask.
  3. Start one dedicated signal thread that keeps those signals blocked and waits with sigwait.
  4. Let sigwait synchronously consume pending process-directed signals from that blocked set.
  5. Dispatch appbase behavior from that known signal owner.

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-directed SIGTERM becomes pending for the blocked signal set, the signal waiter receives it synchronously, and appbase deterministically calls quit(). SIGHUP is still posted back through appbase's dispatcher for reload callbacks, and SIGPIPE is 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 at signal_loop entry and then runs only the sigwait loop, 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/sigprocmask if 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 the sigwait design removes appbase's dependency on an unblocked signal owner.

Testing

  • macOS: cmake --build build/macos-arm64-ci-repro --target appbase_test custom_appbase_test
  • macOS: build/macos-arm64-ci-repro/libraries/appbase/tests/appbase_test --run_test=posix_signal_handling --report_level=detailed --color_output=no
  • macOS: full appbase_test passed before the final rebase
  • macOS: full custom_appbase_test passed before the final rebase
  • macOS: instrumented old-Asio failure run confirmed the signal-catching thread started with the target signals unblocked, later reported them blocked while still alive in its dedicated io_context, and then stopped receiving SIGTERM/SIGHUP
  • macOS: direct Boost.Asio posix_signal_blocker probe logged zero signal-thread entries before the abort-trap detection
  • macOS: sys-vm invoke_with_signal_handler mask-operation ablation did not prevent the invariant failure
  • macOS: Boost.Asio scheduler own-thread signal_blocker ablation did not prevent the invariant failure
  • Linux sandbox: appbase_test passed
  • Linux sandbox: custom_appbase_test passed
  • Linux sandbox: posix_signal_handling passed

@heifner

heifner commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

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 sigwait owner dispatch back onto the executor. I confirmed the invariant it rests on: neither the appbase default_executor nor wire-sysio's priority_queue_executor starts a thread in its constructor, so every worker is created after the block and inherits it. Defaulting the std::function members to no-ops (plus the null guard in set_sighup_callback) is a nice independent hardening. A few things worth a look:

Worth discussing

  • SIGPIPE is routed to quit() (handle_signal): everything that isn't SIGHUP, including SIGPIPE, falls through to quit(). Shutting the process down on a broken pipe is almost never intended -- the conventional handling is SIG_IGN, since the write already returns EPIPE. Two mitigating facts: it matches the old behavior (the old asio handler also quit on SIGPIPE), and a synchronously generated SIGPIPE stays pending on the writing worker thread (blocked there, not dequeued by the waiter), so in practice only a process-directed kill(pid, SIGPIPE) would actually quit. Still reads as unintended -- suggest dropping SIGPIPE from the quit path or making it an explicit no-op.

  • signal_loop relies on an inherited mask rather than guaranteeing its own. sigwait is only reliable when the awaited signals are blocked in the calling thread; today that holds only because start_signal_thread happens to run on the thread that blocked them in the constructor. A one-line pthread_sigmask(SIG_BLOCK, &blocked_signals, nullptr) at the top of signal_loop makes it self-contained and immune to which thread calls initialize.

  • The double stop_signal_thread() (in ~application_base and again in ~application_impl) is correct, but subtle enough to deserve a comment: the explicit base-dtor call stops the thread before post_cb / stop_executor_cb / initialized_plugins are destroyed, closing a window where a late signal could dispatch into half-destroyed state. Without a note, a future cleanup could "simplify" the seemingly-redundant call away and reintroduce the teardown race.

Minor

  • Windows now has no signal handling (the _WIN32 branch stubs start/stop_signal_thread). I checked -- old Windows didn't really handle these either (the asio signal_set lived on an io_context that was never run() on Windows, and SIGHUP is undefined there), so it's not a regression. A one-line comment in the _WIN32 branch would save the next reader the same dig.
  • pthread_kill's return value is ignored in stop_signal_thread -- harmless (the join still succeeds), but a (void) cast documents intent.
  • application_base.cpp now has two adjacent #include <boost/algorithm/string.hpp> lines (pre-existing duplicate, just more visible after the signal_set.hpp removal).
  • On a sigwait error the loop logs and continues; since sigwait only fails with EINVAL here it's effectively unreachable, but break would be safer than risking a hot cerr spin.

Tests

Good addition -- posix_signal_handling covers the two important paths (SIGHUP reloads without quitting, SIGTERM quits). The gaps are SIGINT and SIGPIPE; given the SIGPIPE question above, a test pinning the intended SIGPIPE behavior would be the most valuable to add, since it forces the decision.

@heifner

heifner commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Threading & WASM-signal interference (wire-sysio side)

I checked whether moving to a dedicated sigwait thread interferes with wire-sysio's WASM runtime (sys-vm-oc) or its threading model, since appbase now owns a signal thread the chain layer is unaware of. Short version: no interference, because the change preserves the blocked-signal set, and the WASM runtime uses signals outside that set.

The blocked-signal set is unchanged. get_target_sigset() is still {SIGINT, SIGTERM, SIGPIPE, SIGHUP}, identical to master. This PR only changes how those four are consumed (sigwait vs. the asio signal_set on a dedicated io_context thread), so no thread's exposure to any other signal changes.

sys-vm-oc memory faults (SIGSEGV) -- installed by the OC executor's signal initializer with SA_SIGINFO | SA_NODEFER, chaining any prior handler. SIGSEGV is synchronous and thread-directed (delivered to the faulting thread) and is never in appbase's set. Untouched.

Deadline timer (SIGRTMIN) -- platform_timer_posix uses timer_create with SIGEV_SIGNAL, i.e. a process-directed SIGRTMIN that is not in appbase's set. Its handler is thread-agnostic: it only does an atomic CAS via the tagged sival_ptr and runs the expiration callback. The OC deadline actually interrupts running native code like this: the expiration callback mprotects the code mapping to PROT_NONE (a process-wide address-space change, done via a raw syscall so it is async-signal-safe), and the WASM-executing thread then faults on its next fetch and takes its own synchronous SIGSEGV, which siglongjmps out. That is robust to which thread received SIGRTMIN by construction. appbase's sigwait thread is simply one more SIGRTMIN-eligible thread, exactly as the old asio signal thread was, so exposure is unchanged.

OC compile monitor -- a forked subprocess that does its own sigprocmask + SIGCHLD setup after fork. fork() only carries the calling thread, so appbase's signal thread isn't present in the child. Independent of appbase.

Threading -- every thread-creation site in wire-sysio (the net/http/producer/chain/etc. plugins, named_thread_pool, platform_timer_asio_fallback, the OC code cache) runs during plugin/controller startup -- after appbase's constructor blocks signals and after initialize(). They all inherit the blocked mask, which is what the design wants. Nothing spawns a thread in static init before the app is constructed.

The wire-sysio integration suite already covers the externally-visible behavior: SIGTERM-driven node shutdown is exercised throughout (e.g. nodeop_contrl_c_test.py and the kill(SIGTERM) teardown used across the suite), and SIGHUP is verified to reload logging without shutting the node down.

@huangminghuang huangminghuang force-pushed the feature/posix-sigwait-signal-handling branch from 7171626 to aee7562 Compare June 2, 2026 15:53

@heifner heifner left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hold off

@huangminghuang huangminghuang requested a review from heifner June 3, 2026 00:17
@huangminghuang huangminghuang merged commit 104ec22 into master Jun 3, 2026
0 of 2 checks passed
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