From 14edebaf2f6e9f793b66f711dbe6fd7154bff4d8 Mon Sep 17 00:00:00 2001 From: Steve Gerbino Date: Wed, 21 Jan 2026 16:03:53 +0100 Subject: [PATCH] Replace signal() with sigaction() and add signal flags support Replace the POSIX signal() implementation with sigaction() and add support for signal flags (SA_RESTART, SA_NOCLDSTOP, etc.) while keeping OS-specific headers out of the public API. Public API changes: - Add flags_t enum directly to signal_set with abstract bit values - Add signal_set::add(int, flags_t) overload - Existing add(int) becomes inline convenience calling add(signal, none) - Add bitwise operators for flag manipulation (|, &, |=, &=, ~) Available flags: none, restart, no_child_stop, no_child_wait, no_defer, reset_handler, dont_care POSIX implementation (posix/signals.cpp): - Replace signal() with sigaction() for robust signal handling - Add flags_supported() to validate flags available on this platform - Add to_sigaction_flags() to map abstract flags to SA_* constants - Add flag conflict detection for multiple signal_sets on same signal - Remove handler re-registration (not needed with sigaction) - Track registered flags in global signal_state for cross-set validation - Return operation_not_supported if SA_NOCLDWAIT unavailable - Document async-signal-safety limitation in signal handler Windows implementation (win/signals.cpp): - Only none and dont_care flags are supported - Other flags return operation_not_supported Testing: Cross-platform tests for none/dont_care, POSIX-only tests for actual flag behavior, Windows-only test for operation_not_supported. --- doc/modules/ROOT/pages/guide/signals.adoc | 110 +++++++++- include/boost/corosio/signal_set.hpp | 137 ++++++++++++- src/corosio/src/detail/posix/signals.cpp | 216 ++++++++++++++++++-- src/corosio/src/detail/posix/signals.hpp | 33 ++- src/corosio/src/detail/win/signals.cpp | 24 ++- src/corosio/src/detail/win/signals.hpp | 29 ++- test/unit/signal_set.cpp | 236 ++++++++++++++++++++++ 7 files changed, 749 insertions(+), 36 deletions(-) diff --git a/doc/modules/ROOT/pages/guide/signals.adoc b/doc/modules/ROOT/pages/guide/signals.adoc index dd2bff6..f4ae440 100644 --- a/doc/modules/ROOT/pages/guide/signals.adoc +++ b/doc/modules/ROOT/pages/guide/signals.adoc @@ -100,16 +100,75 @@ Add a signal to the set: [source,cpp] ---- signals.add(SIGUSR1); - -// With error code -boost::system::error_code ec; -signals.add(SIGUSR1, ec); -if (ec) - std::cerr << "Failed to add signal: " << ec.message() << "\n"; ---- Adding a signal that's already in the set has no effect. +==== Signal Flags (POSIX) + +On POSIX systems, you can specify signal flags when adding a signal: + +[source,cpp] +---- +using flags = corosio::signal_set; + +// Restart interrupted system calls automatically +signals.add(SIGCHLD, flags::restart); + +// Multiple flags can be combined +signals.add(SIGCHLD, flags::restart | flags::no_child_stop); +---- + +Available flags: + +[cols="1,2"] +|=== +| Flag | Description + +| `none` +| No special flags (default) + +| `restart` +| Automatically restart interrupted system calls (SA_RESTART) + +| `no_child_stop` +| Don't generate SIGCHLD when children stop (SA_NOCLDSTOP) + +| `no_child_wait` +| Don't create zombie processes on child termination (SA_NOCLDWAIT) + +| `no_defer` +| Don't block the signal while its handler runs (SA_NODEFER) + +| `reset_handler` +| Reset handler to SIG_DFL after one invocation (SA_RESETHAND) + +| `dont_care` +| Accept existing flags if signal is already registered +|=== + +NOTE: On Windows, only `none` and `dont_care` flags are supported. On some POSIX +systems, `no_child_wait` may not be available. Using unsupported flags returns +`operation_not_supported`. + +==== Flag Compatibility + +When multiple `signal_set` objects register for the same signal, they must use +compatible flags: + +[source,cpp] +---- +corosio::signal_set s1(ioc); +corosio::signal_set s2(ioc); + +s1.add(SIGINT, flags::restart); // OK - first registration +s2.add(SIGINT, flags::restart); // OK - same flags +s2.add(SIGINT, flags::no_defer); // Error! - different flags + +// Use dont_care to accept existing flags +s2.add(SIGINT, flags::dont_care); // OK - accepts existing flags +---- + === remove() Remove a signal from the set: @@ -253,6 +312,32 @@ capy::task config_reloader( } ---- +=== Child Process Management (POSIX) + +[source,cpp] +---- +capy::task child_reaper(corosio::io_context& ioc) +{ + using flags = corosio::signal_set; + + corosio::signal_set signals(ioc); + + // Only notify on child termination, not stop/continue + // Prevent zombie processes automatically + signals.add(SIGCHLD, flags::no_child_stop | flags::no_child_wait); + + for (;;) + { + auto [ec, signum] = co_await signals.async_wait(); + if (ec) + break; + + // With no_child_wait, children are reaped automatically + std::cout << "Child process terminated\n"; + } +} +---- + == Move Semantics Signal sets are move-only: @@ -323,12 +408,19 @@ capy::task run_server(corosio::io_context& ioc) === Windows Windows has limited signal support. The library uses `signal()` from the -C runtime for compatibility. +C runtime for compatibility. Only `none` and `dont_care` flags are supported; +other flags return `operation_not_supported`. === POSIX -On POSIX systems, signals are handled using platform-native mechanisms -for reliable delivery. +On POSIX systems, signals are handled using `sigaction()` which provides: + +* Reliable signal delivery (handler doesn't reset to SIG_DFL) +* Support for signal flags (SA_RESTART, SA_NOCLDSTOP, etc.) +* Proper signal masking during handler execution + +The `restart` flag is particularly useful—without it, blocking calls like +`read()` can fail with `EINTR` when a signal arrives. == Next Steps diff --git a/include/boost/corosio/signal_set.hpp b/include/boost/corosio/signal_set.hpp index fbf1413..82c7a8e 100644 --- a/include/boost/corosio/signal_set.hpp +++ b/include/boost/corosio/signal_set.hpp @@ -26,6 +26,34 @@ #include #include +/* + Signal Set Public API + ===================== + + This header provides the public interface for asynchronous signal handling. + The implementation is split across platform-specific files: + - posix/signals.cpp: Uses sigaction() for robust signal handling + - win/signals.cpp: Uses C runtime signal() (Windows lacks sigaction) + + Key design decisions: + + 1. Abstract flag values: The flags_t enum uses arbitrary bit positions + (not SA_RESTART, etc.) to avoid including in public headers. + The POSIX implementation maps these to actual SA_* constants internally. + + 2. Flag conflict detection: When multiple signal_sets register for the + same signal, they must use compatible flags. The first registration + establishes the flags; subsequent registrations must match or use + dont_care. + + 3. Polymorphic implementation: signal_set_impl is an abstract base that + platform-specific implementations (posix_signal_impl, win_signal_impl) + derive from. This allows the public API to be platform-agnostic. + + 4. The inline add(int) overload avoids a virtual call for the common case + of adding signals without flags (delegates to add(int, none)). +*/ + namespace boost { namespace corosio { @@ -33,7 +61,8 @@ namespace corosio { This class provides the ability to perform an asynchronous wait for one or more signals to occur. The signal set registers for - signals using the C runtime signal() function. + signals using sigaction() on POSIX systems or the C runtime + signal() function on Windows. @par Thread Safety Distinct objects: Safe.@n @@ -54,6 +83,81 @@ namespace corosio { */ class BOOST_COROSIO_DECL signal_set : public io_object { +public: + /** Flags for signal registration. + + These flags control the behavior of signal handling. Multiple + flags can be combined using the bitwise OR operator. + + @note Flags only have effect on POSIX systems. On Windows, + only `none` and `dont_care` are supported; other flags return + `operation_not_supported`. + */ + enum flags_t : unsigned + { + /// Use existing flags if signal is already registered. + /// When adding a signal that's already registered by another + /// signal_set, this flag indicates acceptance of whatever + /// flags were used for the existing registration. + dont_care = 1u << 16, + + /// No special flags. + none = 0, + + /// Restart interrupted system calls. + /// Equivalent to SA_RESTART on POSIX systems. + restart = 1u << 0, + + /// Don't generate SIGCHLD when children stop. + /// Equivalent to SA_NOCLDSTOP on POSIX systems. + no_child_stop = 1u << 1, + + /// Don't create zombie processes on child termination. + /// Equivalent to SA_NOCLDWAIT on POSIX systems. + no_child_wait = 1u << 2, + + /// Don't block the signal while its handler runs. + /// Equivalent to SA_NODEFER on POSIX systems. + no_defer = 1u << 3, + + /// Reset handler to SIG_DFL after one invocation. + /// Equivalent to SA_RESETHAND on POSIX systems. + reset_handler = 1u << 4 + }; + + /// Combine two flag values. + friend constexpr flags_t operator|(flags_t a, flags_t b) noexcept + { + return static_cast( + static_cast(a) | static_cast(b)); + } + + /// Mask two flag values. + friend constexpr flags_t operator&(flags_t a, flags_t b) noexcept + { + return static_cast( + static_cast(a) & static_cast(b)); + } + + /// Compound assignment OR. + friend constexpr flags_t& operator|=(flags_t& a, flags_t b) noexcept + { + return a = a | b; + } + + /// Compound assignment AND. + friend constexpr flags_t& operator&=(flags_t& a, flags_t b) noexcept + { + return a = a & b; + } + + /// Bitwise NOT (complement). + friend constexpr flags_t operator~(flags_t a) noexcept + { + return static_cast(~static_cast(a)); + } + +private: struct wait_awaitable { signal_set& s_; @@ -97,7 +201,7 @@ class BOOST_COROSIO_DECL signal_set : public io_object system::error_code*, int*) = 0; - virtual system::result add(int signal_number) = 0; + virtual system::result add(int signal_number, flags_t flags) = 0; virtual system::result remove(int signal_number) = 0; virtual system::result clear() = 0; virtual void cancel() = 0; @@ -161,14 +265,37 @@ class BOOST_COROSIO_DECL signal_set : public io_object /** Add a signal to the signal set. - This function adds the specified signal to the set. It has no - effect if the signal is already in the set. + This function adds the specified signal to the set with the + specified flags. It has no effect if the signal is already + in the set with the same flags. + + If the signal is already registered globally (by another + signal_set) and the flags differ, an error is returned + unless one of them has the `dont_care` flag. + + @param signal_number The signal to be added to the set. + @param flags The flags to apply when registering the signal. + On POSIX systems, these map to sigaction() flags. + On Windows, flags are accepted but ignored. + + @return Success, or an error if the signal could not be added. + Returns `errc::invalid_argument` if the signal is already + registered with different flags. + */ + system::result add(int signal_number, flags_t flags); + + /** Add a signal to the signal set with default flags. + + This is equivalent to calling `add(signal_number, none)`. @param signal_number The signal to be added to the set. @return Success, or an error if the signal could not be added. */ - system::result add(int signal_number); + system::result add(int signal_number) + { + return add(signal_number, none); + } /** Remove a signal from the signal set. diff --git a/src/corosio/src/detail/posix/signals.cpp b/src/corosio/src/detail/posix/signals.cpp index 8a4d853..092bfb3 100644 --- a/src/corosio/src/detail/posix/signals.cpp +++ b/src/corosio/src/detail/posix/signals.cpp @@ -20,6 +20,103 @@ #include +/* + POSIX Signal Implementation + =========================== + + This file implements signal handling for POSIX systems using sigaction(). + The implementation supports signal flags (SA_RESTART, etc.) and integrates + with the epoll-based scheduler. + + Architecture Overview + --------------------- + + Three layers manage signal registrations: + + 1. signal_state (global singleton) + - Tracks the global service list and per-signal registration counts + - Stores the flags used for first registration of each signal (for + conflict detection when multiple signal_sets register same signal) + - Owns the mutex that protects signal handler installation/removal + + 2. posix_signals (one per execution_context) + - Maintains registrations_[] table indexed by signal number + - Each slot is a doubly-linked list of signal_registrations for that signal + - Also maintains impl_list_ of all posix_signal_impl objects it owns + + 3. posix_signal_impl (one per signal_set) + - Owns a singly-linked list (sorted by signal number) of signal_registrations + - Contains the pending_op_ used for async_wait operations + + Signal Delivery Flow + -------------------- + + 1. Signal arrives -> corosio_posix_signal_handler() (must be async-signal-safe) + -> deliver_signal() + + 2. deliver_signal() iterates all posix_signals services: + - If a signal_set is waiting (impl->waiting_ == true), post the signal_op + to the scheduler for immediate completion + - Otherwise, increment reg->undelivered to queue the signal + + 3. When async_wait() is called via start_wait(): + - First check for queued signals (undelivered > 0); if found, post + immediate completion without blocking + - Otherwise, set waiting_ = true and call on_work_started() to keep + the io_context alive + + Locking Protocol + ---------------- + + Two mutex levels exist (MUST acquire in this order to avoid deadlock): + 1. signal_state::mutex - protects handler registration and service list + 2. posix_signals::mutex_ - protects per-service registration tables + + Async-Signal-Safety Limitation + ------------------------------ + + IMPORTANT: deliver_signal() is called from signal handler context and + acquires mutexes. This is NOT strictly async-signal-safe per POSIX. + The limitation: + - If a signal arrives while another thread holds state->mutex or + service->mutex_, and that same thread receives the signal, a + deadlock can occur (self-deadlock on non-recursive mutex). + + This design trades strict async-signal-safety for implementation simplicity. + In practice, deadlocks are rare because: + - Mutexes are held only briefly during registration changes + - Most programs don't modify signal sets while signals are expected + - The window for signal arrival during mutex hold is small + + A fully async-signal-safe implementation would require lock-free data + structures and atomic operations throughout, significantly increasing + complexity. + + Flag Handling + ------------- + + - Flags are abstract values in the public API (signal_set::flags_t) + - flags_supported() validates that requested flags are available on + this platform; returns false if SA_NOCLDWAIT is unavailable and + no_child_wait is requested + - to_sigaction_flags() maps validated flags to actual SA_* constants + - First registration of a signal establishes the flags; subsequent + registrations must be compatible (same flags or dont_care) + - Requesting unavailable flags returns operation_not_supported + + Work Tracking + ------------- + + When waiting for a signal: + - start_wait() calls sched_.on_work_started() to prevent io_context::run() + from returning while we wait + - signal_op::svc is set to point to the service + - signal_op::operator()() calls work_finished() after resuming the coroutine + + If a signal was already queued (undelivered > 0), no work tracking is needed + because completion is posted immediately. +*/ + namespace boost { namespace corosio { namespace detail { @@ -37,6 +134,7 @@ struct signal_state std::mutex mutex; posix_signals* service_list = nullptr; std::size_t registration_count[max_signal_number] = {}; + signal_set::flags_t registered_flags[max_signal_number] = {}; }; signal_state* get_signal_state() @@ -45,13 +143,58 @@ signal_state* get_signal_state() return &state; } +// Check if requested flags are supported on this platform. +// Returns true if all flags are supported, false otherwise. +bool flags_supported(signal_set::flags_t flags) +{ +#ifndef SA_NOCLDWAIT + if (flags & signal_set::no_child_wait) + return false; +#endif + return true; +} + +// Map abstract flags to sigaction() flags. +// Caller must ensure flags_supported() returns true first. +int to_sigaction_flags(signal_set::flags_t flags) +{ + int sa_flags = 0; + if (flags & signal_set::restart) + sa_flags |= SA_RESTART; + if (flags & signal_set::no_child_stop) + sa_flags |= SA_NOCLDSTOP; +#ifdef SA_NOCLDWAIT + if (flags & signal_set::no_child_wait) + sa_flags |= SA_NOCLDWAIT; +#endif + if (flags & signal_set::no_defer) + sa_flags |= SA_NODEFER; + if (flags & signal_set::reset_handler) + sa_flags |= SA_RESETHAND; + return sa_flags; +} + +// Check if two flag values are compatible +bool flags_compatible( + signal_set::flags_t existing, + signal_set::flags_t requested) +{ + // dont_care is always compatible + if ((existing & signal_set::dont_care) || + (requested & signal_set::dont_care)) + return true; + + // Mask out dont_care bit for comparison + constexpr auto mask = ~signal_set::dont_care; + return (existing & mask) == (requested & mask); +} + // C signal handler - must be async-signal-safe extern "C" void corosio_posix_signal_handler(int signal_number) { posix_signals::deliver_signal(signal_number); - - // Re-register handler (some systems reset to SIG_DFL after each signal) - ::signal(signal_number, corosio_posix_signal_handler); + // Note: With sigaction(), the handler persists automatically + // (unlike some signal() implementations that reset to SIG_DFL) } } // namespace @@ -140,9 +283,9 @@ wait( system::result posix_signal_impl:: -add(int signal_number) +add(int signal_number, signal_set::flags_t flags) { - return svc_.add_signal(*this, signal_number); + return svc_.add_signal(*this, signal_number, flags); } system::result @@ -238,11 +381,17 @@ system::result posix_signals:: add_signal( posix_signal_impl& impl, - int signal_number) + int signal_number, + signal_set::flags_t flags) { if (signal_number < 0 || signal_number >= max_signal_number) return make_error_code(system::errc::invalid_argument); + // Validate that requested flags are supported on this platform + // (e.g., SA_NOCLDWAIT may not be available on all POSIX systems) + if (!flags_supported(flags)) + return make_error_code(system::errc::operation_not_supported); + signal_state* state = get_signal_state(); std::lock_guard state_lock(state->mutex); std::lock_guard lock(mutex_); @@ -256,22 +405,45 @@ add_signal( reg = reg->next_in_set; } + // Already registered in this set - check flag compatibility + // (same signal_set adding same signal twice with different flags) if (reg && reg->signal_number == signal_number) + { + if (!flags_compatible(reg->flags, flags)) + return make_error_code(system::errc::invalid_argument); return {}; + } + + // Check flag compatibility with global registration + // (different signal_set already registered this signal with different flags) + if (state->registration_count[signal_number] > 0) + { + if (!flags_compatible(state->registered_flags[signal_number], flags)) + return make_error_code(system::errc::invalid_argument); + } auto* new_reg = new signal_registration; new_reg->signal_number = signal_number; + new_reg->flags = flags; new_reg->owner = &impl; new_reg->undelivered = 0; // Install signal handler on first global registration if (state->registration_count[signal_number] == 0) { - if (::signal(signal_number, corosio_posix_signal_handler) == SIG_ERR) + struct sigaction sa = {}; + sa.sa_handler = corosio_posix_signal_handler; + sigemptyset(&sa.sa_mask); + sa.sa_flags = to_sigaction_flags(flags); + + if (::sigaction(signal_number, &sa, nullptr) < 0) { delete new_reg; return make_error_code(system::errc::invalid_argument); } + + // Store the flags used for first registration + state->registered_flags[signal_number] = flags; } new_reg->next_in_set = reg; @@ -316,8 +488,16 @@ remove_signal( // Restore default handler on last global unregistration if (state->registration_count[signal_number] == 1) { - if (::signal(signal_number, SIG_DFL) == SIG_ERR) + struct sigaction sa = {}; + sa.sa_handler = SIG_DFL; + sigemptyset(&sa.sa_mask); + sa.sa_flags = 0; + + if (::sigaction(signal_number, &sa, nullptr) < 0) return make_error_code(system::errc::invalid_argument); + + // Clear stored flags + state->registered_flags[signal_number] = signal_set::none; } *deletion_point = reg->next_in_set; @@ -352,8 +532,16 @@ clear_signals(posix_signal_impl& impl) if (state->registration_count[signal_number] == 1) { - if (::signal(signal_number, SIG_DFL) == SIG_ERR && !first_error) + struct sigaction sa = {}; + sa.sa_handler = SIG_DFL; + sigemptyset(&sa.sa_mask); + sa.sa_flags = 0; + + if (::sigaction(signal_number, &sa, nullptr) < 0 && !first_error) first_error = make_error_code(system::errc::invalid_argument); + + // Clear stored flags + state->registered_flags[signal_number] = signal_set::none; } impl.signals_ = reg->next_in_set; @@ -411,6 +599,7 @@ start_wait(posix_signal_impl& impl, signal_op* op) { std::lock_guard lock(mutex_); + // Check for queued signals first (signal arrived before wait started) signal_registration* reg = impl.signals_; while (reg) { @@ -418,6 +607,7 @@ start_wait(posix_signal_impl& impl, signal_op* op) { --reg->undelivered; op->signal_number = reg->signal_number; + // svc=nullptr: no work_finished needed since we never called work_started op->svc = nullptr; sched_.post(op); return; @@ -425,9 +615,9 @@ start_wait(posix_signal_impl& impl, signal_op* op) reg = reg->next_in_set; } - // No queued signals - wait for delivery. - // svc is set so signal_op::operator() calls work_finished(). + // No queued signals - wait for delivery impl.waiting_ = true; + // svc=this: signal_op::operator() will call work_finished() to balance this op->svc = this; sched_.on_work_started(); } @@ -576,9 +766,9 @@ operator=(signal_set&& other) system::result signal_set:: -add(int signal_number) +add(int signal_number, flags_t flags) { - return get().add(signal_number); + return get().add(signal_number, flags); } system::result diff --git a/src/corosio/src/detail/posix/signals.hpp b/src/corosio/src/detail/posix/signals.hpp index eb13553..7b27425 100644 --- a/src/corosio/src/detail/posix/signals.hpp +++ b/src/corosio/src/detail/posix/signals.hpp @@ -32,6 +32,32 @@ #include +/* + POSIX Signal Implementation - Header + ===================================== + + This header declares the internal types for POSIX signal handling. + See signals.cpp for the full implementation overview. + + Data Structure Summary: + + signal_op - Pending async_wait operation, posted to scheduler on signal + signal_registration - Links a signal to its owning signal_set; tracks queued signals + posix_signal_impl - Per-signal_set state (derives from signal_set::signal_set_impl) + posix_signals - Per-execution_context service managing all signal_sets + + Pointer Relationships: + + signal_registration has two linked list memberships: + - next_in_set: Singly-linked list of all signals in one signal_set (sorted) + - prev/next_in_table: Doubly-linked list of all registrations for one signal + number across all signal_sets in one execution_context + + This dual-linking allows efficient: + - Per-set iteration (add/remove/clear operations) + - Per-signal iteration (signal delivery to all waiting sets) +*/ + namespace boost { namespace corosio { namespace detail { @@ -65,6 +91,7 @@ struct signal_op : scheduler_op struct signal_registration { int signal_number = 0; + signal_set::flags_t flags = signal_set::none; posix_signal_impl* owner = nullptr; std::size_t undelivered = 0; signal_registration* next_in_table = nullptr; @@ -104,7 +131,7 @@ class posix_signal_impl system::error_code*, int*) override; - system::result add(int signal_number) override; + system::result add(int signal_number, signal_set::flags_t flags) override; system::result remove(int signal_number) override; system::result clear() override; void cancel() override; @@ -155,11 +182,13 @@ class posix_signals : public capy::execution_context::service @param impl The signal implementation to modify. @param signal_number The signal to register. + @param flags The flags to apply when registering the signal. @return Success, or an error. */ system::result add_signal( posix_signal_impl& impl, - int signal_number); + int signal_number, + signal_set::flags_t flags); /** Remove a signal from a signal set. diff --git a/src/corosio/src/detail/win/signals.cpp b/src/corosio/src/detail/win/signals.cpp index a58e217..449b679 100644 --- a/src/corosio/src/detail/win/signals.cpp +++ b/src/corosio/src/detail/win/signals.cpp @@ -88,6 +88,14 @@ If a signal was already queued (undelivered > 0), no work tracking is needed because completion is posted immediately. + + Signal Flags + ------------ + + Windows only supports `none` and `dont_care` flags. Any other flags + (restart, no_child_stop, etc.) return `operation_not_supported`. The + C runtime signal() function has no equivalent to sigaction() flags + like SA_RESTART or SA_NOCLDSTOP. */ namespace boost { @@ -217,9 +225,9 @@ wait( system::result win_signal_impl:: -add(int signal_number) +add(int signal_number, signal_set::flags_t flags) { - return svc_.add_signal(*this, signal_number); + return svc_.add_signal(*this, signal_number, flags); } system::result @@ -314,11 +322,17 @@ system::result win_signals:: add_signal( win_signal_impl& impl, - int signal_number) + int signal_number, + signal_set::flags_t flags) { if (signal_number < 0 || signal_number >= max_signal_number) return make_error_code(system::errc::invalid_argument); + // Windows only supports none and dont_care flags + constexpr auto supported = signal_set::none | signal_set::dont_care; + if ((flags & ~supported) != signal_set::none) + return make_error_code(system::errc::operation_not_supported); + signal_state* state = get_signal_state(); std::lock_guard state_lock(state->mutex); std::lock_guard lock(mutex_); @@ -668,9 +682,9 @@ operator=(signal_set&& other) system::result signal_set:: -add(int signal_number) +add(int signal_number, flags_t flags) { - return get().add(signal_number); + return get().add(signal_number, flags); } system::result diff --git a/src/corosio/src/detail/win/signals.hpp b/src/corosio/src/detail/win/signals.hpp index 7732ab3..77e6b2f 100644 --- a/src/corosio/src/detail/win/signals.hpp +++ b/src/corosio/src/detail/win/signals.hpp @@ -32,6 +32,29 @@ #include +/* + Windows Signal Implementation - Header + ====================================== + + This header declares the internal types for Windows signal handling. + See signals.cpp for the full implementation overview. + + Key Differences from POSIX: + - Uses C runtime signal() instead of sigaction() (Windows has no sigaction) + - Only `none` and `dont_care` flags are supported; other flags return + `operation_not_supported` (Windows has no equivalent to SA_* flags) + - Windows resets handler to SIG_DFL after each signal, so we must re-register + - Only supports: SIGINT, SIGTERM, SIGABRT, SIGFPE, SIGILL, SIGSEGV + - max_signal_number is 32 (vs 64 on Linux) + + The data structures mirror the POSIX implementation for consistency: + - signal_op, signal_registration, win_signal_impl, win_signals + + Threading note: Windows signal handling is synchronous (runs on faulting + thread), so we can safely acquire locks in the signal handler. This differs + from POSIX where the handler must be async-signal-safe. +*/ + namespace boost { namespace corosio { namespace detail { @@ -105,7 +128,7 @@ class win_signal_impl system::error_code*, int*) override; - system::result add(int signal_number) override; + system::result add(int signal_number, signal_set::flags_t flags) override; system::result remove(int signal_number) override; system::result clear() override; void cancel() override; @@ -158,11 +181,13 @@ class win_signals : public capy::execution_context::service @param impl The signal implementation to modify. @param signal_number The signal to register. + @param flags The flags to apply (ignored on Windows). @return Success, or an error. */ system::result add_signal( win_signal_impl& impl, - int signal_number); + int signal_number, + signal_set::flags_t flags); /** Remove a signal from a signal set. diff --git a/test/unit/signal_set.cpp b/test/unit/signal_set.cpp index 29dc71b..5ae95d4 100644 --- a/test/unit/signal_set.cpp +++ b/test/unit/signal_set.cpp @@ -541,6 +541,220 @@ struct signal_set_test BOOST_TEST_EQ(captured_signal, SIGINT); } + //-------------------------------------------- + // Signal flags tests (cross-platform) + //-------------------------------------------- + + void + testFlagsBitwiseOperations() + { + // Test OR + auto combined = signal_set::restart | signal_set::no_defer; + BOOST_TEST((combined & signal_set::restart) != signal_set::none); + BOOST_TEST((combined & signal_set::no_defer) != signal_set::none); + BOOST_TEST((combined & signal_set::no_child_stop) == signal_set::none); + + // Test compound assignment + auto flags = signal_set::none; + flags |= signal_set::restart; + BOOST_TEST((flags & signal_set::restart) != signal_set::none); + + // Test NOT + auto all_but_restart = ~signal_set::restart; + BOOST_TEST((all_but_restart & signal_set::restart) == signal_set::none); + } + + void + testAddWithNoneFlags() + { + io_context ioc; + signal_set s(ioc); + + // Add signal with none (default behavior) - works on all platforms + auto result = s.add(SIGINT, signal_set::none); + BOOST_TEST(result.has_value()); + } + + void + testAddWithDontCareFlags() + { + io_context ioc; + signal_set s(ioc); + + // Add signal with dont_care - works on all platforms + auto result = s.add(SIGINT, signal_set::dont_care); + BOOST_TEST(result.has_value()); + } + +#if !defined(_WIN32) + //-------------------------------------------- + // Signal flags tests (POSIX only) + // Windows returns operation_not_supported for + // flags other than none/dont_care + //-------------------------------------------- + + void + testAddWithFlags() + { + io_context ioc; + signal_set s(ioc); + + // Add signal with restart flag + auto result = s.add(SIGINT, signal_set::restart); + BOOST_TEST(result.has_value()); + } + + void + testAddWithMultipleFlags() + { + io_context ioc; + signal_set s(ioc); + + // Add signal with combined flags + auto result = s.add(SIGINT, signal_set::restart | signal_set::no_defer); + BOOST_TEST(result.has_value()); + } + + void + testAddSameSignalSameFlags() + { + io_context ioc; + signal_set s(ioc); + + // Add signal twice with same flags (should be no-op) + BOOST_TEST(s.add(SIGINT, signal_set::restart).has_value()); + BOOST_TEST(s.add(SIGINT, signal_set::restart).has_value()); + } + + void + testAddSameSignalDifferentFlags() + { + io_context ioc; + signal_set s(ioc); + + // Add signal with one flag, then try to add with different flag + BOOST_TEST(s.add(SIGINT, signal_set::restart).has_value()); + auto result = s.add(SIGINT, signal_set::no_defer); + BOOST_TEST(result.has_error()); // Should fail due to flag mismatch + } + + void + testAddSameSignalWithDontCare() + { + io_context ioc; + signal_set s(ioc); + + // Add signal with specific flags, then add with dont_care + BOOST_TEST(s.add(SIGINT, signal_set::restart).has_value()); + auto result = s.add(SIGINT, signal_set::dont_care); + BOOST_TEST(result.has_value()); // Should succeed with dont_care + } + + void + testAddSameSignalDontCareFirst() + { + io_context ioc; + signal_set s(ioc); + + // Add signal with dont_care, then add with specific flags + BOOST_TEST(s.add(SIGINT, signal_set::dont_care).has_value()); + auto result = s.add(SIGINT, signal_set::restart); + BOOST_TEST(result.has_value()); // Should succeed + } + + void + testMultipleSetsCompatibleFlags() + { + io_context ioc; + signal_set s1(ioc); + signal_set s2(ioc); + + // Both sets add same signal with same flags + BOOST_TEST(s1.add(SIGINT, signal_set::restart).has_value()); + BOOST_TEST(s2.add(SIGINT, signal_set::restart).has_value()); + } + + void + testMultipleSetsIncompatibleFlags() + { + io_context ioc; + signal_set s1(ioc); + signal_set s2(ioc); + + // First set adds with one flag + BOOST_TEST(s1.add(SIGINT, signal_set::restart).has_value()); + // Second set tries to add with different flag + auto result = s2.add(SIGINT, signal_set::no_defer); + BOOST_TEST(result.has_error()); // Should fail + } + + void + testMultipleSetsWithDontCare() + { + io_context ioc; + signal_set s1(ioc); + signal_set s2(ioc); + + // First set adds with specific flags + BOOST_TEST(s1.add(SIGINT, signal_set::restart).has_value()); + // Second set adds with dont_care + BOOST_TEST(s2.add(SIGINT, signal_set::dont_care).has_value()); + } + + void + testWaitWithFlagsWorks() + { + io_context ioc; + signal_set s(ioc); + timer t(ioc); + + // Add signal with restart flag and verify wait still works + BOOST_TEST(s.add(SIGINT, signal_set::restart).has_value()); + + bool completed = false; + int received_signal = 0; + + auto wait_task = [](signal_set& s_ref, int& sig_out, bool& done_out) -> capy::task<> + { + auto [ec, signum] = co_await s_ref.async_wait(); + sig_out = signum; + done_out = true; + (void)ec; + }; + capy::run_async(ioc.get_executor())(wait_task(s, received_signal, completed)); + + t.expires_after(std::chrono::milliseconds(10)); + auto raise_task = [](timer& t_ref) -> capy::task<> + { + co_await t_ref.wait(); + std::raise(SIGINT); + }; + capy::run_async(ioc.get_executor())(raise_task(t)); + + ioc.run(); + BOOST_TEST(completed); + BOOST_TEST_EQ(received_signal, SIGINT); + } + +#else // _WIN32 + //-------------------------------------------- + // Signal flags tests (Windows only) + //-------------------------------------------- + + void + testFlagsNotSupportedOnWindows() + { + io_context ioc; + signal_set s(ioc); + + // Windows returns operation_not_supported for actual flags + auto result = s.add(SIGINT, signal_set::restart); + BOOST_TEST(result.has_error()); + BOOST_TEST(result.error() == system::errc::operation_not_supported); + } + +#endif // _WIN32 + void run() { @@ -585,6 +799,28 @@ struct signal_set_test testIoResultSuccess(); testIoResultCanceled(); testIoResultStructuredBinding(); + + // Signal flags tests (cross-platform) + testFlagsBitwiseOperations(); + testAddWithNoneFlags(); + testAddWithDontCareFlags(); + +#if !defined(_WIN32) + // Signal flags tests (POSIX only) + testAddWithFlags(); + testAddWithMultipleFlags(); + testAddSameSignalSameFlags(); + testAddSameSignalDifferentFlags(); + testAddSameSignalWithDontCare(); + testAddSameSignalDontCareFirst(); + testMultipleSetsCompatibleFlags(); + testMultipleSetsIncompatibleFlags(); + testMultipleSetsWithDontCare(); + testWaitWithFlagsWorks(); +#else + // Signal flags tests (Windows only) + testFlagsNotSupportedOnWindows(); +#endif } };