From 907c1f90856204bc8ecfca05c6d2cd7d2a5b523a Mon Sep 17 00:00:00 2001 From: Noah Oblath Date: Mon, 8 Dec 2025 14:37:29 -0800 Subject: [PATCH 01/29] Removing Windows compatibility for signal handling; Adapting to use of sigwait; does not currently work --- library/utility/signal_handler.cc | 367 ++++++++-------------- library/utility/signal_handler.hh | 44 +-- testing/applications/test_raise_sigint.cc | 31 +- testing/test_signal_handler.cc | 73 +---- 4 files changed, 200 insertions(+), 315 deletions(-) diff --git a/library/utility/signal_handler.cc b/library/utility/signal_handler.cc index 5ccb7afe..de4f9c22 100644 --- a/library/utility/signal_handler.cc +++ b/library/utility/signal_handler.cc @@ -15,13 +15,8 @@ #include #include -#include - -#ifdef _WIN32 -#include -#include -//#include "processthreadsapi.h" -#endif +#include +#include namespace { // function to catch unhandled exceptions @@ -39,32 +34,34 @@ namespace scarab bool signal_handler::s_exited = false; int signal_handler::s_return_code = RETURN_SUCCESS; - bool signal_handler::s_handling_sig_abrt = false; - bool signal_handler::s_handling_sig_term = false; - bool signal_handler::s_handling_sig_int = false; - bool signal_handler::s_handling_sig_quit = false; - -#ifndef _WIN32 - struct sigaction signal_handler::s_old_sig_abrt_action; - struct sigaction signal_handler::s_old_sig_term_action; - struct sigaction signal_handler::s_old_sig_int_action; - struct sigaction signal_handler::s_old_sig_quit_action; -#else // _WIN32 - signal_handler::handler_t signal_handler::s_old_sig_abrt_handler = SIG_DFL; - signal_handler::handler_t signal_handler::s_old_sig_term_handler = SIG_DFL; - signal_handler::handler_t signal_handler::s_old_sig_int_handler = SIG_DFL; - signal_handler::handler_t signal_handler::s_old_sig_quit_handler = SIG_DFL; -#endif - std::recursive_mutex signal_handler::s_mutex; signal_handler::cancelers signal_handler::s_cancelers; + sigset_t signal_handler::s_sigset; + std::mutex signal_handler::s_handle_mutex; + signal_handler::signal_handler() { //std::cerr << "signal_handler constructor" << std::endl; ++signal_handler::s_ref_count; + if( signal_handler::s_ref_count == 1 ) + { + LWARN( slog, "Blocking signals SIGABRT, SIGTERM, SIGINT, SIGQUIT, and SIGUSR1" ); + + sigemptyset( &s_sigset ); + sigaddset( &s_sigset, SIGABRT ); + sigaddset( &s_sigset, SIGTERM ); + sigaddset( &s_sigset, SIGINT ); + sigaddset( &s_sigset, SIGQUIT ); + sigaddset( &s_sigset, SIGUSR1); - signal_handler::handle_signals(); + if( pthread_sigmask( SIG_BLOCK, &s_sigset, nullptr ) != 0 ) + { + LERROR( slog, "Unable to block the required signals" ); + std::cerr << "Unable to block the required signals" << std::endl; + throw error(__FILE_NAME__, __LINE__) << "Unable to block the required signals"; + } + } } signal_handler::~signal_handler() @@ -73,7 +70,7 @@ namespace scarab --signal_handler::s_ref_count; if( signal_handler::s_ref_count == 0 ) { - signal_handler::unhandle_signals(); + abort_handle_signals(); } } @@ -109,245 +106,159 @@ namespace scarab return; } - void signal_handler::handle_signals() + int signal_handler::do_handle_signals() { - std::unique_lock< std::recursive_mutex > t_lock( s_mutex ); + std::unique_lock t_lock( s_handle_mutex, std::defer_lock_t() ); + if( ! t_lock.try_lock() ) return -1; - // we create a new logger here so that handle_signals() can be called from the signal_handler constructor during static initalization - LOGGER( slog_constr, "signal_handler handle_signals" ); + LWARN( slog, "Starting to handle signals SIGABRT, SIGTERM, SIGINT, SIGQUIT, and SIGUSR1" ); - LDEBUG( slog_constr, "Taking over signal handling for SIGABRT, SIGTERM, SIGINT, and SIGQUIT" ); + auto sigwait_fcn = []() { + LOGGER( sigwait_log, "signal_handler handle_signals" ); -#ifndef _WIN32 // on a POSIX system we use sigaction + std::cerr << "In sigwait lambda" << std::endl; - struct sigaction t_exit_error_action, t_exit_success_action; + // wait until a signal is delivered, an abort was requested, or an error occurs + int signum = 0; + int error_num = sigwait( &s_sigset, &signum ); + std::cerr << "Done with sigwait" << std::endl; - t_exit_error_action.sa_handler = signal_handler::handle_exit_error; - sigemptyset(&t_exit_error_action.sa_mask); - t_exit_error_action.sa_flags = 0; + if( error_num != 0 ) + { + LERROR( sigwait_log, "An error occurred while waiting for signals: error code = " << error_num ); + std::cerr << "An error occurred while waiting for signals: error code = " << error_num << std::endl; + return -1; + } - t_exit_success_action.sa_handler = signal_handler::handle_exit_success; - sigemptyset(&t_exit_success_action.sa_mask); - t_exit_success_action.sa_flags = 0; + if( signum == SIGABRT || signum == SIGTERM ) + { + // An error-like signal was received + LERROR( sigwait_log, "Received signal <" << signum << "> as an error condition; return code: " << RETURN_ERROR ); + std::cerr << "Received signal <" << signum << "> as an error condition; return code: " << RETURN_ERROR << std::endl; + exit( RETURN_ERROR ); + } + else if( signum == SIGINT || signum == SIGQUIT ) + { + // A non-error-like signal was received + LWARN( sigwait_log, "Received signal <" << signum << ">; return code: " << RETURN_SUCCESS ); + std::cerr << "Received signal <" << signum << ">; return code: " << RETURN_SUCCESS << std::endl; + exit( RETURN_SUCCESS ); + } + else if( signum == SIGUSR1 ) + { + // A request to stop handling signals was received + LWARN( sigwait_log, "Received request to stop handling signals" ); + std::cerr << "Received request to stop handling signals" << std::endl; + } + else + { + // Unexpected signal + LERROR( sigwait_log, "An unexpected signal was recieved: " << signum ); + LERROR( sigwait_log, "Quitting with an error condition" ); + std::cerr << "An unexpected signal was recieved: " << signum << std::endl; + exit( RETURN_ERROR ); + } - // setup to handle SIGABRT - if( ! s_handling_sig_abrt && sigaction( SIGABRT, &t_exit_error_action, &s_old_sig_abrt_action ) != 0 ) - { - LWARN( slog_constr, "Unable to setup handling of SIGABRT: abort() and unhandled exceptions will result in an unclean exit" ); - } - else - { - LTRACE( slog_constr, "Handling SIGABRT (abort() and unhandled exceptions)" ); - s_handling_sig_abrt = true; - } + return signum; + }; - // setup to handle SIGTERM - if( ! s_handling_sig_term && sigaction( SIGTERM, &t_exit_error_action, &s_old_sig_term_action ) != 0 ) - { - LWARN( slog_constr, "Unable to setup handling of SIGTERM: SIGTERM will result in an unclean exit" ); - } - else - { - LTRACE( slog_constr, "Handling SIGTERM" ); - s_handling_sig_term = true; - } + auto ft_signal_handler = std::async( std::launch::async, sigwait_fcn ); - // setup to handle SIGINT - if( ! s_handling_sig_int && sigaction( SIGINT, &t_exit_success_action, &s_old_sig_int_action ) != 0 ) - { - LWARN( slog_constr, "Unable to setup handling of SIGINT: ctrl-c cancellation will result in an unclean exit" ); - } - else - { - LTRACE( slog_constr, "Handling SIGINT (ctrl-c)" ); - s_handling_sig_int = true; - } + int sig_received = ft_signal_handler.get(); + LWARN( slog, "Signal-waiting function exited; received signal " << sig_received ); + std::cerr << "Signal-waiting function exited; received signal " << sig_received << std::endl; - // setup to handle SIGQUIT - if( ! s_handling_sig_quit && sigaction( SIGQUIT, &t_exit_success_action, &s_old_sig_quit_action ) != 0 ) - { - LWARN( slog_constr, "Unable to setup handling of SIGQUIT: ctrl-\\ cancellation will result in an unclean exit" ); - } - else - { - LTRACE( slog_constr, "Handling SIGQUIT (ctrl-\\)" ); - s_handling_sig_quit = true; - } + return sig_received; + } - if( signal(SIGPIPE, SIG_IGN) == SIG_ERR ) - { - throw error() << "Unable to ignore SIGPIPE\n"; - } + int signal_handler::wait_on_signals() + { + LOGGER( sigwait_log, "signal_handler handle_signals" ); -#else // _WIN32; on a Windows system we use std::signal + LWARN( sigwait_log, "In wait-on-signals"); + std::cerr << "In wait-on-signals" << std::endl; - // setup to handle SIGABRT - if( ! s_handling_sig_abrt ) - { - auto t_sig_ret = signal( SIGABRT, signal_handler::handle_exit_error ); - if( t_sig_ret == SIG_ERR ) - { - LWARN( slog_constr, "Unable to setup handling of SIGABRT: abort() and unhandled exceptions will result in an unclean exit" ); - } - else + // wait until a signal is delivered, an abort was requested, or an error occurs + int signum = 0; + int error_num = sigwait( &s_sigset, &signum ); + LWARN( sigwait_log, "Done with sigwait" ); + std::cerr << "Done with sigwait" << std::endl; + + if( error_num != 0 ) { - s_handling_sig_abrt = true; - s_old_sig_abrt_handler = t_sig_ret; + LERROR( sigwait_log, "An error occurred while waiting for signals: error code = " << error_num ); + std::cerr << "An error occurred while waiting for signals: error code = " << error_num << std::endl; + return -1; } - } - if( s_handling_sig_abrt ) LTRACE( slog_constr, "Handling SIGABRT (abort() and unhandled exceptions)" ); - - // setup to handle SIGTERM - if( ! s_handling_sig_term ) - { - auto t_sig_ret = signal( SIGTERM, signal_handler::handle_exit_error ); - if( t_sig_ret == SIG_ERR ) + if( signum == SIGABRT || signum == SIGTERM ) { - LWARN( slog_constr, "Unable to setup handling of SIGTERM: SIGTERM will result in an unclean exit" ); + // An error-like signal was received + LERROR( sigwait_log, "Received signal <" << signum << "> as an error condition; return code: " << RETURN_ERROR ); + std::cerr << "Received signal <" << signum << "> as an error condition; return code: " << RETURN_ERROR << std::endl; + exit( RETURN_ERROR ); } - else + else if( signum == SIGINT || signum == SIGQUIT ) { - s_handling_sig_term = true; - s_old_sig_abrt_handler = t_sig_ret; + // A non-error-like signal was received + LWARN( sigwait_log, "Received signal <" << signum << ">; return code: " << RETURN_SUCCESS ); + std::cerr << "Received signal <" << signum << ">; return code: " << RETURN_SUCCESS << std::endl; + exit( RETURN_SUCCESS ); } - } - if( s_handling_sig_term ) LTRACE( slog_constr, "Handling SIGTERM" ); - - // setup to handle SIGINT - if( ! s_handling_sig_int ) - { - auto t_sig_ret = signal( SIGINT, signal_handler::handle_exit_error ); - if( t_sig_ret == SIG_ERR ) + else if( signum == SIGUSR1 ) { - LWARN( slog_constr, "Unable to setup handling of SIGINT: ctrl-c cancellation will result in an unclean exit" ); + // A request to stop handling signals was received + LWARN( sigwait_log, "Received request to stop handling signals" ); + std::cerr << "Received request to stop handling signals" << std::endl; } else { - s_handling_sig_int = true; - s_old_sig_abrt_handler = t_sig_ret; + // Unexpected signal + LERROR( sigwait_log, "An unexpected signal was recieved: " << signum ); + LERROR( sigwait_log, "Quitting with an error condition" ); + std::cerr << "An unexpected signal was recieved: " << signum << std::endl; + exit( RETURN_ERROR ); } - } - if( s_handling_sig_int ) LTRACE( slog_constr, "Handling SIGINT (ctrl-c))" ); -#endif - return; + return signum; } - void signal_handler::unhandle_signals() + void signal_handler::abort_handle_signals() { - std::unique_lock< std::recursive_mutex > t_lock( s_mutex ); - - LDEBUG( slog, "Returning signal handling for SIGABRT, SIGTERM, SIGINT, and SIGQUIT" ); + LDEBUG( slog, "Requesting stop of signal handling" ); + std::cerr << "Requesting stop of signal handling" << std::endl; + raise( SIGUSR1 ); -#ifndef _WIN32 + std::this_thread::sleep_for( std::chrono::milliseconds(500) ); // pause before unblocking so the raised signal can be handled - if( s_handling_sig_abrt && sigaction( SIGABRT, &s_old_sig_abrt_action, nullptr ) != 0 ) - { - LWARN( slog, "Unable to switch SIGABRT to previous handler" ); - } - else - { - s_handling_sig_abrt = false; - } - - if( s_handling_sig_term && sigaction( SIGTERM, &s_old_sig_term_action, nullptr ) != 0 ) + LDEBUG( slog, "Unblocking signals" ); + std::cerr << "Unblocking sigals" << std::endl; + if( pthread_sigmask( SIG_UNBLOCK, &s_sigset, nullptr ) != 0 ) { - LWARN( slog, "Unable to switch SIGTERM to previous handler" ); + LERROR( slog, "Unable to unblock the required signals" ); + std::cerr << "Unable to unblock the required signals" << std::endl; + throw error(__FILE_NAME__, __LINE__) << "Unable to unblock the required signals"; } - else - { - s_handling_sig_term = false; - } - - if( s_handling_sig_int && sigaction( SIGINT, &s_old_sig_int_action, nullptr ) != 0 ) - { - LWARN( slog, "Unable to switch SIGINT to previous handler" ); - } - else - { - s_handling_sig_int = false; - } - - if( s_handling_sig_quit && sigaction( SIGQUIT, &s_old_sig_quit_action, nullptr ) != 0 ) - { - LWARN( slog, "Unable to switch SIGQUIT to previous handler" ); - } - else - { - s_handling_sig_quit = false; - } - -#else // _WIN32 - if( s_handling_sig_abrt && signal( SIGABRT, s_old_sig_abrt_handler ) == SIG_ERR ) - { - LWARN( slog, "Unable to switch SIGABRT to previous handler" ); - } - else - { - s_handling_sig_abrt = false; - } - - if( s_handling_sig_term && signal( SIGTERM, s_old_sig_term_handler ) == SIG_ERR ) - { - LWARN( slog, "Unable to switch SIGTERM to previous handler" ); - } - else - { - s_handling_sig_term = false; - } - - if( s_handling_sig_int && signal( SIGINT, s_old_sig_int_handler ) == SIG_ERR ) - { - LWARN( slog, "Unable to switch SIGINT to previous handler" ); - } - else - { - s_handling_sig_int = false; - } - -#endif return; } - bool signal_handler::is_handling( int a_signal ) + bool signal_handler::is_handling() { - std::unique_lock< std::recursive_mutex > t_lock( s_mutex ); - - bool t_is_handled = false; - switch( a_signal ) - { - case SIGABRT: - if( s_handling_sig_abrt) t_is_handled = true; - break; - case SIGTERM: - if( s_handling_sig_term) t_is_handled = true; - break; - case SIGINT: - if( s_handling_sig_int) t_is_handled = true; - break; -#ifndef _WIN32 - case SIGQUIT: - if( s_handling_sig_quit) t_is_handled = true; - break; -#endif - default: - break; - } - return t_is_handled; + std::unique_lock t_lock( s_handle_mutex, std::defer_lock_t() ); + return ! t_lock.try_lock(); } void signal_handler::reset() { LDEBUG( slog, "Resetting signal_handler" ); - std::unique_lock< std::recursive_mutex > t_lock( s_mutex ); + + signal_handler::abort_handle_signals(); s_exited = false; s_return_code = RETURN_SUCCESS; s_cancelers.clear(); - signal_handler::unhandle_signals(); return; } @@ -355,23 +266,25 @@ namespace scarab { terminate( RETURN_ERROR ); } - +/* void signal_handler::handle_exit_error( int a_sig ) { - std::unique_lock< std::recursive_mutex > t_lock( s_mutex ); - LERROR( slog, "Handling signal <" << a_sig << "> as an error condition; return code: " << RETURN_ERROR ); - exit( RETURN_ERROR ); + signal_handler::s_exit_signal = a_sig; + //std::unique_lock< std::recursive_mutex > t_lock( s_mutex ); + //LERROR( slog, "Handling signal <" << a_sig << "> as an error condition; return code: " << RETURN_ERROR ); + //exit( RETURN_ERROR ); return; } void signal_handler::handle_exit_success( int a_sig ) { - std::unique_lock< std::recursive_mutex > t_lock( s_mutex ); - LPROG( slog, "Handling signal <" << a_sig << ">; return code: " << RETURN_SUCCESS ); - exit( RETURN_SUCCESS ); + signal_handler::s_exit_signal = a_sig; + //std::unique_lock< std::recursive_mutex > t_lock( s_mutex ); + //LPROG( slog, "Handling signal <" << a_sig << ">; return code: " << RETURN_SUCCESS ); + //exit( RETURN_SUCCESS ); return; } - +*/ [[noreturn]] void signal_handler::terminate( int a_code ) noexcept { std::unique_lock< std::recursive_mutex > t_lock( s_mutex ); @@ -443,7 +356,6 @@ namespace scarab void signal_handler::print_stack_trace( bool a_use_logging ) { // no mutex locking needed here -#ifndef _WIN32 // stack trace printing not implemented for windows void* t_bt_array[50]; int t_size = backtrace( t_bt_array, 50 ); @@ -461,7 +373,6 @@ namespace scarab else { std::cerr << "Backtrace:\n" << t_bt_str.str() << std::endl; } free( t_messages ); -#endif return; } @@ -481,10 +392,6 @@ namespace scarab std::this_thread::sleep_for( std::chrono::seconds(1) ); } -#ifdef _WIN32 - ExitProcess( a_code ); -#endif - return; } diff --git a/library/utility/signal_handler.hh b/library/utility/signal_handler.hh index dbbd5713..e6ffd830 100644 --- a/library/utility/signal_handler.hh +++ b/library/utility/signal_handler.hh @@ -14,10 +14,7 @@ #include #include #include - -#ifndef _WIN32 -#include // for struct sigaction, which is in signal.h but not csignal -#endif +#include #ifndef _GNU_SOURCE #define _GNU_SOURCE @@ -134,21 +131,24 @@ namespace scarab /// Remove all cancelables and signal handling static void reset(); - /// Start handling signals - static void handle_signals(); - /// Stop handling signals - static void unhandle_signals(); + /// Blocking call to handle signals; waits for a signal + /// Returns immediately if it's already been called; otherwise blocks then returns the signal or error value + static int do_handle_signals(); + + static int wait_on_signals(); + /// Requests the aborting of waiting for a signal; causes do_handle_signal() to return + static void abort_handle_signals(); /// Check if a signal is handled - static bool is_handling( int a_signal ); + static bool is_handling(); /// Handler for std::terminate -- does not cleanup memory or threads [[noreturn]] static void handle_terminate() noexcept; /// Handler for error signals -- cleanly exits - static void handle_exit_error( int a_sig ); + //static void handle_exit_error( int a_sig ); /// Handler for success signals -- cleanly exits - static void handle_exit_success( int a_sig ); + //static void handle_exit_success( int a_sig ); /// Main terminate function -- does not cleanup memory or threads [[noreturn]] static void terminate( int a_code ) noexcept; @@ -184,23 +184,11 @@ namespace scarab mv_accessible_static_noset( bool, exited ); mv_accessible_static( int, return_code ); - mv_accessible_static_noset( bool, handling_sig_abrt ); - mv_accessible_static_noset( bool, handling_sig_term ); - mv_accessible_static_noset( bool, handling_sig_int ); - mv_accessible_static_noset( bool, handling_sig_quit ); - -#ifndef _WIN32 - mv_referrable_static( struct sigaction, old_sig_abrt_action ); - mv_referrable_static( struct sigaction, old_sig_term_action ); - mv_referrable_static( struct sigaction, old_sig_int_action ); - mv_referrable_static( struct sigaction, old_sig_quit_action ); -#else // _WIN32 - typedef void (*handler_t)(int); - mv_referrable_static( handler_t, old_sig_abrt_handler ); - mv_referrable_static( handler_t, old_sig_term_handler ); - mv_referrable_static( handler_t, old_sig_int_handler ); - mv_referrable_static( handler_t, old_sig_quit_handler ); -#endif + mv_accessible_static( sigset_t, sigset ); + + protected: + static std::mutex s_handle_mutex; + }; } /* namespace scarab */ diff --git a/testing/applications/test_raise_sigint.cc b/testing/applications/test_raise_sigint.cc index 28bd0e64..36ec7f6e 100644 --- a/testing/applications/test_raise_sigint.cc +++ b/testing/applications/test_raise_sigint.cc @@ -23,14 +23,43 @@ #include "signal_handler.hh" #include "logger.hh" +#include #include +#include +LOGGER( testlog, "test_raise_sigint" ); int main(int , char ** ) { - scarab::signal_handler t_handler; + scarab::signal_handler t_sh; + std::cerr << "Starting to wait-on-signals thread" << std::endl; + std::thread t_sh_thread( &scarab::signal_handler::wait_on_signals ); + + std::this_thread::sleep_for( std::chrono::seconds(1) ); + + std::cerr << "Raising SIGINT" << std::endl; raise( SIGINT ); + std::cerr << "Raised" << std::endl; + + t_sh_thread.join(); + + /* + auto raise_signal = []() { + std::this_thread::sleep_for( std::chrono::seconds(1) ); + LWARN( testlog, "Raising SIGINT" ); + std::cerr << "Raising SIGINT" << std::endl; + raise( SIGINT ); + return; + }; + auto ft = std::async( std::launch::async, raise_signal ); + + // blocks + std::cerr << "Starting to handle signals" << std::endl; + scarab::signal_handler::do_handle_signals(); + std::cerr << "Exited from handling signals" << std::endl; + ft.get(); + */ return( EXIT_SUCCESS ); } diff --git a/testing/test_signal_handler.cc b/testing/test_signal_handler.cc index 615d5cf3..5ffd28a8 100644 --- a/testing/test_signal_handler.cc +++ b/testing/test_signal_handler.cc @@ -18,7 +18,7 @@ #include "catch2/catch_test_macros.hpp" -#include +#include using scarab::testing::compare_and_clear; @@ -29,68 +29,27 @@ TEST_CASE( "signal_handler", "[utility]" ) SECTION( "not_handling" ) { - REQUIRE_FALSE( scarab::signal_handler::get_handling_sig_abrt() ); - REQUIRE_FALSE( scarab::signal_handler::get_handling_sig_term() ); - REQUIRE_FALSE( scarab::signal_handler::get_handling_sig_int() ); - REQUIRE_FALSE( scarab::signal_handler::is_handling( SIGABRT ) ); - REQUIRE_FALSE( scarab::signal_handler::is_handling( SIGTERM ) ); - REQUIRE_FALSE( scarab::signal_handler::is_handling( SIGINT ) ); - #ifndef _WIN32 - REQUIRE_FALSE( scarab::signal_handler::get_handling_sig_quit() ); - REQUIRE_FALSE( scarab::signal_handler::is_handling( SIGQUIT ) ); - #endif - REQUIRE( scarab::signal_handler::get_ref_count() == 0 ); - } + REQUIRE_FALSE( scarab::signal_handler::is_handling() ); - scarab::signal_handler t_handler; - REQUIRE( scarab::signal_handler::get_ref_count() == 1 ); + // Prior to v3.13.5 signal_handler started handling on construction, so this ensures that's not the case + scarab::signal_handler t_handler; + REQUIRE( scarab::signal_handler::get_ref_count() == 1 ); + REQUIRE_FALSE( scarab::signal_handler::is_handling() ); + } SECTION( "handling" ) { - REQUIRE( scarab::signal_handler::get_handling_sig_abrt() ); - REQUIRE( scarab::signal_handler::get_handling_sig_term() ); - REQUIRE( scarab::signal_handler::get_handling_sig_int() ); - REQUIRE( scarab::signal_handler::is_handling( SIGABRT ) ); - REQUIRE( scarab::signal_handler::is_handling( SIGTERM ) ); - REQUIRE( scarab::signal_handler::is_handling( SIGINT ) ); - #ifndef _WIN32 - REQUIRE( scarab::signal_handler::get_handling_sig_quit() ); - REQUIRE( scarab::signal_handler::is_handling( SIGQUIT ) ); - #endif - - scarab::signal_handler t_handler_2; - REQUIRE( scarab::signal_handler::get_ref_count() == 2 ); - - REQUIRE( scarab::signal_handler::get_handling_sig_abrt() ); - REQUIRE( scarab::signal_handler::get_handling_sig_term() ); - REQUIRE( scarab::signal_handler::get_handling_sig_int() ); - #ifndef _WIN32 - REQUIRE( scarab::signal_handler::get_handling_sig_quit() ); - #endif - + // start handling threads + std::thread t_handler_thread( [](){ scarab::signal_handler::do_handle_signals(); } ); + std::this_thread::sleep_for( std::chrono::seconds(1) ); + REQUIRE( scarab::signal_handler::is_handling() ); + + scarab::signal_handler::abort_handle_signals(); + t_handler_thread.join(); + REQUIRE_FALSE( scarab::signal_handler::is_handling() ); } - REQUIRE( scarab::signal_handler::get_ref_count() == 1 ); - - SECTION( "handling_switch" ) - { - scarab::signal_handler::unhandle_signals(); - REQUIRE_FALSE( scarab::signal_handler::get_handling_sig_abrt() ); - REQUIRE_FALSE( scarab::signal_handler::get_handling_sig_term() ); - REQUIRE_FALSE( scarab::signal_handler::get_handling_sig_int() ); - #ifndef _WIN32 - REQUIRE_FALSE( scarab::signal_handler::get_handling_sig_quit() ); - #endif - - scarab::signal_handler::handle_signals(); - REQUIRE( scarab::signal_handler::get_handling_sig_abrt() ); - REQUIRE( scarab::signal_handler::get_handling_sig_term() ); - REQUIRE( scarab::signal_handler::get_handling_sig_int() ); - #ifndef _WIN32 - REQUIRE( scarab::signal_handler::get_handling_sig_quit() ); - #endif - } /* SECTION( "printing" ) { @@ -116,6 +75,8 @@ TEST_CASE( "signal_handler", "[utility]" ) } } */ + scarab::signal_handler t_handler; + scarab::cancelable t_cancel; REQUIRE_FALSE( t_cancel.is_canceled() ); From 9f6ce0f6708b02fb76614179dfea1547649af95c Mon Sep 17 00:00:00 2001 From: Noah Oblath Date: Wed, 10 Dec 2025 18:07:39 -0800 Subject: [PATCH 02/29] Swap the sigwait version of signal handling for a custom signal-watching function; refined the signal-handling functions. --- library/utility/signal_handler.cc | 250 +++++++++------------ library/utility/signal_handler.hh | 84 +++++-- testing/applications/test_raise_sigabrt.cc | 15 +- testing/applications/test_raise_sigint.cc | 28 +-- testing/applications/test_raise_sigquit.cc | 13 +- testing/applications/test_raise_sigterm.cc | 13 +- testing/test_signal_handler.cc | 50 ++++- 7 files changed, 263 insertions(+), 190 deletions(-) diff --git a/library/utility/signal_handler.cc b/library/utility/signal_handler.cc index de4f9c22..476618b4 100644 --- a/library/utility/signal_handler.cc +++ b/library/utility/signal_handler.cc @@ -37,40 +37,34 @@ namespace scarab std::recursive_mutex signal_handler::s_mutex; signal_handler::cancelers signal_handler::s_cancelers; - sigset_t signal_handler::s_sigset; + bool signal_handler::s_is_handling = false; + std::mutex signal_handler::s_handle_mutex; + signal_handler::signal_map_t signal_handler::s_handled_signals = { + {SIGABRT, signal_handler::handled_signal_info{"SIGABRT", false, {}, true, RETURN_ERROR}}, + {SIGTERM, signal_handler::handled_signal_info{"SIGTERM", false, {}, true, RETURN_ERROR}}, + {SIGINT, signal_handler::handled_signal_info{"SIGINT", false, {}, false, RETURN_SUCCESS}}, + {SIGQUIT, signal_handler::handled_signal_info{"SIGQUIT", false, {}, false, RETURN_SUCCESS}} + }; + + volatile sig_atomic_t signal_handler::s_signal_received = 0; + signal_handler::signal_handler() { - //std::cerr << "signal_handler constructor" << std::endl; ++signal_handler::s_ref_count; if( signal_handler::s_ref_count == 1 ) { - LWARN( slog, "Blocking signals SIGABRT, SIGTERM, SIGINT, SIGQUIT, and SIGUSR1" ); - - sigemptyset( &s_sigset ); - sigaddset( &s_sigset, SIGABRT ); - sigaddset( &s_sigset, SIGTERM ); - sigaddset( &s_sigset, SIGINT ); - sigaddset( &s_sigset, SIGQUIT ); - sigaddset( &s_sigset, SIGUSR1); - - if( pthread_sigmask( SIG_BLOCK, &s_sigset, nullptr ) != 0 ) - { - LERROR( slog, "Unable to block the required signals" ); - std::cerr << "Unable to block the required signals" << std::endl; - throw error(__FILE_NAME__, __LINE__) << "Unable to block the required signals"; - } + signal_handler::start_handling_signals(); } } signal_handler::~signal_handler() { - //std::cerr << "signal_handler destructor" << std::endl; --signal_handler::s_ref_count; if( signal_handler::s_ref_count == 0 ) { - abort_handle_signals(); + signal_handler::unhandle_signals(); } } @@ -106,144 +100,126 @@ namespace scarab return; } - int signal_handler::do_handle_signals() + void signal_handler::start_handling_signals() { - std::unique_lock t_lock( s_handle_mutex, std::defer_lock_t() ); - if( ! t_lock.try_lock() ) return -1; - - LWARN( slog, "Starting to handle signals SIGABRT, SIGTERM, SIGINT, SIGQUIT, and SIGUSR1" ); + if( signal_handler::get_is_handling() || signal_handler::is_waiting() ) return; - auto sigwait_fcn = []() { - LOGGER( sigwait_log, "signal_handler handle_signals" ); + std::unique_lock t_lock( s_handle_mutex ); - std::cerr << "In sigwait lambda" << std::endl; + LOGGER( slog_constr, "signal_handler::start_handling_signals" ); + LDEBUG( slog_constr, "Taking over signal handling for SIGABRT, SIGTERM, SIGINT, and SIGQUIT" ); - // wait until a signal is delivered, an abort was requested, or an error occurs - int signum = 0; - int error_num = sigwait( &s_sigset, &signum ); - std::cerr << "Done with sigwait" << std::endl; - - if( error_num != 0 ) - { - LERROR( sigwait_log, "An error occurred while waiting for signals: error code = " << error_num ); - std::cerr << "An error occurred while waiting for signals: error code = " << error_num << std::endl; - return -1; - } + struct sigaction t_action; + t_action.sa_handler = signal_handler::handle_signal; + sigemptyset(&t_action.sa_mask); + t_action.sa_flags = 0; - if( signum == SIGABRT || signum == SIGTERM ) - { - // An error-like signal was received - LERROR( sigwait_log, "Received signal <" << signum << "> as an error condition; return code: " << RETURN_ERROR ); - std::cerr << "Received signal <" << signum << "> as an error condition; return code: " << RETURN_ERROR << std::endl; - exit( RETURN_ERROR ); - } - else if( signum == SIGINT || signum == SIGQUIT ) - { - // A non-error-like signal was received - LWARN( sigwait_log, "Received signal <" << signum << ">; return code: " << RETURN_SUCCESS ); - std::cerr << "Received signal <" << signum << ">; return code: " << RETURN_SUCCESS << std::endl; - exit( RETURN_SUCCESS ); - } - else if( signum == SIGUSR1 ) + for( auto it = signal_handler::s_handled_signals.begin(); it != signal_handler::s_handled_signals.end(); ++it ) + { + if( ! it->second.handling && sigaction( it->first, &t_action, &it->second.old_action ) != 0 ) { - // A request to stop handling signals was received - LWARN( sigwait_log, "Received request to stop handling signals" ); - std::cerr << "Received request to stop handling signals" << std::endl; + LWARN( slog_constr, "Unable to setup handling of " << it->second.name ); } else { - // Unexpected signal - LERROR( sigwait_log, "An unexpected signal was recieved: " << signum ); - LERROR( sigwait_log, "Quitting with an error condition" ); - std::cerr << "An unexpected signal was recieved: " << signum << std::endl; - exit( RETURN_ERROR ); + LTRACE( slog_constr, "Handling " << it->second.name ); + it->second.handling = true; } + } - return signum; - }; - - auto ft_signal_handler = std::async( std::launch::async, sigwait_fcn ); + signal_handler::s_is_handling = true; - int sig_received = ft_signal_handler.get(); - LWARN( slog, "Signal-waiting function exited; received signal " << sig_received ); - std::cerr << "Signal-waiting function exited; received signal " << sig_received << std::endl; + return; + } - return sig_received; + bool signal_handler::is_handling() + { + return signal_handler::get_is_handling(); } - int signal_handler::wait_on_signals() + void signal_handler::unhandle_signals() { - LOGGER( sigwait_log, "signal_handler handle_signals" ); + if( ! signal_handler::get_is_handling() ) return; - LWARN( sigwait_log, "In wait-on-signals"); - std::cerr << "In wait-on-signals" << std::endl; + if( signal_handler::is_waiting() ) signal_handler::abort_wait(); - // wait until a signal is delivered, an abort was requested, or an error occurs - int signum = 0; - int error_num = sigwait( &s_sigset, &signum ); - LWARN( sigwait_log, "Done with sigwait" ); - std::cerr << "Done with sigwait" << std::endl; + std::unique_lock t_lock( s_handle_mutex ); - if( error_num != 0 ) - { - LERROR( sigwait_log, "An error occurred while waiting for signals: error code = " << error_num ); - std::cerr << "An error occurred while waiting for signals: error code = " << error_num << std::endl; - return -1; - } + LDEBUG( slog, "Returning signal handling for SIGABRT, SIGTERM, SIGINT, and SIGQUIT" ); - if( signum == SIGABRT || signum == SIGTERM ) - { - // An error-like signal was received - LERROR( sigwait_log, "Received signal <" << signum << "> as an error condition; return code: " << RETURN_ERROR ); - std::cerr << "Received signal <" << signum << "> as an error condition; return code: " << RETURN_ERROR << std::endl; - exit( RETURN_ERROR ); - } - else if( signum == SIGINT || signum == SIGQUIT ) - { - // A non-error-like signal was received - LWARN( sigwait_log, "Received signal <" << signum << ">; return code: " << RETURN_SUCCESS ); - std::cerr << "Received signal <" << signum << ">; return code: " << RETURN_SUCCESS << std::endl; - exit( RETURN_SUCCESS ); - } - else if( signum == SIGUSR1 ) + for( auto it = signal_handler::s_handled_signals.begin(); it != signal_handler::s_handled_signals.end(); ++it ) + { + if( it->second.handling && sigaction( it->first, &it->second.old_action, nullptr ) != 0 ) { - // A request to stop handling signals was received - LWARN( sigwait_log, "Received request to stop handling signals" ); - std::cerr << "Received request to stop handling signals" << std::endl; + LWARN( slog, "Unable to switch " << it->second.name << " to previous handler" ); } else { - // Unexpected signal - LERROR( sigwait_log, "An unexpected signal was recieved: " << signum ); - LERROR( sigwait_log, "Quitting with an error condition" ); - std::cerr << "An unexpected signal was recieved: " << signum << std::endl; - exit( RETURN_ERROR ); + LTRACE( slog, "Stopped handling " << it->second.name ); + it->second.handling = false; } + } + + signal_handler::s_is_handling = false; - return signum; + return; } - void signal_handler::abort_handle_signals() + int signal_handler::wait_for_signals( bool call_exit ) { - LDEBUG( slog, "Requesting stop of signal handling" ); - std::cerr << "Requesting stop of signal handling" << std::endl; - raise( SIGUSR1 ); + // Try to lock the mutex; if it didn't lock, then return + std::unique_lock t_lock( s_handle_mutex, std::defer_lock_t() ); + if( ! t_lock.try_lock() ) return -1; - std::this_thread::sleep_for( std::chrono::milliseconds(500) ); // pause before unblocking so the raised signal can be handled + while( signal_handler::s_signal_received == 0 ) + { + std::this_thread::sleep_for( std::chrono::milliseconds(500)) ; + } - LDEBUG( slog, "Unblocking signals" ); - std::cerr << "Unblocking sigals" << std::endl; - if( pthread_sigmask( SIG_UNBLOCK, &s_sigset, nullptr ) != 0 ) + // Handle situation where a signal was recieved + if( s_signal_received > 0 ) { - LERROR( slog, "Unable to unblock the required signals" ); - std::cerr << "Unable to unblock the required signals" << std::endl; - throw error(__FILE_NAME__, __LINE__) << "Unable to unblock the required signals"; + // this is a valid signal + LDEBUG( slog, "Exiting loop for signals after getting signal " << s_signal_received ); + if( signal_handler::s_handled_signals.count( int(signal_handler::s_signal_received) ) > 0 ) + { + handled_signal_info& sig_info = signal_handler::s_handled_signals.at( int(signal_handler::s_signal_received) ); + if( sig_info.indicates_error ) + { + LERROR( slog, "Received signal " << sig_info.name << "<" << s_signal_received << "> as an error condition; return code: " << sig_info.return_code ); + } + else + { + LDEBUG( slog, "Received signal " << sig_info.name << "<" << s_signal_received << "> as an exit condition; return code: " << sig_info.return_code ); + } + + // Call exit to cancel cancelables if requested + if( call_exit ) exit( sig_info.return_code ); + } + else + { + LERROR( slog, "Received unknown signal <" << s_signal_received << ">; using with return code " << RETURN_ERROR ); + if( call_exit ) exit( RETURN_ERROR ); + } + + return s_signal_received; } + // Handle the situation where no signal was received; wait loop was aborted + // s_signal_received < 0 --> requested to exit wait for signals + LDEBUG( slog, "Exited loop waiting for signals (wait aborted)" ); + return s_signal_received; + + } + + void signal_handler::abort_wait() + { + LDEBUG( slog, "Requesting stop of signal wait" ); + signal_handler::s_signal_received = -1; return; } - bool signal_handler::is_handling() + bool signal_handler::is_waiting() { std::unique_lock t_lock( s_handle_mutex, std::defer_lock_t() ); return ! t_lock.try_lock(); @@ -253,7 +229,7 @@ namespace scarab { LDEBUG( slog, "Resetting signal_handler" ); - signal_handler::abort_handle_signals(); + signal_handler::unhandle_signals(); s_exited = false; s_return_code = RETURN_SUCCESS; @@ -266,25 +242,17 @@ namespace scarab { terminate( RETURN_ERROR ); } -/* - void signal_handler::handle_exit_error( int a_sig ) - { - signal_handler::s_exit_signal = a_sig; - //std::unique_lock< std::recursive_mutex > t_lock( s_mutex ); - //LERROR( slog, "Handling signal <" << a_sig << "> as an error condition; return code: " << RETURN_ERROR ); - //exit( RETURN_ERROR ); - return; - } - void signal_handler::handle_exit_success( int a_sig ) + void signal_handler::handle_signal( int a_sig ) { - signal_handler::s_exit_signal = a_sig; - //std::unique_lock< std::recursive_mutex > t_lock( s_mutex ); - //LPROG( slog, "Handling signal <" << a_sig << ">; return code: " << RETURN_SUCCESS ); - //exit( RETURN_SUCCESS ); + signal_handler::s_signal_received = a_sig; + + const char str[] = "Received a signal\n"; + write( STDERR_FILENO, str, sizeof(str)-1 ); + return; } -*/ + [[noreturn]] void signal_handler::terminate( int a_code ) noexcept { std::unique_lock< std::recursive_mutex > t_lock( s_mutex ); @@ -293,7 +261,6 @@ namespace scarab { print_stack_trace( false ); } - std::cerr << "Exiting abruptly" << std::endl; std::_Exit( a_code ); } @@ -303,10 +270,11 @@ namespace scarab s_exited = true; s_return_code = a_code; print_current_exception( true ); - if( a_code > 0 ) - { - print_stack_trace( true ); - } + // Removed stack trace printing because it's just the trace at the point exit() is called, which is often the wait_for_signals(), which isn't very useful + //if( a_code > 0 ) + //{ + // print_stack_trace( true ); + //} cancel_all( a_code ); return; } diff --git a/library/utility/signal_handler.hh b/library/utility/signal_handler.hh index e6ffd830..e5974b79 100644 --- a/library/utility/signal_handler.hh +++ b/library/utility/signal_handler.hh @@ -70,18 +70,30 @@ namespace scarab To perform the tasks of exiting and terminating applications, `signal_handler` allows the user to control when signals are handled, and when they aren't. In most cases, a user will control signal handling through the existence of - a `signal_handler` instance. The user should create the instance of `signal_handler` when they want `signal_handler` - to start handling those signals, and it should be destructed when handling the signals should cease. - Here are several examples of how this could be done: + a `signal_handler` instance and watch for signals with signal_handler::watch_for_signals() run in a separate thread. + The user should create the instance of `signal_handler` when they want `signal_handler` + to start handling those signals, run the signal-watcher thread for as long as the application should react to + raised signals, and the handler should be destructed when handling the signals should cease. + + Here are several examples of how the lifetime of the signal_handler can be managed: 1. The instance is created as a global static object at static initialization time. Signals will be handled for the entire - time the application is running. + time the application is running. 2. The instance is created in the `main()` function of the application and exists until the application exits. 3. The instance is created in some function for a specific limited time to cover a particular action. This could be for the entire scope of the function, or it could be destructed when signal handling is no longer wanted. Note that signals are handled as long as one or more instances of `signal_handler` exist. If two instance exist, and one - goes out of scope, signals are still handled until the second instance is destructed. + goes out of scope, signals are still handled until the second instance is destructed. Signals are only reacted to + while the watcher function/thread is active. If there is a delay between when the watching function/thread exits and when + the signal_handler is destructed (or unhandle_signals() is called), during that period signals will not cause + the application to exit. + + For simple examples of creating a signal_handler and running a watcher thread, see the test_raise_sig*.cc files. + + By default the signals handled are SIGABRT (abort() and unhandled exceptions), SIGTERM (shell command kill), + SIGINT (ctrl-c), and SIGQUIT (ctrl-\). + The set of signals can be edited by modifying the handled signals map (via signal_handler::handled_signals()). In addition to this typical behavior, `signal_handler` includes an interface for more fine-grained control over when signals are handled while the `signal_handler` exists (i.e. handle_signals(), unhandle_signals(), and is_handling()), @@ -107,6 +119,24 @@ namespace scarab terminate(), exit(), or cancel_all(), respectively. cancel_all() will only affect the canceled objects. exit() perform a clean exit using cancel_all(), in addition to setting a return code, though it assumes the application will take care of actually exiting. And terminate() will immediately exit the application using std::_Exit(). + + # Design details for signal handling + + We need a way to turn a raised signal into a call to cancel_all() (via exit()). However, a POSIX signal handler + is only allowed to call a limited class of functions that are async-signal-safe (see https://pubs.opengroup.org/onlinepubs/9799919799/functions/V2_chap02.html#tag_16_04). + So we need an asynchronous method for getting the information about the raised signal to exit() instead. + We are allowed to set variables of type volatile sig_atomic_t (an integer), so we can use that as an indicator + that exit() needs to be called. We can call exit() in a function that runs in a separate thread and watches the value of the + volatile sig_atomic_t indicator. So our functionality has two parts: + + 1. "handling" signals: we replace the default handler functions for the four signals of interest + (SIGABRT, SIGTERM, SIGINT, and SIGQUIT). It prints a simple message to the error stream (write() is allowed) and + sets the value of s_signal_received, a volatile sig_atomic_t static variable. + 2. "watching" for signals: The function watch_for_signals() starts a loop that waits for s_signal_received to be nonzero. + It then breaks out of the loop and calls exit() with the appropriate return value (depending on the specific signal raised). + The watch loop can be broken out of manually by calling abort_wait(), which sets s_signal_received to a negative number. + All signals are positive integers in the POSIX standard, so the negative number differentiates an abort (exit() is not called) + from a handled signal (exit() is called). */ class SCARAB_API signal_handler { @@ -131,24 +161,30 @@ namespace scarab /// Remove all cancelables and signal handling static void reset(); + /// Fine-grained signal-handling control; called from the constructor + /// Uses handle_signal() as the signal-handling function for the relevant signals. + static void start_handling_signals(); + /// Fine-grained signal-handling control; called from the destructor + /// Returns the previous signal-handling functions to the signals. + static void unhandle_signals(); + static bool is_handling(); // just calls get_is_waiting(); is here to have parallel syntax to waiting + /// Blocking call to handle signals; waits for a signal /// Returns immediately if it's already been called; otherwise blocks then returns the signal or error value - static int do_handle_signals(); + /// Has an option to not call the exit() function; defaults to true (i.e. exit() will be called) + static int wait_for_signals( bool call_exit = true ); - static int wait_on_signals(); + //static int wait_on_signals(); /// Requests the aborting of waiting for a signal; causes do_handle_signal() to return - static void abort_handle_signals(); + static void abort_wait(); /// Check if a signal is handled - static bool is_handling(); + static bool is_waiting(); /// Handler for std::terminate -- does not cleanup memory or threads [[noreturn]] static void handle_terminate() noexcept; - /// Handler for error signals -- cleanly exits - //static void handle_exit_error( int a_sig ); - - /// Handler for success signals -- cleanly exits - //static void handle_exit_success( int a_sig ); + /// Handler for the relevant signals (SIGABRT, SIGTERM, SIGINT, and SIGQUIT by default) + static void handle_signal( int a_sig ); /// Main terminate function -- does not cleanup memory or threads [[noreturn]] static void terminate( int a_code ) noexcept; @@ -179,16 +215,34 @@ namespace scarab static std::recursive_mutex s_mutex; public: + struct handled_signal_info + { + std::string name; + bool handling; + struct sigaction old_action; + bool indicates_error; + int return_code; + }; + typedef std::map signal_map_t; + + static sig_atomic_t get_signal_received() { return s_signal_received; } + static signal_map_t handled_signals() { return s_handled_signals; } + mv_accessible_static_noset( int, ref_count ); mv_accessible_static_noset( bool, exited ); mv_accessible_static( int, return_code ); - mv_accessible_static( sigset_t, sigset ); + mv_accessible_static_noset( bool, is_handling ); + protected: static std::mutex s_handle_mutex; + static volatile sig_atomic_t s_signal_received; + + static signal_map_t s_handled_signals; + }; } /* namespace scarab */ diff --git a/testing/applications/test_raise_sigabrt.cc b/testing/applications/test_raise_sigabrt.cc index 07c49cce..86beafd7 100644 --- a/testing/applications/test_raise_sigabrt.cc +++ b/testing/applications/test_raise_sigabrt.cc @@ -37,13 +37,24 @@ #include "logger.hh" #include +#include +LOGGER_ST( testlog, "test_raise_sigabrt" ); int main(int , char ** ) { - scarab::signal_handler t_handler; - + scarab::signal_handler t_sh; + + LINFO( testlog, "Starting to wait-on-signals thread" ); + std::thread t_sh_thread( [](){scarab::signal_handler::wait_for_signals();} ); + + std::this_thread::sleep_for( std::chrono::seconds(1) ); + + LINFO( testlog, "Raising SIGABRT" ); raise( SIGABRT ); + LINFO( testlog, "Raised" ); + t_sh_thread.join(); + return( EXIT_SUCCESS ); } diff --git a/testing/applications/test_raise_sigint.cc b/testing/applications/test_raise_sigint.cc index 36ec7f6e..94f67be1 100644 --- a/testing/applications/test_raise_sigint.cc +++ b/testing/applications/test_raise_sigint.cc @@ -23,43 +23,25 @@ #include "signal_handler.hh" #include "logger.hh" -#include #include #include -LOGGER( testlog, "test_raise_sigint" ); +LOGGER_ST( testlog, "test_raise_sigint" ); int main(int , char ** ) { scarab::signal_handler t_sh; - std::cerr << "Starting to wait-on-signals thread" << std::endl; - std::thread t_sh_thread( &scarab::signal_handler::wait_on_signals ); + LINFO( testlog, "Starting to wait-on-signals thread" ); + std::thread t_sh_thread( [](){scarab::signal_handler::wait_for_signals();} ); std::this_thread::sleep_for( std::chrono::seconds(1) ); - std::cerr << "Raising SIGINT" << std::endl; + LINFO( testlog, "Raising SIGINT" ); raise( SIGINT ); - std::cerr << "Raised" << std::endl; + LINFO( testlog, "Raised" ); t_sh_thread.join(); - /* - auto raise_signal = []() { - std::this_thread::sleep_for( std::chrono::seconds(1) ); - LWARN( testlog, "Raising SIGINT" ); - std::cerr << "Raising SIGINT" << std::endl; - raise( SIGINT ); - return; - }; - auto ft = std::async( std::launch::async, raise_signal ); - - // blocks - std::cerr << "Starting to handle signals" << std::endl; - scarab::signal_handler::do_handle_signals(); - std::cerr << "Exited from handling signals" << std::endl; - ft.get(); - */ - return( EXIT_SUCCESS ); } diff --git a/testing/applications/test_raise_sigquit.cc b/testing/applications/test_raise_sigquit.cc index 5864013a..efa216ee 100644 --- a/testing/applications/test_raise_sigquit.cc +++ b/testing/applications/test_raise_sigquit.cc @@ -24,13 +24,24 @@ #include "logger.hh" #include +#include +LOGGER_ST( testlog, "test_raise_sigquit" ); int main(int , char ** ) { - scarab::signal_handler t_handler; + scarab::signal_handler t_sh; + LINFO( testlog, "Starting to wait-on-signals thread" ); + std::thread t_sh_thread( [](){scarab::signal_handler::wait_for_signals();} ); + + std::this_thread::sleep_for( std::chrono::seconds(1) ); + + LINFO( testlog, "Raising SIGQUIT" ); raise( SIGQUIT ); + LINFO( testlog, "Raised" ); + t_sh_thread.join(); + return( EXIT_SUCCESS ); } diff --git a/testing/applications/test_raise_sigterm.cc b/testing/applications/test_raise_sigterm.cc index eca3b5d8..938b1c62 100644 --- a/testing/applications/test_raise_sigterm.cc +++ b/testing/applications/test_raise_sigterm.cc @@ -37,13 +37,24 @@ #include "logger.hh" #include +#include +LOGGER_ST( testlog, "test_raise_sigterm" ); int main(int , char ** ) { - scarab::signal_handler t_handler; + scarab::signal_handler t_sh; + LINFO( testlog, "Starting to wait-on-signals thread" ); + std::thread t_sh_thread( [](){scarab::signal_handler::wait_for_signals();} ); + + std::this_thread::sleep_for( std::chrono::seconds(1) ); + + LINFO( testlog, "Raising SIGTERM" ); raise( SIGTERM ); + LINFO( testlog, "Raised" ); + t_sh_thread.join(); + return( EXIT_SUCCESS ); } diff --git a/testing/test_signal_handler.cc b/testing/test_signal_handler.cc index 5ffd28a8..79e33bcc 100644 --- a/testing/test_signal_handler.cc +++ b/testing/test_signal_handler.cc @@ -27,27 +27,63 @@ TEST_CASE( "signal_handler", "[utility]" ) { LOGGER( tlog, "test_signal_handler" ); - SECTION( "not_handling" ) + // Thread safety note + // From: https://github.com/catchorg/Catch2/blob/devel/docs/thread-safety.md + // Assertion macros are thread safe. + // Section macros, generator macros, and test-case macros are not thread safe. + // So starting a test case or section and then spawning a thread inside is ok; + // Having multiple threads running in different sections or test cases in parallel is not ok. + // Calling REQUIRE in a spawned thread is dangerous because if it is false, the exception thrown will not be caught properly, bringing down the whole process. + // Instead, call CHECK in a thread, which will record the proper failures/successes, but not crash the program. + + SECTION( "handling" ) { REQUIRE( scarab::signal_handler::get_ref_count() == 0 ); REQUIRE_FALSE( scarab::signal_handler::is_handling() ); + REQUIRE_FALSE( scarab::signal_handler::is_waiting() ); + REQUIRE( scarab::signal_handler::get_signal_received() == 0 ); - // Prior to v3.13.5 signal_handler started handling on construction, so this ensures that's not the case scarab::signal_handler t_handler; REQUIRE( scarab::signal_handler::get_ref_count() == 1 ); + REQUIRE( scarab::signal_handler::is_handling() ); + REQUIRE_FALSE( scarab::signal_handler::is_waiting() ); + + t_handler.unhandle_signals(); REQUIRE_FALSE( scarab::signal_handler::is_handling() ); } - SECTION( "handling" ) + SECTION( "handle while in scope" ) + { + REQUIRE( scarab::signal_handler::get_ref_count() == 0 ); + REQUIRE_FALSE( scarab::signal_handler::is_handling() ); + + { + scarab::signal_handler t_handler; + REQUIRE( scarab::signal_handler::get_ref_count() == 1 ); + REQUIRE( scarab::signal_handler::is_handling() ); + } // t_handler destructed, which should unhandle signals + + REQUIRE( scarab::signal_handler::get_ref_count() == 0 ); + REQUIRE_FALSE( scarab::signal_handler::is_handling() ); + + } + + SECTION( "wait then abort" ) { + scarab::signal_handler t_handler; + REQUIRE( scarab::signal_handler::is_handling() ); + REQUIRE_FALSE( scarab::signal_handler::is_waiting() ); + // start handling threads - std::thread t_handler_thread( [](){ scarab::signal_handler::do_handle_signals(); } ); + std::thread t_handler_thread( [](){ scarab::signal_handler::wait_for_signals(); } ); std::this_thread::sleep_for( std::chrono::seconds(1) ); - REQUIRE( scarab::signal_handler::is_handling() ); + REQUIRE( scarab::signal_handler::is_waiting() ); - scarab::signal_handler::abort_handle_signals(); + scarab::signal_handler::abort_wait(); t_handler_thread.join(); - REQUIRE_FALSE( scarab::signal_handler::is_handling() ); + REQUIRE( scarab::signal_handler::get_signal_received() == -1 ); + REQUIRE( scarab::signal_handler::is_handling() ); + REQUIRE_FALSE( scarab::signal_handler::is_waiting() ); } /* From b6b0119a7961ae98d29306571df5926df6409aad Mon Sep 17 00:00:00 2001 From: Noah Oblath Date: Wed, 10 Dec 2025 18:08:08 -0800 Subject: [PATCH 03/29] Removed the local versions of loggers to see if it avoids the logging-thread-stopped errors --- library/logger/logger.hh | 8 ++++---- library/utility/indexed_factory.hh | 8 ++++---- testing/test_logger.cc | 5 +++-- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/library/logger/logger.hh b/library/logger/logger.hh index d92f1714..3e7bbf86 100644 --- a/library/logger/logger.hh +++ b/library/logger/logger.hh @@ -223,13 +223,13 @@ namespace scarab /// Creates a local scarab::logger object with variable name a_logger /// Uses spdlog's asynchronous logger; spdlog logger name will be a_name. -#define LOCAL_LOGGER( a_logger, a_name ) \ - ::scarab::logger_type< ::scarab::spd_initializer_async_stdout_color_mt > a_logger( a_name, __FILE_NAME__, __LINE__ ); +//#define LOCAL_LOGGER( a_logger, a_name ) \ +// ::scarab::logger_type< ::scarab::spd_initializer_async_stdout_color_mt > a_logger( a_name, __FILE_NAME__, __LINE__ ); /// Creates a local single-threaded (non-asynchronous) scarab::logger object with variable name a_logger /// Uses spdlog's basic logger; spdlog logger name will be a_name. -#define LOCAL_LOGGER_ST( a_logger, a_name ) \ - ::scarab::logger_type< ::scarab::spd_initializer_stdout_color > a_logger( a_name, __FILE_NAME__, __LINE__ ); +//#define LOCAL_LOGGER_ST( a_logger, a_name ) \ +// ::scarab::logger_type< ::scarab::spd_initializer_stdout_color > a_logger( a_name, __FILE_NAME__, __LINE__ ); // Logging functions #ifdef NDEBUG diff --git a/library/utility/indexed_factory.hh b/library/utility/indexed_factory.hh index a936f705..5fd3a56e 100644 --- a/library/utility/indexed_factory.hh +++ b/library/utility/indexed_factory.hh @@ -212,7 +212,7 @@ namespace scarab void indexed_factory< XIndexType, XBaseType, XArgs... >::register_class( const XIndexType& a_index, const base_registrar< XBaseType, XArgs... >* a_registrar ) { // A local (non-static) logger is created inside this function to avoid static initialization order problems - LOCAL_LOGGER( slog_ind_factory_reg, "indexed_factory_register"); + LOGGER( slog_ind_factory_reg, "indexed_factory_register"); std::unique_lock< std::mutex > t_lock( this->f_factory_mutex ); FactoryCIt it = fMap->find(a_index); @@ -235,7 +235,7 @@ namespace scarab void indexed_factory< XIndexType, XBaseType, XArgs... >::remove_class(const XIndexType& a_index ) { // A local (non-static) logger is created inside this function to avoid static destruction problems - LOCAL_LOGGER( slog_ind_factory_rem, "indexed_factory_remove"); + LOGGER( slog_ind_factory_rem, "indexed_factory_remove"); LTRACE( slog_ind_factory_rem, "Removing indexed_factory for class " << a_index << " from " << this ); /* #ifndef NDEBUG @@ -353,7 +353,7 @@ namespace scarab void indexed_factory< XIndexType, XBaseType, void >::register_class( const XIndexType& a_index, const base_registrar< XBaseType >* a_registrar ) { // A local (non-static) logger is created inside this function to avoid static initialization order problems - LOCAL_LOGGER( slog_ind_factory_reg, "indexed_factory_register"); + LOGGER( slog_ind_factory_reg, "indexed_factory_register"); std::unique_lock< std::mutex > t_lock( this->f_factory_mutex ); FactoryCIt it = fMap->find(a_index); @@ -377,7 +377,7 @@ namespace scarab void indexed_factory< XIndexType, XBaseType, void >::remove_class(const XIndexType& a_index ) { // A local (non-static) logger is created inside this function to avoid static destruction problems - LOCAL_LOGGER( slog_ind_factory_rem, "indexed_factory_remove"); + LOGGER( slog_ind_factory_rem, "indexed_factory_remove"); LTRACE( slog_ind_factory_rem, "Removing indexed_factory for class " << a_index << " from " << this ); //#ifndef NDEBUG // if( ELevel::eTrace >= f_global_threshold ) diff --git a/testing/test_logger.cc b/testing/test_logger.cc index 6c2c8003..d0289706 100644 --- a/testing/test_logger.cc +++ b/testing/test_logger.cc @@ -85,7 +85,7 @@ TEST_CASE( "logger", "[logger]" ) //REQUIRE( compare_and_clear( t_errstream, "test FATAL" ) ); } - +/* TEST_CASE( "local_logger", "[logger]" ) { LOCAL_LOGGER( tlog, "test_logger" ); @@ -117,4 +117,5 @@ TEST_CASE( "async_logging_stop", "[logger]" ) t_async = std::dynamic_pointer_cast( tlog.spdlogger() ); REQUIRE( t_async ); -} \ No newline at end of file +} +*/ \ No newline at end of file From 57c0d012185c1bcd32c6cde6cb7818720c491a60 Mon Sep 17 00:00:00 2001 From: Noah Oblath Date: Wed, 10 Dec 2025 18:14:28 -0800 Subject: [PATCH 04/29] Removed deprecated actions runner --- .github/workflows/run_tests.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/run_tests.yaml b/.github/workflows/run_tests.yaml index 6f71799a..14af2ce5 100644 --- a/.github/workflows/run_tests.yaml +++ b/.github/workflows/run_tests.yaml @@ -17,7 +17,7 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-24.04, ubuntu-22.04, macos-15, macos-14, macos-13] + os: [ubuntu-24.04, ubuntu-22.04, macos-26, macos-15-intel, macos-15, macos-14] steps: From 0f708fff7a742f3447896f2d8af072363165164c Mon Sep 17 00:00:00 2001 From: Noah Oblath Date: Wed, 10 Dec 2025 18:14:37 -0800 Subject: [PATCH 05/29] Bumped version --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index d82b1182..bf7f15fe 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -3 13 4 +3 13 5 From 73179e1e8c4ca57a3c335b064972784929615d93 Mon Sep 17 00:00:00 2001 From: Noah Oblath Date: Wed, 10 Dec 2025 18:17:11 -0800 Subject: [PATCH 06/29] Missing include --- library/utility/signal_handler.hh | 1 + 1 file changed, 1 insertion(+) diff --git a/library/utility/signal_handler.hh b/library/utility/signal_handler.hh index e5974b79..03a07510 100644 --- a/library/utility/signal_handler.hh +++ b/library/utility/signal_handler.hh @@ -15,6 +15,7 @@ #include #include #include +#include #ifndef _GNU_SOURCE #define _GNU_SOURCE From 89f6e8c5c6ca19833740749c19260464adb06cac Mon Sep 17 00:00:00 2001 From: Noah Oblath Date: Wed, 10 Dec 2025 18:25:22 -0800 Subject: [PATCH 07/29] [no ci] Added missing space --- library/utility/signal_handler.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/utility/signal_handler.cc b/library/utility/signal_handler.cc index 476618b4..db31a7d4 100644 --- a/library/utility/signal_handler.cc +++ b/library/utility/signal_handler.cc @@ -186,11 +186,11 @@ namespace scarab handled_signal_info& sig_info = signal_handler::s_handled_signals.at( int(signal_handler::s_signal_received) ); if( sig_info.indicates_error ) { - LERROR( slog, "Received signal " << sig_info.name << "<" << s_signal_received << "> as an error condition; return code: " << sig_info.return_code ); + LERROR( slog, "Received signal " << sig_info.name << " <" << s_signal_received << "> as an error condition; return code: " << sig_info.return_code ); } else { - LDEBUG( slog, "Received signal " << sig_info.name << "<" << s_signal_received << "> as an exit condition; return code: " << sig_info.return_code ); + LDEBUG( slog, "Received signal " << sig_info.name << " <" << s_signal_received << "> as an exit condition; return code: " << sig_info.return_code ); } // Call exit to cancel cancelables if requested From 3f42a0123c95dfb34d0ee56996a92371d1b8b234 Mon Sep 17 00:00:00 2001 From: Noah Oblath Date: Thu, 11 Dec 2025 17:04:36 -0800 Subject: [PATCH 08/29] Preparing for logger debugging --- CMakeLists.txt | 2 +- library/logger/logger.cc | 5 +++++ library/logger/logger.hh | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d794b09f..372527e9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -85,7 +85,7 @@ set( SPDLOG_BUILD_SHARED TRUE ) set( SPDLOG_INSTALL TRUE ) FetchContent_Declare( spdlog GIT_REPOSITORY https://github.com/project8/spdlog.git - GIT_TAG v1.x_p8 + GIT_TAG v1.x_p8_debugging ) FetchContent_MakeAvailable( spdlog ) # if Scarab is being built as a submodule, the parent might need to know where to find spdlog diff --git a/library/logger/logger.cc b/library/logger/logger.cc index ff64a219..5175755d 100644 --- a/library/logger/logger.cc +++ b/library/logger/logger.cc @@ -24,6 +24,7 @@ namespace scarab std::shared_ptr< spdlog::logger > t_logger = spdlog::get( a_name ); if( ! t_logger ) { + std::cerr << "Creating logger <" << a_name << ">" << std::endl; // see the comment in logger.hh about why the a_make_logger_fcn callback is used instead of the virtual function call t_logger = a_make_logger_fcn( a_name ); //this->make_logger( a_name ); t_logger->set_level( spdlog::level::level_enum(unsigned(logger::ELevel::SCARAB_LOGGER_DEFAULT_THRESHOLD)) ); @@ -37,6 +38,7 @@ namespace scarab spd_initializer( a_pattern ), f_sink() { + std::cerr << "initializing thread pool and mt sink" << std::endl; spdlog::init_thread_pool(8192, 1); f_sink = std::make_shared< spdlog::sinks::stdout_color_sink_mt >(); f_sink->set_pattern( f_pattern ); @@ -64,6 +66,7 @@ namespace scarab spd_initializer( a_pattern ), f_sink() { + std::cerr << "initializing st sink" << std::endl; f_sink = std::make_shared< spdlog::sinks::stdout_color_sink_st >(); f_sink->set_pattern( f_pattern ); } @@ -80,11 +83,13 @@ namespace scarab f_strstr(), f_spdlogger() { + std::cerr << "creating logger <" << a_name << ">" << std::endl; scarab::logger::all_loggers().insert(this); } logger::~logger() { + std::cerr << "destructing logger <" << f_name << ">" << std::endl; logger::all_loggers().erase(this); } diff --git a/library/logger/logger.hh b/library/logger/logger.hh index 3e7bbf86..934498a0 100644 --- a/library/logger/logger.hh +++ b/library/logger/logger.hh @@ -188,7 +188,7 @@ namespace scarab f_initializer_ptr( nullptr ) { #ifdef SCARAB_LOGGER_DEBUG - std::cout << "Logger <" << a_name << "> was initialized from <" << a_file << ":" << a_line << ">; type: " << ::scarab::type( *this ) << std::endl; + std::cerr << "Logger <" << a_name << "> was initialized from <" << a_file << ":" << a_line << ">; type: " << ::scarab::type( *this ) << std::endl; #endif // Start the backend, but only once static initializer_x s_init; From a96bddb22e47c4c27c53ba500035beb1664910dc Mon Sep 17 00:00:00 2001 From: Noah Oblath Date: Fri, 12 Dec 2025 01:52:33 -0800 Subject: [PATCH 09/29] Adding debug printing --- library/logger/logger.cc | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/library/logger/logger.cc b/library/logger/logger.cc index 5175755d..75274bd6 100644 --- a/library/logger/logger.cc +++ b/library/logger/logger.cc @@ -24,12 +24,16 @@ namespace scarab std::shared_ptr< spdlog::logger > t_logger = spdlog::get( a_name ); if( ! t_logger ) { - std::cerr << "Creating logger <" << a_name << ">" << std::endl; + std::cerr << "Creating spdlog logger <" << a_name << ">" << std::endl; // see the comment in logger.hh about why the a_make_logger_fcn callback is used instead of the virtual function call t_logger = a_make_logger_fcn( a_name ); //this->make_logger( a_name ); t_logger->set_level( spdlog::level::level_enum(unsigned(logger::ELevel::SCARAB_LOGGER_DEFAULT_THRESHOLD)) ); spdlog::register_logger( t_logger ); } + else + { + std::cerr << "Returning existing logger <" << a_name << ">" << std::endl; + } return t_logger; } @@ -42,7 +46,7 @@ namespace scarab spdlog::init_thread_pool(8192, 1); f_sink = std::make_shared< spdlog::sinks::stdout_color_sink_mt >(); f_sink->set_pattern( f_pattern ); - auto at_exit_fcn = [](){ logger::stop_using_spd_async(); }; + auto at_exit_fcn = [](){ std::cerr << "at exit fcn" << std::endl; logger::stop_using_spd_async(); }; std::atexit( at_exit_fcn ); } @@ -83,7 +87,7 @@ namespace scarab f_strstr(), f_spdlogger() { - std::cerr << "creating logger <" << a_name << ">" << std::endl; + std::cerr << "scarab::logger constructor: creating logger <" << a_name << ">" << std::endl; scarab::logger::all_loggers().insert(this); } From bc1472a9d52ec4b230143ff5a4cf8b3cd3cea203 Mon Sep 17 00:00:00 2001 From: Noah Oblath Date: Fri, 12 Dec 2025 16:01:18 -0800 Subject: [PATCH 10/29] Switch to using a tag for the spdlog dependency --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 372527e9..a4be0d19 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -85,7 +85,7 @@ set( SPDLOG_BUILD_SHARED TRUE ) set( SPDLOG_INSTALL TRUE ) FetchContent_Declare( spdlog GIT_REPOSITORY https://github.com/project8/spdlog.git - GIT_TAG v1.x_p8_debugging + GIT_TAG v1.x_p8_r1 ) FetchContent_MakeAvailable( spdlog ) # if Scarab is being built as a submodule, the parent might need to know where to find spdlog From dd727ecb64bb8e93c60736b041e5a43525599eb3 Mon Sep 17 00:00:00 2001 From: Noah Oblath Date: Fri, 12 Dec 2025 16:07:47 -0800 Subject: [PATCH 11/29] Revert "Adding debug printing" This reverts commit a96bddb22e47c4c27c53ba500035beb1664910dc. --- library/logger/logger.cc | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/library/logger/logger.cc b/library/logger/logger.cc index 75274bd6..5175755d 100644 --- a/library/logger/logger.cc +++ b/library/logger/logger.cc @@ -24,16 +24,12 @@ namespace scarab std::shared_ptr< spdlog::logger > t_logger = spdlog::get( a_name ); if( ! t_logger ) { - std::cerr << "Creating spdlog logger <" << a_name << ">" << std::endl; + std::cerr << "Creating logger <" << a_name << ">" << std::endl; // see the comment in logger.hh about why the a_make_logger_fcn callback is used instead of the virtual function call t_logger = a_make_logger_fcn( a_name ); //this->make_logger( a_name ); t_logger->set_level( spdlog::level::level_enum(unsigned(logger::ELevel::SCARAB_LOGGER_DEFAULT_THRESHOLD)) ); spdlog::register_logger( t_logger ); } - else - { - std::cerr << "Returning existing logger <" << a_name << ">" << std::endl; - } return t_logger; } @@ -46,7 +42,7 @@ namespace scarab spdlog::init_thread_pool(8192, 1); f_sink = std::make_shared< spdlog::sinks::stdout_color_sink_mt >(); f_sink->set_pattern( f_pattern ); - auto at_exit_fcn = [](){ std::cerr << "at exit fcn" << std::endl; logger::stop_using_spd_async(); }; + auto at_exit_fcn = [](){ logger::stop_using_spd_async(); }; std::atexit( at_exit_fcn ); } @@ -87,7 +83,7 @@ namespace scarab f_strstr(), f_spdlogger() { - std::cerr << "scarab::logger constructor: creating logger <" << a_name << ">" << std::endl; + std::cerr << "creating logger <" << a_name << ">" << std::endl; scarab::logger::all_loggers().insert(this); } From 4ceecb3722f47f0ca81c7f296d7f0d8a27145989 Mon Sep 17 00:00:00 2001 From: Noah Oblath Date: Fri, 12 Dec 2025 16:09:08 -0800 Subject: [PATCH 12/29] Revert "Preparing for logger debugging" --- library/logger/logger.cc | 5 ----- library/logger/logger.hh | 2 +- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/library/logger/logger.cc b/library/logger/logger.cc index 5175755d..ff64a219 100644 --- a/library/logger/logger.cc +++ b/library/logger/logger.cc @@ -24,7 +24,6 @@ namespace scarab std::shared_ptr< spdlog::logger > t_logger = spdlog::get( a_name ); if( ! t_logger ) { - std::cerr << "Creating logger <" << a_name << ">" << std::endl; // see the comment in logger.hh about why the a_make_logger_fcn callback is used instead of the virtual function call t_logger = a_make_logger_fcn( a_name ); //this->make_logger( a_name ); t_logger->set_level( spdlog::level::level_enum(unsigned(logger::ELevel::SCARAB_LOGGER_DEFAULT_THRESHOLD)) ); @@ -38,7 +37,6 @@ namespace scarab spd_initializer( a_pattern ), f_sink() { - std::cerr << "initializing thread pool and mt sink" << std::endl; spdlog::init_thread_pool(8192, 1); f_sink = std::make_shared< spdlog::sinks::stdout_color_sink_mt >(); f_sink->set_pattern( f_pattern ); @@ -66,7 +64,6 @@ namespace scarab spd_initializer( a_pattern ), f_sink() { - std::cerr << "initializing st sink" << std::endl; f_sink = std::make_shared< spdlog::sinks::stdout_color_sink_st >(); f_sink->set_pattern( f_pattern ); } @@ -83,13 +80,11 @@ namespace scarab f_strstr(), f_spdlogger() { - std::cerr << "creating logger <" << a_name << ">" << std::endl; scarab::logger::all_loggers().insert(this); } logger::~logger() { - std::cerr << "destructing logger <" << f_name << ">" << std::endl; logger::all_loggers().erase(this); } diff --git a/library/logger/logger.hh b/library/logger/logger.hh index 934498a0..3e7bbf86 100644 --- a/library/logger/logger.hh +++ b/library/logger/logger.hh @@ -188,7 +188,7 @@ namespace scarab f_initializer_ptr( nullptr ) { #ifdef SCARAB_LOGGER_DEBUG - std::cerr << "Logger <" << a_name << "> was initialized from <" << a_file << ":" << a_line << ">; type: " << ::scarab::type( *this ) << std::endl; + std::cout << "Logger <" << a_name << "> was initialized from <" << a_file << ":" << a_line << ">; type: " << ::scarab::type( *this ) << std::endl; #endif // Start the backend, but only once static initializer_x s_init; From 79a32f7e837a730656828c91a4b69b99af3bc590 Mon Sep 17 00:00:00 2001 From: Noah Oblath Date: Mon, 15 Dec 2025 09:11:30 -0800 Subject: [PATCH 13/29] Removing macos-26 GHA runner as it's in beta and was failing mysteriously --- .github/workflows/run_tests.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/run_tests.yaml b/.github/workflows/run_tests.yaml index 14af2ce5..bf7dc14b 100644 --- a/.github/workflows/run_tests.yaml +++ b/.github/workflows/run_tests.yaml @@ -17,7 +17,7 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-24.04, ubuntu-22.04, macos-26, macos-15-intel, macos-15, macos-14] + os: [ubuntu-24.04, ubuntu-22.04, macos-15-intel, macos-15, macos-14] steps: From 08b7a9c5dfb8a8dc2428a71b19943d5287ed6814 Mon Sep 17 00:00:00 2001 From: Noah Oblath Date: Mon, 15 Dec 2025 09:16:09 -0800 Subject: [PATCH 14/29] [no ci] Updated changelog --- changelog.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/changelog.md b/changelog.md index bdbcbc85..2b23a7b5 100644 --- a/changelog.md +++ b/changelog.md @@ -9,6 +9,21 @@ Types of changes: Added, Changed, Deprecated, Removed, Fixed, Security ## [Unreleased] +## [3.13.5] - 2025-12-15 + +### Changed + +- Signal handling now requires a dedicated thread +- Updated GHA runners +- Removed local loggers + +### Fixed + +- Tied spdlog to a tag and not a branch +- Updated spdlog fork from upstream +- signal_handler signal handling functions use only approved function calls + + ## [3.13.4] - 2025-11-04 ### Fixed From 0a587045234c1b2a855e386d19cb61fec0b0b892 Mon Sep 17 00:00:00 2001 From: Noah Oblath Date: Mon, 15 Dec 2025 14:10:06 -0800 Subject: [PATCH 15/29] Adding thread management and an easier API for the waiting thread --- library/utility/signal_handler.cc | 16 +++++++ library/utility/signal_handler.hh | 51 ++++++++++++++++------ testing/applications/test_raise_sigabrt.cc | 4 +- testing/applications/test_raise_sigint.cc | 4 +- testing/applications/test_raise_sigquit.cc | 4 +- 5 files changed, 59 insertions(+), 20 deletions(-) diff --git a/library/utility/signal_handler.cc b/library/utility/signal_handler.cc index db31a7d4..c62c60a5 100644 --- a/library/utility/signal_handler.cc +++ b/library/utility/signal_handler.cc @@ -39,6 +39,8 @@ namespace scarab bool signal_handler::s_is_handling = false; + std::thread signal_handler::s_waiting_thread; + std::mutex signal_handler::s_handle_mutex; signal_handler::signal_map_t signal_handler::s_handled_signals = { @@ -225,12 +227,26 @@ namespace scarab return ! t_lock.try_lock(); } + void signal_handler::start_waiting_thread() + { + signal_handler::s_waiting_thread = std::thread( [](){scarab::signal_handler::wait_for_signals();} ); + return; + } + + void signal_handler::join_waiting_thread() + { + s_waiting_thread.join(); + s_waiting_thread = std::thread(); + return; + } + void signal_handler::reset() { LDEBUG( slog, "Resetting signal_handler" ); signal_handler::unhandle_signals(); + s_waiting_thread = std::thread(); s_exited = false; s_return_code = RETURN_SUCCESS; s_cancelers.clear(); diff --git a/library/utility/signal_handler.hh b/library/utility/signal_handler.hh index 03a07510..720ea9c2 100644 --- a/library/utility/signal_handler.hh +++ b/library/utility/signal_handler.hh @@ -16,6 +16,7 @@ #include #include #include +#include #ifndef _GNU_SOURCE #define _GNU_SOURCE @@ -67,15 +68,25 @@ namespace scarab Responsibility for canceling a cancelable is given to the signal_handler using add_cancelable(), and it's taken away using remove_cancelable(). + Note that the interface for `signal_handler` uses all static functions, so there is no need to create an extra instance of + the class to use any part of that interface. So the creation and destruction of instances can be used to control + whether or not signals are handled, and the rest of the interface can be used without an instance. + # Signal Handling To perform the tasks of exiting and terminating applications, `signal_handler` allows the user to control when signals - are handled, and when they aren't. In most cases, a user will control signal handling through the existence of - a `signal_handler` instance and watch for signals with signal_handler::watch_for_signals() run in a separate thread. - The user should create the instance of `signal_handler` when they want `signal_handler` - to start handling those signals, run the signal-watcher thread for as long as the application should react to - raised signals, and the handler should be destructed when handling the signals should cease. + are handled, and when they aren't. In most cases, a user will (1) control signal handling through the existence of + a `signal_handler` instance and then (2) wait for signals with a separate thread: + 1. The user should create the instance of `signal_handler` when they want `signal_handler` + to start handling those signals. + 2. The user should run the signal-waiting thread for as long as the application should react to + raised signals. + 3. Waiting for signals is complete when a signal is received or the wait is aborted; the waiting thread is then joined. + 4. The handler should be destructed when handling the signals should cease. + + ## Controling the scope of signal handling and which signals are handled + Here are several examples of how the lifetime of the signal_handler can be managed: 1. The instance is created as a global static object at static initialization time. Signals will be handled for the entire @@ -90,19 +101,28 @@ namespace scarab the signal_handler is destructed (or unhandle_signals() is called), during that period signals will not cause the application to exit. - For simple examples of creating a signal_handler and running a watcher thread, see the test_raise_sig*.cc files. - By default the signals handled are SIGABRT (abort() and unhandled exceptions), SIGTERM (shell command kill), SIGINT (ctrl-c), and SIGQUIT (ctrl-\). - The set of signals can be edited by modifying the handled signals map (via signal_handler::handled_signals()). + For customized signal handling, the set of signals can be edited by modifying the handled signals map + (via signal_handler::handled_signals()). In addition to this typical behavior, `signal_handler` includes an interface for more fine-grained control over when signals are handled while the `signal_handler` exists (i.e. handle_signals(), unhandle_signals(), and is_handling()), but the recommended use case is as described above, where signal handling is controled by the existence of a `signal_handler` instance. - Note that the interface for `signal_handler` uses all static functions, so there is no need to create an extra instance of - the class to use any part of that interface. So the creation and destruction of instances can be used to control - whether or not signals are handled, and the rest of the interface can be used without an instance. + ## Waiting for signals + + The function `signal_handler::wait_for_signals()` is a blocking call to wait for the arrival of signals. + That wait can be aborted asynchronously with `signal_handler::abort_wait()`. + The waiting is intended to be done in a separate thread from the main application execution. + + Thread management for the wait can be done manually (i.e. by creating a thread for `wait_for_signals()` in the application code) + or internally in signal_handler with `signal_handler::start_waiting_thread()` and `signal_handler::join_waiting_thread()`. + In the latter case, the thread object is a static member of the class. + + ## Examples + + For simple examples of creating a signal_handler and running a watcher thread, see the test_raise_sig*.cc files. # Cancelables @@ -168,18 +188,20 @@ namespace scarab /// Fine-grained signal-handling control; called from the destructor /// Returns the previous signal-handling functions to the signals. static void unhandle_signals(); - static bool is_handling(); // just calls get_is_waiting(); is here to have parallel syntax to waiting + static bool is_handling(); // just calls get_is_handling(); is here to have parallel syntax to waiting /// Blocking call to handle signals; waits for a signal /// Returns immediately if it's already been called; otherwise blocks then returns the signal or error value /// Has an option to not call the exit() function; defaults to true (i.e. exit() will be called) static int wait_for_signals( bool call_exit = true ); - //static int wait_on_signals(); /// Requests the aborting of waiting for a signal; causes do_handle_signal() to return static void abort_wait(); /// Check if a signal is handled - static bool is_waiting(); + static bool is_waiting(); + + static void start_waiting_thread(); + static void join_waiting_thread(); /// Handler for std::terminate -- does not cleanup memory or threads [[noreturn]] static void handle_terminate() noexcept; @@ -236,6 +258,7 @@ namespace scarab mv_accessible_static_noset( bool, is_handling ); + mv_referrable_static( std::thread, waiting_thread ); protected: static std::mutex s_handle_mutex; diff --git a/testing/applications/test_raise_sigabrt.cc b/testing/applications/test_raise_sigabrt.cc index 86beafd7..f5bfa2ca 100644 --- a/testing/applications/test_raise_sigabrt.cc +++ b/testing/applications/test_raise_sigabrt.cc @@ -46,7 +46,7 @@ int main(int , char ** ) scarab::signal_handler t_sh; LINFO( testlog, "Starting to wait-on-signals thread" ); - std::thread t_sh_thread( [](){scarab::signal_handler::wait_for_signals();} ); + scarab::signal_handler::start_waiting_thread(); std::this_thread::sleep_for( std::chrono::seconds(1) ); @@ -54,7 +54,7 @@ int main(int , char ** ) raise( SIGABRT ); LINFO( testlog, "Raised" ); - t_sh_thread.join(); + scarab::signal_handler::join_waiting_thread(); return( EXIT_SUCCESS ); } diff --git a/testing/applications/test_raise_sigint.cc b/testing/applications/test_raise_sigint.cc index 94f67be1..8d158d3c 100644 --- a/testing/applications/test_raise_sigint.cc +++ b/testing/applications/test_raise_sigint.cc @@ -33,7 +33,7 @@ int main(int , char ** ) scarab::signal_handler t_sh; LINFO( testlog, "Starting to wait-on-signals thread" ); - std::thread t_sh_thread( [](){scarab::signal_handler::wait_for_signals();} ); + scarab::signal_handler::start_waiting_thread(); std::this_thread::sleep_for( std::chrono::seconds(1) ); @@ -41,7 +41,7 @@ int main(int , char ** ) raise( SIGINT ); LINFO( testlog, "Raised" ); - t_sh_thread.join(); + scarab::signal_handler::join_waiting_thread(); return( EXIT_SUCCESS ); } diff --git a/testing/applications/test_raise_sigquit.cc b/testing/applications/test_raise_sigquit.cc index efa216ee..d9241a62 100644 --- a/testing/applications/test_raise_sigquit.cc +++ b/testing/applications/test_raise_sigquit.cc @@ -33,7 +33,7 @@ int main(int , char ** ) scarab::signal_handler t_sh; LINFO( testlog, "Starting to wait-on-signals thread" ); - std::thread t_sh_thread( [](){scarab::signal_handler::wait_for_signals();} ); + scarab::signal_handler::start_waiting_thread(); std::this_thread::sleep_for( std::chrono::seconds(1) ); @@ -41,7 +41,7 @@ int main(int , char ** ) raise( SIGQUIT ); LINFO( testlog, "Raised" ); - t_sh_thread.join(); + scarab::signal_handler::join_waiting_thread(); return( EXIT_SUCCESS ); } From 5a13e571a2c0e3d204fcc126d3bc2daa6fc3392a Mon Sep 17 00:00:00 2001 From: Noah Oblath Date: Mon, 15 Dec 2025 14:12:05 -0800 Subject: [PATCH 16/29] Cherry picking some updates from hf3.13.5 --- .github/workflows/run_tests.yaml | 2 +- changelog.md | 15 +++++++ library/utility/signal_handler.cc | 16 +++++++ library/utility/signal_handler.hh | 51 ++++++++++++++++------ testing/applications/test_raise_sigabrt.cc | 4 +- testing/applications/test_raise_sigint.cc | 4 +- testing/applications/test_raise_sigquit.cc | 4 +- 7 files changed, 75 insertions(+), 21 deletions(-) diff --git a/.github/workflows/run_tests.yaml b/.github/workflows/run_tests.yaml index 14af2ce5..bf7dc14b 100644 --- a/.github/workflows/run_tests.yaml +++ b/.github/workflows/run_tests.yaml @@ -17,7 +17,7 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-24.04, ubuntu-22.04, macos-26, macos-15-intel, macos-15, macos-14] + os: [ubuntu-24.04, ubuntu-22.04, macos-15-intel, macos-15, macos-14] steps: diff --git a/changelog.md b/changelog.md index bdbcbc85..2b23a7b5 100644 --- a/changelog.md +++ b/changelog.md @@ -9,6 +9,21 @@ Types of changes: Added, Changed, Deprecated, Removed, Fixed, Security ## [Unreleased] +## [3.13.5] - 2025-12-15 + +### Changed + +- Signal handling now requires a dedicated thread +- Updated GHA runners +- Removed local loggers + +### Fixed + +- Tied spdlog to a tag and not a branch +- Updated spdlog fork from upstream +- signal_handler signal handling functions use only approved function calls + + ## [3.13.4] - 2025-11-04 ### Fixed diff --git a/library/utility/signal_handler.cc b/library/utility/signal_handler.cc index db31a7d4..c62c60a5 100644 --- a/library/utility/signal_handler.cc +++ b/library/utility/signal_handler.cc @@ -39,6 +39,8 @@ namespace scarab bool signal_handler::s_is_handling = false; + std::thread signal_handler::s_waiting_thread; + std::mutex signal_handler::s_handle_mutex; signal_handler::signal_map_t signal_handler::s_handled_signals = { @@ -225,12 +227,26 @@ namespace scarab return ! t_lock.try_lock(); } + void signal_handler::start_waiting_thread() + { + signal_handler::s_waiting_thread = std::thread( [](){scarab::signal_handler::wait_for_signals();} ); + return; + } + + void signal_handler::join_waiting_thread() + { + s_waiting_thread.join(); + s_waiting_thread = std::thread(); + return; + } + void signal_handler::reset() { LDEBUG( slog, "Resetting signal_handler" ); signal_handler::unhandle_signals(); + s_waiting_thread = std::thread(); s_exited = false; s_return_code = RETURN_SUCCESS; s_cancelers.clear(); diff --git a/library/utility/signal_handler.hh b/library/utility/signal_handler.hh index 03a07510..720ea9c2 100644 --- a/library/utility/signal_handler.hh +++ b/library/utility/signal_handler.hh @@ -16,6 +16,7 @@ #include #include #include +#include #ifndef _GNU_SOURCE #define _GNU_SOURCE @@ -67,15 +68,25 @@ namespace scarab Responsibility for canceling a cancelable is given to the signal_handler using add_cancelable(), and it's taken away using remove_cancelable(). + Note that the interface for `signal_handler` uses all static functions, so there is no need to create an extra instance of + the class to use any part of that interface. So the creation and destruction of instances can be used to control + whether or not signals are handled, and the rest of the interface can be used without an instance. + # Signal Handling To perform the tasks of exiting and terminating applications, `signal_handler` allows the user to control when signals - are handled, and when they aren't. In most cases, a user will control signal handling through the existence of - a `signal_handler` instance and watch for signals with signal_handler::watch_for_signals() run in a separate thread. - The user should create the instance of `signal_handler` when they want `signal_handler` - to start handling those signals, run the signal-watcher thread for as long as the application should react to - raised signals, and the handler should be destructed when handling the signals should cease. + are handled, and when they aren't. In most cases, a user will (1) control signal handling through the existence of + a `signal_handler` instance and then (2) wait for signals with a separate thread: + 1. The user should create the instance of `signal_handler` when they want `signal_handler` + to start handling those signals. + 2. The user should run the signal-waiting thread for as long as the application should react to + raised signals. + 3. Waiting for signals is complete when a signal is received or the wait is aborted; the waiting thread is then joined. + 4. The handler should be destructed when handling the signals should cease. + + ## Controling the scope of signal handling and which signals are handled + Here are several examples of how the lifetime of the signal_handler can be managed: 1. The instance is created as a global static object at static initialization time. Signals will be handled for the entire @@ -90,19 +101,28 @@ namespace scarab the signal_handler is destructed (or unhandle_signals() is called), during that period signals will not cause the application to exit. - For simple examples of creating a signal_handler and running a watcher thread, see the test_raise_sig*.cc files. - By default the signals handled are SIGABRT (abort() and unhandled exceptions), SIGTERM (shell command kill), SIGINT (ctrl-c), and SIGQUIT (ctrl-\). - The set of signals can be edited by modifying the handled signals map (via signal_handler::handled_signals()). + For customized signal handling, the set of signals can be edited by modifying the handled signals map + (via signal_handler::handled_signals()). In addition to this typical behavior, `signal_handler` includes an interface for more fine-grained control over when signals are handled while the `signal_handler` exists (i.e. handle_signals(), unhandle_signals(), and is_handling()), but the recommended use case is as described above, where signal handling is controled by the existence of a `signal_handler` instance. - Note that the interface for `signal_handler` uses all static functions, so there is no need to create an extra instance of - the class to use any part of that interface. So the creation and destruction of instances can be used to control - whether or not signals are handled, and the rest of the interface can be used without an instance. + ## Waiting for signals + + The function `signal_handler::wait_for_signals()` is a blocking call to wait for the arrival of signals. + That wait can be aborted asynchronously with `signal_handler::abort_wait()`. + The waiting is intended to be done in a separate thread from the main application execution. + + Thread management for the wait can be done manually (i.e. by creating a thread for `wait_for_signals()` in the application code) + or internally in signal_handler with `signal_handler::start_waiting_thread()` and `signal_handler::join_waiting_thread()`. + In the latter case, the thread object is a static member of the class. + + ## Examples + + For simple examples of creating a signal_handler and running a watcher thread, see the test_raise_sig*.cc files. # Cancelables @@ -168,18 +188,20 @@ namespace scarab /// Fine-grained signal-handling control; called from the destructor /// Returns the previous signal-handling functions to the signals. static void unhandle_signals(); - static bool is_handling(); // just calls get_is_waiting(); is here to have parallel syntax to waiting + static bool is_handling(); // just calls get_is_handling(); is here to have parallel syntax to waiting /// Blocking call to handle signals; waits for a signal /// Returns immediately if it's already been called; otherwise blocks then returns the signal or error value /// Has an option to not call the exit() function; defaults to true (i.e. exit() will be called) static int wait_for_signals( bool call_exit = true ); - //static int wait_on_signals(); /// Requests the aborting of waiting for a signal; causes do_handle_signal() to return static void abort_wait(); /// Check if a signal is handled - static bool is_waiting(); + static bool is_waiting(); + + static void start_waiting_thread(); + static void join_waiting_thread(); /// Handler for std::terminate -- does not cleanup memory or threads [[noreturn]] static void handle_terminate() noexcept; @@ -236,6 +258,7 @@ namespace scarab mv_accessible_static_noset( bool, is_handling ); + mv_referrable_static( std::thread, waiting_thread ); protected: static std::mutex s_handle_mutex; diff --git a/testing/applications/test_raise_sigabrt.cc b/testing/applications/test_raise_sigabrt.cc index 86beafd7..f5bfa2ca 100644 --- a/testing/applications/test_raise_sigabrt.cc +++ b/testing/applications/test_raise_sigabrt.cc @@ -46,7 +46,7 @@ int main(int , char ** ) scarab::signal_handler t_sh; LINFO( testlog, "Starting to wait-on-signals thread" ); - std::thread t_sh_thread( [](){scarab::signal_handler::wait_for_signals();} ); + scarab::signal_handler::start_waiting_thread(); std::this_thread::sleep_for( std::chrono::seconds(1) ); @@ -54,7 +54,7 @@ int main(int , char ** ) raise( SIGABRT ); LINFO( testlog, "Raised" ); - t_sh_thread.join(); + scarab::signal_handler::join_waiting_thread(); return( EXIT_SUCCESS ); } diff --git a/testing/applications/test_raise_sigint.cc b/testing/applications/test_raise_sigint.cc index 94f67be1..8d158d3c 100644 --- a/testing/applications/test_raise_sigint.cc +++ b/testing/applications/test_raise_sigint.cc @@ -33,7 +33,7 @@ int main(int , char ** ) scarab::signal_handler t_sh; LINFO( testlog, "Starting to wait-on-signals thread" ); - std::thread t_sh_thread( [](){scarab::signal_handler::wait_for_signals();} ); + scarab::signal_handler::start_waiting_thread(); std::this_thread::sleep_for( std::chrono::seconds(1) ); @@ -41,7 +41,7 @@ int main(int , char ** ) raise( SIGINT ); LINFO( testlog, "Raised" ); - t_sh_thread.join(); + scarab::signal_handler::join_waiting_thread(); return( EXIT_SUCCESS ); } diff --git a/testing/applications/test_raise_sigquit.cc b/testing/applications/test_raise_sigquit.cc index efa216ee..d9241a62 100644 --- a/testing/applications/test_raise_sigquit.cc +++ b/testing/applications/test_raise_sigquit.cc @@ -33,7 +33,7 @@ int main(int , char ** ) scarab::signal_handler t_sh; LINFO( testlog, "Starting to wait-on-signals thread" ); - std::thread t_sh_thread( [](){scarab::signal_handler::wait_for_signals();} ); + scarab::signal_handler::start_waiting_thread(); std::this_thread::sleep_for( std::chrono::seconds(1) ); @@ -41,7 +41,7 @@ int main(int , char ** ) raise( SIGQUIT ); LINFO( testlog, "Raised" ); - t_sh_thread.join(); + scarab::signal_handler::join_waiting_thread(); return( EXIT_SUCCESS ); } From 8a74cb48451dda1b182de980ce42d2b80d45c684 Mon Sep 17 00:00:00 2001 From: Noah Oblath Date: Mon, 15 Dec 2025 14:27:12 -0800 Subject: [PATCH 17/29] Python-bind the waiting_thread functions --- python/signal_handler_pybind.hh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/python/signal_handler_pybind.hh b/python/signal_handler_pybind.hh index 908277bf..188f4961 100644 --- a/python/signal_handler_pybind.hh +++ b/python/signal_handler_pybind.hh @@ -27,6 +27,8 @@ namespace scarab_pybind .def_static( "reset", &scarab::signal_handler::reset, "remove all cancelable objects", SCARAB_BIND_CALL_GUARD_STREAMS ) .def_static( "cancel_all", &scarab::signal_handler::cancel_all, "cancel all cancelable objects known to the signal_handler", SCARAB_BIND_CALL_GUARD_STREAMS_AND_GIL ) .def_static( "print_cancelables", &scarab::signal_handler::print_cancelables, "print pointers to the cancelables known to the signal_handler", SCARAB_BIND_CALL_GUARD_STREAMS ) + .def_static( "start_waiting_thread", &scarab::signal_handler::start_waiting_thread, "starts the thread that waits for signals", SCARAB_BIND_CALL_GUARD_STREAMS ) + .def_static( "join_waiting_thread", &scarab::signal_handler::join_waiting_thread, "joins the thread that waits for signals", SCARAB_BIND_CALL_GUARD_STREAMS_AND_GIL ) ; return all_members; From 0bf29be7e28613a293a806d53732e2e997bb351b Mon Sep 17 00:00:00 2001 From: Noah Oblath Date: Mon, 15 Dec 2025 14:27:12 -0800 Subject: [PATCH 18/29] Python-bind the waiting_thread functions --- python/signal_handler_pybind.hh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/python/signal_handler_pybind.hh b/python/signal_handler_pybind.hh index 908277bf..188f4961 100644 --- a/python/signal_handler_pybind.hh +++ b/python/signal_handler_pybind.hh @@ -27,6 +27,8 @@ namespace scarab_pybind .def_static( "reset", &scarab::signal_handler::reset, "remove all cancelable objects", SCARAB_BIND_CALL_GUARD_STREAMS ) .def_static( "cancel_all", &scarab::signal_handler::cancel_all, "cancel all cancelable objects known to the signal_handler", SCARAB_BIND_CALL_GUARD_STREAMS_AND_GIL ) .def_static( "print_cancelables", &scarab::signal_handler::print_cancelables, "print pointers to the cancelables known to the signal_handler", SCARAB_BIND_CALL_GUARD_STREAMS ) + .def_static( "start_waiting_thread", &scarab::signal_handler::start_waiting_thread, "starts the thread that waits for signals", SCARAB_BIND_CALL_GUARD_STREAMS ) + .def_static( "join_waiting_thread", &scarab::signal_handler::join_waiting_thread, "joins the thread that waits for signals", SCARAB_BIND_CALL_GUARD_STREAMS_AND_GIL ) ; return all_members; From c01770fd094aa3b8d907c1137dcb29d2f0250e0d Mon Sep 17 00:00:00 2001 From: Noah Oblath Date: Fri, 19 Dec 2025 13:19:23 -0800 Subject: [PATCH 19/29] Adding more debug tracing --- library/logger/logger.cc | 7 ++++++- library/logger/logger.hh | 4 ++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/library/logger/logger.cc b/library/logger/logger.cc index 75274bd6..48177d7d 100644 --- a/library/logger/logger.cc +++ b/library/logger/logger.cc @@ -43,7 +43,12 @@ namespace scarab f_sink() { std::cerr << "initializing thread pool and mt sink" << std::endl; - spdlog::init_thread_pool(8192, 1); + int counter = __COUNTER__; + std::cerr << "initialization counter: " << counter << std::endl; + spdlog::init_thread_pool(8192, 1, + [counter](){ std::cerr << "thread starting for counter " << counter << std::endl; }, + [counter](){ std::cerr << "thread stopping for counter " << counter << std::endl; } + ); f_sink = std::make_shared< spdlog::sinks::stdout_color_sink_mt >(); f_sink->set_pattern( f_pattern ); auto at_exit_fcn = [](){ std::cerr << "at exit fcn" << std::endl; logger::stop_using_spd_async(); }; diff --git a/library/logger/logger.hh b/library/logger/logger.hh index 934498a0..72ddf0e3 100644 --- a/library/logger/logger.hh +++ b/library/logger/logger.hh @@ -187,9 +187,9 @@ namespace scarab logger( a_name ), f_initializer_ptr( nullptr ) { -#ifdef SCARAB_LOGGER_DEBUG +//#ifdef SCARAB_LOGGER_DEBUG std::cerr << "Logger <" << a_name << "> was initialized from <" << a_file << ":" << a_line << ">; type: " << ::scarab::type( *this ) << std::endl; -#endif +//#endif // Start the backend, but only once static initializer_x s_init; f_initializer_ptr = &s_init; From b84f49798ad09dfee04a2050120fcdfed57bb69a Mon Sep 17 00:00:00 2001 From: Noah Oblath Date: Fri, 19 Dec 2025 13:28:56 -0800 Subject: [PATCH 20/29] Fixing counter for debug tracing --- library/logger/logger.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/logger/logger.cc b/library/logger/logger.cc index 48177d7d..80710e35 100644 --- a/library/logger/logger.cc +++ b/library/logger/logger.cc @@ -43,12 +43,13 @@ namespace scarab f_sink() { std::cerr << "initializing thread pool and mt sink" << std::endl; - int counter = __COUNTER__; + static int counter = 0; std::cerr << "initialization counter: " << counter << std::endl; spdlog::init_thread_pool(8192, 1, [counter](){ std::cerr << "thread starting for counter " << counter << std::endl; }, [counter](){ std::cerr << "thread stopping for counter " << counter << std::endl; } ); + ++counter; f_sink = std::make_shared< spdlog::sinks::stdout_color_sink_mt >(); f_sink->set_pattern( f_pattern ); auto at_exit_fcn = [](){ std::cerr << "at exit fcn" << std::endl; logger::stop_using_spd_async(); }; From bb54d99e5d3fb5ec2c08d4d8b6ae5b4a6e55051f Mon Sep 17 00:00:00 2001 From: Noah Oblath Date: Fri, 19 Dec 2025 13:36:07 -0800 Subject: [PATCH 21/29] Fixing the fix in my last commit --- library/logger/logger.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/logger/logger.cc b/library/logger/logger.cc index 80710e35..8f4a3388 100644 --- a/library/logger/logger.cc +++ b/library/logger/logger.cc @@ -46,8 +46,8 @@ namespace scarab static int counter = 0; std::cerr << "initialization counter: " << counter << std::endl; spdlog::init_thread_pool(8192, 1, - [counter](){ std::cerr << "thread starting for counter " << counter << std::endl; }, - [counter](){ std::cerr << "thread stopping for counter " << counter << std::endl; } + [count = counter](){ std::cerr << "thread starting for counter " << counter << std::endl; }, + [count = counter](){ std::cerr << "thread stopping for counter " << counter << std::endl; } ); ++counter; f_sink = std::make_shared< spdlog::sinks::stdout_color_sink_mt >(); From eea7745b2a42bb18f5f169197c83c235bdff3aae Mon Sep 17 00:00:00 2001 From: Noah Oblath Date: Fri, 19 Dec 2025 13:43:05 -0800 Subject: [PATCH 22/29] :facepalm: --- library/logger/logger.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/logger/logger.cc b/library/logger/logger.cc index 8f4a3388..710c9f77 100644 --- a/library/logger/logger.cc +++ b/library/logger/logger.cc @@ -46,8 +46,8 @@ namespace scarab static int counter = 0; std::cerr << "initialization counter: " << counter << std::endl; spdlog::init_thread_pool(8192, 1, - [count = counter](){ std::cerr << "thread starting for counter " << counter << std::endl; }, - [count = counter](){ std::cerr << "thread stopping for counter " << counter << std::endl; } + [count = counter](){ std::cerr << "thread starting for counter " << count << std::endl; }, + [count = counter](){ std::cerr << "thread stopping for counter " << count << std::endl; } ); ++counter; f_sink = std::make_shared< spdlog::sinks::stdout_color_sink_mt >(); From 224364df37b37c9d7e870de56a2ae071fdf39223 Mon Sep 17 00:00:00 2001 From: Noah Oblath Date: Mon, 22 Dec 2025 10:36:21 -0800 Subject: [PATCH 23/29] Don't create a thread pool if one exists already --- library/logger/logger.cc | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/library/logger/logger.cc b/library/logger/logger.cc index 710c9f77..3b3a5be6 100644 --- a/library/logger/logger.cc +++ b/library/logger/logger.cc @@ -45,10 +45,18 @@ namespace scarab std::cerr << "initializing thread pool and mt sink" << std::endl; static int counter = 0; std::cerr << "initialization counter: " << counter << std::endl; - spdlog::init_thread_pool(8192, 1, - [count = counter](){ std::cerr << "thread starting for counter " << count << std::endl; }, - [count = counter](){ std::cerr << "thread stopping for counter " << count << std::endl; } - ); + if( ! spdlog::thread_pool() ) + { + std::cerr << "No thread pool exists; creating a new one" << std::endl; + spdlog::init_thread_pool(8192, 1, + [count = counter](){ std::cerr << "thread starting for counter " << count << std::endl; }, + [count = counter](){ std::cerr << "thread stopping for counter " << count << std::endl; } + ); + } + else + { + std::cerr << "Thread pool exists; not creating a new one" << std::endl; + } ++counter; f_sink = std::make_shared< spdlog::sinks::stdout_color_sink_mt >(); f_sink->set_pattern( f_pattern ); From 77f1fc19260d9884f486bf5e06868aec8fe1c9b3 Mon Sep 17 00:00:00 2001 From: Noah Oblath Date: Mon, 22 Dec 2025 11:20:23 -0800 Subject: [PATCH 24/29] Cleaning up debugging output --- library/logger/logger.cc | 28 +++++----------------------- library/logger/logger.hh | 6 +++--- 2 files changed, 8 insertions(+), 26 deletions(-) diff --git a/library/logger/logger.cc b/library/logger/logger.cc index 3b3a5be6..2bafa05b 100644 --- a/library/logger/logger.cc +++ b/library/logger/logger.cc @@ -24,16 +24,11 @@ namespace scarab std::shared_ptr< spdlog::logger > t_logger = spdlog::get( a_name ); if( ! t_logger ) { - std::cerr << "Creating spdlog logger <" << a_name << ">" << std::endl; // see the comment in logger.hh about why the a_make_logger_fcn callback is used instead of the virtual function call t_logger = a_make_logger_fcn( a_name ); //this->make_logger( a_name ); t_logger->set_level( spdlog::level::level_enum(unsigned(logger::ELevel::SCARAB_LOGGER_DEFAULT_THRESHOLD)) ); spdlog::register_logger( t_logger ); } - else - { - std::cerr << "Returning existing logger <" << a_name << ">" << std::endl; - } return t_logger; } @@ -42,25 +37,15 @@ namespace scarab spd_initializer( a_pattern ), f_sink() { - std::cerr << "initializing thread pool and mt sink" << std::endl; - static int counter = 0; - std::cerr << "initialization counter: " << counter << std::endl; + // We check before creating the new thread pool because we've had problems in some situations (e.g. when started from Python) + // with a new thread pool being created and the old one being killed, resulting in strange errors during execution if( ! spdlog::thread_pool() ) { - std::cerr << "No thread pool exists; creating a new one" << std::endl; - spdlog::init_thread_pool(8192, 1, - [count = counter](){ std::cerr << "thread starting for counter " << count << std::endl; }, - [count = counter](){ std::cerr << "thread stopping for counter " << count << std::endl; } - ); - } - else - { - std::cerr << "Thread pool exists; not creating a new one" << std::endl; + spdlog::init_thread_pool(8192, 1); } - ++counter; f_sink = std::make_shared< spdlog::sinks::stdout_color_sink_mt >(); f_sink->set_pattern( f_pattern ); - auto at_exit_fcn = [](){ std::cerr << "at exit fcn" << std::endl; logger::stop_using_spd_async(); }; + auto at_exit_fcn = [](){ logger::stop_using_spd_async(); }; std::atexit( at_exit_fcn ); } @@ -84,7 +69,6 @@ namespace scarab spd_initializer( a_pattern ), f_sink() { - std::cerr << "initializing st sink" << std::endl; f_sink = std::make_shared< spdlog::sinks::stdout_color_sink_st >(); f_sink->set_pattern( f_pattern ); } @@ -101,13 +85,11 @@ namespace scarab f_strstr(), f_spdlogger() { - std::cerr << "scarab::logger constructor: creating logger <" << a_name << ">" << std::endl; scarab::logger::all_loggers().insert(this); } logger::~logger() { - std::cerr << "destructing logger <" << f_name << ">" << std::endl; logger::all_loggers().erase(this); } @@ -224,7 +206,7 @@ namespace scarab void logger::reset_using_spd_async() { #ifdef SCARAB_LOGGER_DEBUG - std::cerr << "Resetting use of spd async" << std::endl; + std::cerr << "[logger::reset_using_spd_async()] Resetting use of spd async" << std::endl; #endif logger::using_spd_async().store(true); std::set< logger* >& t_all_loggers = logger::all_loggers(); diff --git a/library/logger/logger.hh b/library/logger/logger.hh index 72ddf0e3..cd68d2d0 100644 --- a/library/logger/logger.hh +++ b/library/logger/logger.hh @@ -187,9 +187,9 @@ namespace scarab logger( a_name ), f_initializer_ptr( nullptr ) { -//#ifdef SCARAB_LOGGER_DEBUG - std::cerr << "Logger <" << a_name << "> was initialized from <" << a_file << ":" << a_line << ">; type: " << ::scarab::type( *this ) << std::endl; -//#endif +#ifdef SCARAB_LOGGER_DEBUG + std::cerr << "[logger_type constructor] Logger <" << a_name << "> was initialized from <" << a_file << ":" << a_line << ">; type: " << ::scarab::type( *this ) << std::endl; +#endif // Start the backend, but only once static initializer_x s_init; f_initializer_ptr = &s_init; From 75fbe755c7eabbdf49ef2981728bcf16af663a50 Mon Sep 17 00:00:00 2001 From: Noah Oblath Date: Mon, 22 Dec 2025 11:40:27 -0800 Subject: [PATCH 25/29] [no ci] Updating the changelog --- changelog.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/changelog.md b/changelog.md index 2b23a7b5..0d2382bb 100644 --- a/changelog.md +++ b/changelog.md @@ -9,7 +9,7 @@ Types of changes: Added, Changed, Deprecated, Removed, Fixed, Security ## [Unreleased] -## [3.13.5] - 2025-12-15 +## [3.13.5] - 2025-12-22 ### Changed @@ -22,6 +22,7 @@ Types of changes: Added, Changed, Deprecated, Removed, Fixed, Security - Tied spdlog to a tag and not a branch - Updated spdlog fork from upstream - signal_handler signal handling functions use only approved function calls +- Check if spdlog thread pool exists before making a new one ## [3.13.4] - 2025-11-04 From 2dfdb00b9493957acb13d55d3bd38b490f475a49 Mon Sep 17 00:00:00 2001 From: Noah Oblath Date: Mon, 22 Dec 2025 12:05:03 -0800 Subject: [PATCH 26/29] Switch SIGTERM to a RETURN_SUCCESS response --- library/utility/signal_handler.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/utility/signal_handler.cc b/library/utility/signal_handler.cc index c62c60a5..9d1d193b 100644 --- a/library/utility/signal_handler.cc +++ b/library/utility/signal_handler.cc @@ -45,7 +45,7 @@ namespace scarab signal_handler::signal_map_t signal_handler::s_handled_signals = { {SIGABRT, signal_handler::handled_signal_info{"SIGABRT", false, {}, true, RETURN_ERROR}}, - {SIGTERM, signal_handler::handled_signal_info{"SIGTERM", false, {}, true, RETURN_ERROR}}, + {SIGTERM, signal_handler::handled_signal_info{"SIGTERM", false, {}, true, RETURN_SUCCESS}}, {SIGINT, signal_handler::handled_signal_info{"SIGINT", false, {}, false, RETURN_SUCCESS}}, {SIGQUIT, signal_handler::handled_signal_info{"SIGQUIT", false, {}, false, RETURN_SUCCESS}} }; From 19c3c1db0ff1e4ff33d1b50fd8637b483108596a Mon Sep 17 00:00:00 2001 From: Noah Oblath Date: Mon, 22 Dec 2025 12:18:01 -0800 Subject: [PATCH 27/29] Fix is_error for SIGTERM --- library/utility/signal_handler.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/utility/signal_handler.cc b/library/utility/signal_handler.cc index 9d1d193b..badba08c 100644 --- a/library/utility/signal_handler.cc +++ b/library/utility/signal_handler.cc @@ -44,8 +44,9 @@ namespace scarab std::mutex signal_handler::s_handle_mutex; signal_handler::signal_map_t signal_handler::s_handled_signals = { + // handled_signal_info args: name, is handling, old action, indicates error, return code {SIGABRT, signal_handler::handled_signal_info{"SIGABRT", false, {}, true, RETURN_ERROR}}, - {SIGTERM, signal_handler::handled_signal_info{"SIGTERM", false, {}, true, RETURN_SUCCESS}}, + {SIGTERM, signal_handler::handled_signal_info{"SIGTERM", false, {}, false, RETURN_SUCCESS}}, {SIGINT, signal_handler::handled_signal_info{"SIGINT", false, {}, false, RETURN_SUCCESS}}, {SIGQUIT, signal_handler::handled_signal_info{"SIGQUIT", false, {}, false, RETURN_SUCCESS}} }; From 845f954f36bff47b4e0026f3d964897a824f8ac8 Mon Sep 17 00:00:00 2001 From: Noah Oblath Date: Mon, 22 Dec 2025 14:34:20 -0800 Subject: [PATCH 28/29] Added default running of the waiting thread in signal_handler --- library/utility/signal_handler.cc | 45 +++++++++++------ library/utility/signal_handler.hh | 59 +++++++++++++++------- testing/applications/test_raise_sigabrt.cc | 2 +- testing/applications/test_raise_sigint.cc | 4 +- testing/applications/test_raise_sigquit.cc | 4 +- testing/test_signal_handler.cc | 54 ++++++++++++++++---- 6 files changed, 119 insertions(+), 49 deletions(-) diff --git a/library/utility/signal_handler.cc b/library/utility/signal_handler.cc index badba08c..6ba6ea8a 100644 --- a/library/utility/signal_handler.cc +++ b/library/utility/signal_handler.cc @@ -34,7 +34,7 @@ namespace scarab bool signal_handler::s_exited = false; int signal_handler::s_return_code = RETURN_SUCCESS; - std::recursive_mutex signal_handler::s_mutex; + std::mutex signal_handler::s_cancelers_mutex; signal_handler::cancelers signal_handler::s_cancelers; bool signal_handler::s_is_handling = false; @@ -53,13 +53,18 @@ namespace scarab volatile sig_atomic_t signal_handler::s_signal_received = 0; - signal_handler::signal_handler() + signal_handler::signal_handler( bool start_waiting_thread ) { ++signal_handler::s_ref_count; if( signal_handler::s_ref_count == 1 ) { signal_handler::start_handling_signals(); } + if( start_waiting_thread && ! signal_handler::s_waiting_thread.joinable() ) + { + signal_handler::start_waiting_thread(); + } + } signal_handler::~signal_handler() @@ -73,28 +78,28 @@ namespace scarab void signal_handler::add_cancelable( std::shared_ptr< scarab::cancelable > a_cancelable ) { - std::unique_lock< std::recursive_mutex > t_lock( s_mutex ); + std::unique_lock< std::mutex > t_lock( s_cancelers_mutex ); s_cancelers.insert( std::make_pair(a_cancelable.get(), cancelable_wptr_t(a_cancelable) ) ); return; } void signal_handler::remove_cancelable( std::shared_ptr< scarab::cancelable > a_cancelable ) { - std::unique_lock< std::recursive_mutex > t_lock( s_mutex ); + std::unique_lock< std::mutex > t_lock( s_cancelers_mutex ); s_cancelers.erase( a_cancelable.get() ); return; } void signal_handler::remove_cancelable( scarab::cancelable* a_cancelable ) { - std::unique_lock< std::recursive_mutex > t_lock( s_mutex ); + std::unique_lock< std::mutex > t_lock( s_cancelers_mutex ); s_cancelers.erase( a_cancelable ); return; } void signal_handler::print_cancelables() { - std::unique_lock< std::recursive_mutex > t_lock( s_mutex ); + std::unique_lock< std::mutex > t_lock( s_cancelers_mutex ); LOGGER( slog_print, "signal_handler print_cancelables" ); for( const auto& [t_key, t_value] : s_cancelers ) { @@ -107,7 +112,7 @@ namespace scarab { if( signal_handler::get_is_handling() || signal_handler::is_waiting() ) return; - std::unique_lock t_lock( s_handle_mutex ); + std::unique_lock< std::mutex > t_lock( s_handle_mutex ); LOGGER( slog_constr, "signal_handler::start_handling_signals" ); LDEBUG( slog_constr, "Taking over signal handling for SIGABRT, SIGTERM, SIGINT, and SIGQUIT" ); @@ -146,7 +151,7 @@ namespace scarab if( signal_handler::is_waiting() ) signal_handler::abort_wait(); - std::unique_lock t_lock( s_handle_mutex ); + std::unique_lock< std::mutex > t_lock( s_handle_mutex ); LDEBUG( slog, "Returning signal handling for SIGABRT, SIGTERM, SIGINT, and SIGQUIT" ); @@ -171,14 +176,17 @@ namespace scarab int signal_handler::wait_for_signals( bool call_exit ) { // Try to lock the mutex; if it didn't lock, then return - std::unique_lock t_lock( s_handle_mutex, std::defer_lock_t() ); + std::unique_lock< std::mutex > t_lock( s_handle_mutex, std::defer_lock_t() ); if( ! t_lock.try_lock() ) return -1; + LTRACE( slog, "Started wait_for_signals" ); while( signal_handler::s_signal_received == 0 ) { std::this_thread::sleep_for( std::chrono::milliseconds(500)) ; } + LTRACE( slog, "past wait-for-signals loop" ); + // Handle situation where a signal was recieved if( s_signal_received > 0 ) { @@ -224,12 +232,13 @@ namespace scarab bool signal_handler::is_waiting() { - std::unique_lock t_lock( s_handle_mutex, std::defer_lock_t() ); + std::unique_lock< std::mutex > t_lock( s_handle_mutex, std::defer_lock_t() ); return ! t_lock.try_lock(); } void signal_handler::start_waiting_thread() { + if( signal_handler::s_waiting_thread.joinable() ) return; signal_handler::s_waiting_thread = std::thread( [](){scarab::signal_handler::wait_for_signals();} ); return; } @@ -241,11 +250,16 @@ namespace scarab return; } + bool signal_handler::waiting_thread_joinable() + { + return signal_handler::s_waiting_thread.joinable(); + } + void signal_handler::reset() { LDEBUG( slog, "Resetting signal_handler" ); - signal_handler::unhandle_signals(); + signal_handler::unhandle_signals(); // calls abort_wait() if waiting s_waiting_thread = std::thread(); s_exited = false; @@ -264,7 +278,7 @@ namespace scarab { signal_handler::s_signal_received = a_sig; - const char str[] = "Received a signal\n"; + const char str[] = "[signal_handler::handle_signal()] Received a signal\n"; write( STDERR_FILENO, str, sizeof(str)-1 ); return; @@ -272,7 +286,6 @@ namespace scarab [[noreturn]] void signal_handler::terminate( int a_code ) noexcept { - std::unique_lock< std::recursive_mutex > t_lock( s_mutex ); print_current_exception( false ); if( a_code > 0 ) { @@ -283,7 +296,6 @@ namespace scarab void signal_handler::exit( int a_code ) { - std::unique_lock< std::recursive_mutex > t_lock( s_mutex ); s_exited = true; s_return_code = a_code; print_current_exception( true ); @@ -363,9 +375,12 @@ namespace scarab void signal_handler::cancel_all( int a_code ) { - std::unique_lock< std::recursive_mutex > t_lock( s_mutex ); + std::unique_lock< std::mutex > t_lock( s_cancelers_mutex ); LDEBUG( slog, "Canceling all cancelables" ); + // Cause the waiting loop to abort if it's running + if( signal_handler::s_signal_received == 0 ) signal_handler::s_signal_received = -1; + while( ! s_cancelers.empty() ) { auto t_canceler_it = s_cancelers.begin(); diff --git a/library/utility/signal_handler.hh b/library/utility/signal_handler.hh index 720ea9c2..1fae7507 100644 --- a/library/utility/signal_handler.hh +++ b/library/utility/signal_handler.hh @@ -74,16 +74,27 @@ namespace scarab # Signal Handling - To perform the tasks of exiting and terminating applications, `signal_handler` allows the user to control when signals - are handled, and when they aren't. In most cases, a user will (1) control signal handling through the existence of - a `signal_handler` instance and then (2) wait for signals with a separate thread: - - 1. The user should create the instance of `signal_handler` when they want `signal_handler` - to start handling those signals. - 2. The user should run the signal-waiting thread for as long as the application should react to - raised signals. - 3. Waiting for signals is complete when a signal is received or the wait is aborted; the waiting thread is then joined. - 4. The handler should be destructed when handling the signals should cease. + The process of signal handling has several steps, some of which are automated: + + 1. Create the signal handler + 2. Start handling signals --- the default function that gets called when a signal is raised is replaced with signal_handler's function + 3. Start the thread waiting for signals + 4. Run the application, including the possibility of raising a signal + 5. Join the thread waiting for signals + 6. Stop handling signals -- the default signal-handling function is put back in place + 7. Signal handler is destructed + + In practice, the user has several options for performing these steps in application code. + The recommended method is the fully automatic method described here. + For other options on thread management, see below. + + 1. Create the signal handler with the default argument for `start_waiting_thread` (true) + * Creates the signal handler + * Starts handling signals + * Runs the signal-waiting thread + 2. Run the application + 3. Join the signal-waiting thread with `signal_handler::join_waiting_thread()` + 4. Let signal handler be destructed automatically ## Controling the scope of signal handling and which signals are handled @@ -112,13 +123,21 @@ namespace scarab ## Waiting for signals - The function `signal_handler::wait_for_signals()` is a blocking call to wait for the arrival of signals. - That wait can be aborted asynchronously with `signal_handler::abort_wait()`. - The waiting is intended to be done in a separate thread from the main application execution. + The process of waiting for signals requires a separate thread, and that thread can be managed in three ways: - Thread management for the wait can be done manually (i.e. by creating a thread for `wait_for_signals()` in the application code) - or internally in signal_handler with `signal_handler::start_waiting_thread()` and `signal_handler::join_waiting_thread()`. - In the latter case, the thread object is a static member of the class. + 1. Automatically -- The thread is started when the first `signal_handler` is constructed. + The thread should still be joined with `signal_handler::join_waiting_thread()` + at the appropriate time (after all application functionality has started). + This option is the default, but can be avoided by setting the `start_waiting_thread` argument to the + `signal_handler` constructor to false. + 2. Manually with internal thread management -- Create the `signal_handler` with `start_waiting_thread` set to false, + and then use `signal_handler::start_waiting_thread()` to manually start the thread. + Use `signal_handler::join_waiting_thread()` to join the thread at the appropriate time. + 3. Manually with user thread management -- Create the `signal_handler` with `start_waiting_thread` set to false, + and then start your own thread to run `signal_handler::wait_for_signals()`, which is a blocking call to wait + for the arrival of signals. + + The waiting for signals is cancelled with `signal_handler::abort_wait()` regardless of hwo the waiting thread is managed. ## Examples @@ -162,7 +181,7 @@ namespace scarab class SCARAB_API signal_handler { public: - signal_handler(); + signal_handler( bool start_waiting_thread = true ); signal_handler( const signal_handler& ) = delete; signal_handler( signal_handler&& ) = delete; virtual ~signal_handler(); @@ -200,8 +219,12 @@ namespace scarab /// Check if a signal is handled static bool is_waiting(); + /// Starts the thread waiting on signals (internally using wait_for_signals() and an internal thread storage) static void start_waiting_thread(); + /// Joins the internal wait-for-signals thread static void join_waiting_thread(); + /// Checks if the internal thread is in use + static bool waiting_thread_joinable(); /// Handler for std::terminate -- does not cleanup memory or threads [[noreturn]] static void handle_terminate() noexcept; @@ -235,7 +258,7 @@ namespace scarab typedef cancelers::iterator cancelers_it_t; static cancelers s_cancelers; - static std::recursive_mutex s_mutex; + static std::mutex s_cancelers_mutex; public: struct handled_signal_info diff --git a/testing/applications/test_raise_sigabrt.cc b/testing/applications/test_raise_sigabrt.cc index f5bfa2ca..ffde286a 100644 --- a/testing/applications/test_raise_sigabrt.cc +++ b/testing/applications/test_raise_sigabrt.cc @@ -43,7 +43,7 @@ LOGGER_ST( testlog, "test_raise_sigabrt" ); int main(int , char ** ) { - scarab::signal_handler t_sh; + scarab::signal_handler t_sh( false ); LINFO( testlog, "Starting to wait-on-signals thread" ); scarab::signal_handler::start_waiting_thread(); diff --git a/testing/applications/test_raise_sigint.cc b/testing/applications/test_raise_sigint.cc index 8d158d3c..15c404a8 100644 --- a/testing/applications/test_raise_sigint.cc +++ b/testing/applications/test_raise_sigint.cc @@ -32,9 +32,7 @@ int main(int , char ** ) { scarab::signal_handler t_sh; - LINFO( testlog, "Starting to wait-on-signals thread" ); - scarab::signal_handler::start_waiting_thread(); - + // let the signal-waiting thread start up std::this_thread::sleep_for( std::chrono::seconds(1) ); LINFO( testlog, "Raising SIGINT" ); diff --git a/testing/applications/test_raise_sigquit.cc b/testing/applications/test_raise_sigquit.cc index d9241a62..1c205210 100644 --- a/testing/applications/test_raise_sigquit.cc +++ b/testing/applications/test_raise_sigquit.cc @@ -32,9 +32,7 @@ int main(int , char ** ) { scarab::signal_handler t_sh; - LINFO( testlog, "Starting to wait-on-signals thread" ); - scarab::signal_handler::start_waiting_thread(); - + // let the signal-waiting thread start up std::this_thread::sleep_for( std::chrono::seconds(1) ); LINFO( testlog, "Raising SIGQUIT" ); diff --git a/testing/test_signal_handler.cc b/testing/test_signal_handler.cc index 79e33bcc..b645130d 100644 --- a/testing/test_signal_handler.cc +++ b/testing/test_signal_handler.cc @@ -40,13 +40,14 @@ TEST_CASE( "signal_handler", "[utility]" ) { REQUIRE( scarab::signal_handler::get_ref_count() == 0 ); REQUIRE_FALSE( scarab::signal_handler::is_handling() ); - REQUIRE_FALSE( scarab::signal_handler::is_waiting() ); REQUIRE( scarab::signal_handler::get_signal_received() == 0 ); + REQUIRE_FALSE( scarab::signal_handler::waiting_thread_joinable() ); - scarab::signal_handler t_handler; + scarab::signal_handler t_handler( false ); REQUIRE( scarab::signal_handler::get_ref_count() == 1 ); REQUIRE( scarab::signal_handler::is_handling() ); REQUIRE_FALSE( scarab::signal_handler::is_waiting() ); + REQUIRE_FALSE( scarab::signal_handler::waiting_thread_joinable() ); t_handler.unhandle_signals(); REQUIRE_FALSE( scarab::signal_handler::is_handling() ); @@ -56,34 +57,68 @@ TEST_CASE( "signal_handler", "[utility]" ) { REQUIRE( scarab::signal_handler::get_ref_count() == 0 ); REQUIRE_FALSE( scarab::signal_handler::is_handling() ); + REQUIRE_FALSE( scarab::signal_handler::waiting_thread_joinable() ); { - scarab::signal_handler t_handler; + scarab::signal_handler t_handler( false ); REQUIRE( scarab::signal_handler::get_ref_count() == 1 ); REQUIRE( scarab::signal_handler::is_handling() ); + REQUIRE_FALSE( scarab::signal_handler::waiting_thread_joinable() ); } // t_handler destructed, which should unhandle signals REQUIRE( scarab::signal_handler::get_ref_count() == 0 ); REQUIRE_FALSE( scarab::signal_handler::is_handling() ); + REQUIRE_FALSE( scarab::signal_handler::waiting_thread_joinable() ); } - SECTION( "wait then abort" ) + SECTION( "wait then abort -- user wait thread" ) { - scarab::signal_handler t_handler; + scarab::signal_handler t_handler( false ); REQUIRE( scarab::signal_handler::is_handling() ); - REQUIRE_FALSE( scarab::signal_handler::is_waiting() ); + REQUIRE_FALSE( scarab::signal_handler::waiting_thread_joinable() ); // start handling threads std::thread t_handler_thread( [](){ scarab::signal_handler::wait_for_signals(); } ); std::this_thread::sleep_for( std::chrono::seconds(1) ); - REQUIRE( scarab::signal_handler::is_waiting() ); scarab::signal_handler::abort_wait(); t_handler_thread.join(); REQUIRE( scarab::signal_handler::get_signal_received() == -1 ); REQUIRE( scarab::signal_handler::is_handling() ); - REQUIRE_FALSE( scarab::signal_handler::is_waiting() ); + REQUIRE_FALSE( scarab::signal_handler::waiting_thread_joinable() ); + } + + SECTION( "wait then abort -- manual wait thread" ) + { + scarab::signal_handler t_handler( false ); + REQUIRE( scarab::signal_handler::is_handling() ); + REQUIRE_FALSE( scarab::signal_handler::waiting_thread_joinable() ); + + // start handling threads + scarab::signal_handler::start_waiting_thread(); + std::this_thread::sleep_for( std::chrono::seconds(2) ); + REQUIRE( scarab::signal_handler::waiting_thread_joinable() ); + + scarab::signal_handler::abort_wait(); + scarab::signal_handler::join_waiting_thread(); + REQUIRE( scarab::signal_handler::get_signal_received() == -1 ); + REQUIRE( scarab::signal_handler::is_handling() ); + REQUIRE_FALSE( scarab::signal_handler::waiting_thread_joinable() ); + } + + SECTION( "wait then abort -- automatic wait thread" ) + { + scarab::signal_handler t_handler; + std::this_thread::sleep_for( std::chrono::seconds(2) ); + REQUIRE( scarab::signal_handler::is_handling() ); + REQUIRE( scarab::signal_handler::waiting_thread_joinable() ); + + scarab::signal_handler::abort_wait(); + scarab::signal_handler::join_waiting_thread(); + REQUIRE( scarab::signal_handler::get_signal_received() == -1 ); + REQUIRE( scarab::signal_handler::is_handling() ); + REQUIRE_FALSE( scarab::signal_handler::waiting_thread_joinable() ); } /* @@ -111,7 +146,7 @@ TEST_CASE( "signal_handler", "[utility]" ) } } */ - scarab::signal_handler t_handler; + scarab::signal_handler t_handler( false ); scarab::cancelable t_cancel; REQUIRE_FALSE( t_cancel.is_canceled() ); @@ -124,6 +159,7 @@ TEST_CASE( "signal_handler", "[utility]" ) { t_handler.cancel_all( 0 ); REQUIRE( t_cancel.is_canceled() ); + REQUIRE( scarab::signal_handler::get_signal_received() == -1 ); } SECTION( "terminating" ) From 713f0d61fa81a2040e83ba5525c7e7ef4ab15a87 Mon Sep 17 00:00:00 2001 From: Noah Oblath Date: Tue, 23 Dec 2025 17:38:56 -0800 Subject: [PATCH 29/29] Modified add_cancelable pybind signature --- python/signal_handler_pybind.hh | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/python/signal_handler_pybind.hh b/python/signal_handler_pybind.hh index 188f4961..7af3469c 100644 --- a/python/signal_handler_pybind.hh +++ b/python/signal_handler_pybind.hh @@ -18,9 +18,13 @@ namespace scarab_pybind pybind11::return_value_policy::take_ownership, SCARAB_BIND_CALL_GUARD_STREAMS ) +// .def("set_wp_potentially_slicing", +// [](WpOwner &self, py::handle obj) { +// self.set_wp(py::potentially_slicing_weak_ptr(obj)); +// }) .def_static( "add_cancelable", - [](std::shared_ptr a_cancelable) { - scarab::signal_handler::add_cancelable(pybind11::potentially_slicing_weak_ptr(pybind11::cast(a_cancelable)).lock()); + [](pybind11::handle a_cancelable) { + scarab::signal_handler::add_cancelable(pybind11::potentially_slicing_weak_ptr(a_cancelable).lock()); }, "add a cancelable object to the list to be canceled in the event of a SIGINT" ) .def_static( "remove_cancelable", static_cast)>( &scarab::signal_handler::remove_cancelable ), "remove a cancelable object from the list to be canceled in the event of a SIGINT" )