diff --git a/nano/core_test/CMakeLists.txt b/nano/core_test/CMakeLists.txt index 276829df6..901ceac1f 100644 --- a/nano/core_test/CMakeLists.txt +++ b/nano/core_test/CMakeLists.txt @@ -31,6 +31,7 @@ add_executable( peer_container.cpp prioritization.cpp request_aggregator.cpp + signal_manager.cpp signing.cpp socket.cpp telemetry.cpp diff --git a/nano/core_test/signal_manager.cpp b/nano/core_test/signal_manager.cpp new file mode 100644 index 000000000..e0b0bca28 --- /dev/null +++ b/nano/core_test/signal_manager.cpp @@ -0,0 +1,106 @@ +/** + * IMPORTANT NOTE: + * These unit tests may or may not work, gtest and boost asio signal handling are not really compatible. + * The boost asio signal handling assumes that it is the only one handling signals but gtest + * also does some signal handling of its own. In my testing this setup works although in theory + * I am playing with unspecified behaviour. If these tests start causing problems then we should + * remove them and try some other approach. + * The tests are designed as death tests because, as normal tests, the boost library asserts + * when I define more than one test case. I have not investigated why, I just turned them into death tests. + * + * Update: it appears that these tests only work if run in isolation so I am disabling them. + */ + +#include + +#include + +#include + +#include +#include + +static void handler_print_signal (int signum) +{ + std::cerr << "boost signal handler " << signum << std::endl + << std::flush; +} + +static int wait_for_sig_received (int millisecs, int & sig_received) +{ + for (int i = 0; i < millisecs && sig_received == 0; i++) + { + std::this_thread::sleep_for (std::chrono::microseconds (1)); + } + return sig_received; +} + +static int trap (int signum) +{ + nano::signal_manager sigman; + int sig_received = 0; + + std::function f = [&sig_received] (int signum) { + std::cerr << "boost signal handler " << signum << std::endl + << std::flush; + sig_received = signum; + }; + + sigman.register_signal_handler (signum, f, false); + + raise (signum); + + exit (wait_for_sig_received (10000, sig_received)); +} + +static void repeattest (int signum, bool repeat) +{ + nano::signal_manager sigman; + int sig_received = 0; + + std::function f = [&sig_received] (int signum) { + std::cerr << "boost signal handler" << std::flush; + sig_received = signum; + }; + + sigman.register_signal_handler (signum, f, repeat); + + for (int i = 0; i < 10; i++) + { + sig_received = 0; + raise (signum); + if (wait_for_sig_received (10000, sig_received) != signum) + { + exit (1); + } + } + + exit (0); +} + +TEST (DISABLED_signal_manager_test, trap) +{ + int signum; + + signum = SIGINT; + ASSERT_EXIT (trap (signum), ::testing::ExitedWithCode (signum), ""); + + signum = SIGTERM; + ASSERT_EXIT (trap (signum), ::testing::ExitedWithCode (signum), ""); +} + +TEST (DISABLED_signal_manager_test, repeat) +{ + int signum; + + signum = SIGINT; + ASSERT_EXIT (repeattest (signum, true), ::testing::ExitedWithCode (0), ""); +} + +TEST (DISABLED_signal_manager_test, norepeat) +{ + int signum; + + signum = SIGINT; + ASSERT_DEATH (repeattest (signum, false), "^boost signal handler$"); +} diff --git a/nano/lib/CMakeLists.txt b/nano/lib/CMakeLists.txt index a34d2632d..3a99d8f25 100644 --- a/nano/lib/CMakeLists.txt +++ b/nano/lib/CMakeLists.txt @@ -62,6 +62,8 @@ add_library( rpc_handler_interface.hpp rpcconfig.hpp rpcconfig.cpp + signal_manager.hpp + signal_manager.cpp stats.hpp stats.cpp stream.hpp diff --git a/nano/lib/signal_manager.cpp b/nano/lib/signal_manager.cpp new file mode 100644 index 000000000..b61d9c8eb --- /dev/null +++ b/nano/lib/signal_manager.cpp @@ -0,0 +1,93 @@ +#include +#include + +#include +#include +#include +#include + +#include + +nano::signal_manager::signal_manager () : + work (boost::asio::make_work_guard (ioc)) +{ + smthread = boost::thread ([&ioc = ioc] () { + ioc.run (); + }); +} + +nano::signal_manager::~signal_manager () +{ + /// Indicate that we have finished with the private io_context. Its + /// io_context::run() function will exit once all other work has completed. + work.reset (); + ioc.stop (); + smthread.join (); +} + +nano::signal_manager::signal_descriptor::signal_descriptor (std::shared_ptr sigset_a, signal_manager & sigman_a, std::function handler_func_a, bool repeat_a) : + sigset (sigset_a), sigman (sigman_a), handler_func (handler_func_a), repeat (repeat_a) +{ +} + +void nano::signal_manager::register_signal_handler (int signum, std::function handler, bool repeat) +{ + // create a signal set to hold the mapping between signals and signal handlers + auto sigset = std::make_shared (ioc, signum); + + // a signal descriptor holds all the data needed by the base handler including the signal set + // working with copies of a descriptor is OK + signal_descriptor descriptor (sigset, *this, handler, repeat); + + // ensure the signal set and descriptors live long enough + descriptor_list.push_back (descriptor); + + // asynchronously listen for signals from this signal set + sigset->async_wait ([descriptor] (const boost::system::error_code & error, int signum) { + nano::signal_manager::base_handler (descriptor, error, signum); + }); + + log (boost::str (boost::format ("Registered signal handler for signal %d") % signum)); +} + +std::function nano::signal_manager::get_debug_files_handler (void) +{ + return [] (int) { + nano::dump_crash_stacktrace (); + nano::create_load_memory_address_files (); + }; +} + +void nano::signal_manager::base_handler (nano::signal_manager::signal_descriptor descriptor, const boost::system::error_code & error, int signum) +{ + if (!error) + { + descriptor.sigman.log (boost::str (boost::format ("Signal received: %d") % signum)); + + // call the user supplied function, if one is provided + if (descriptor.handler_func) + { + descriptor.handler_func (signum); + } + + // continue asynchronously listening for signals from this signal set + if (descriptor.repeat) + { + descriptor.sigset->async_wait (boost::bind (base_handler, descriptor, _1, _2)); + descriptor.sigset->async_wait ([descriptor] (const boost::system::error_code & error, int signum) { + nano::signal_manager::base_handler (descriptor, error, signum); + }); + } + else + { + descriptor.sigman.log (boost::str (boost::format ("Signal handler %d will not repeat") % signum)); + descriptor.sigset->clear (); + } + + descriptor.sigman.log (boost::str (boost::format ("Signal processed: %d") % signum)); + } + else + { + descriptor.sigman.log (boost::str (boost::format ("Signal error: %d (%s)") % error.value () % error.message ())); + } +} diff --git a/nano/lib/signal_manager.hpp b/nano/lib/signal_manager.hpp new file mode 100644 index 000000000..b667ae561 --- /dev/null +++ b/nano/lib/signal_manager.hpp @@ -0,0 +1,82 @@ +#pragma once + +#include + +#include +#include +#include + +#include +#include +#include +#include + +namespace nano +{ +/** + * Manages signal handling and allows to register custom handlers for any signal. + * IMPORTANT NOTE: only one instance of this class should be instantiated per process. + * IMPORTANT NOTE: this is an add-only class, there is currently no way to remove a handler, + although that functionality could be easily be added if needed. + */ +class signal_manager final +{ +public: + /** The signal manager expects to have a boost asio context */ + signal_manager (); + + /** stops the signal manager io context and wait for the thread to finish */ + ~signal_manager (); + + /** Register a handler for a signal to be called from a safe context. + * The handler will be called from the "ioc" io context. + */ + void register_signal_handler (int signum, std::function handler, bool repeat); + + /** returns a signal handler that prints a stacktrace and creates some debug files */ + std::function get_debug_files_handler (void); + +private: + struct signal_descriptor final + { + signal_descriptor (std::shared_ptr sigset_a, signal_manager & sigman_a, std::function handler_func_a, bool repeat_a); + + /** a signal set that maps signals to signal handler and provides the connection to boost asio */ + std::shared_ptr sigset; + + /** reference to the signal manager that owns this signal descriptor */ + signal_manager & sigman; + + /** the caller supplied function to call from the base signal handler */ + std::function handler_func; + + /** indicates if the signal handler should continue handling a signal after receiving one */ + bool repeat; + }; + + /** + * Logging function of signal manager. It does nothing at the moment, it throws away the log. + * I expect to revisit this in the future. It also makes it easy to manually introduce logs, if needed temporarily. + */ + void log (std::string const &){}; + + /** + * This is the actual handler that is registered with boost asio. + * It calls the caller supplied function (if one is given) and sets the handler to repeat (or not). + */ + static void base_handler (nano::signal_manager::signal_descriptor descriptor, const boost::system::error_code & error, int signum); + + /** boost asio context to use */ + boost::asio::io_context ioc; + + /** work object to make the thread run function live as long as a signal manager */ + boost::asio::executor_work_guard work; + + /** a list of descriptors to hold data contexts needed by the asyncronous handlers */ + std::vector descriptor_list; + + /** thread to service the signal manager io context */ + boost::thread smthread; +}; + +} diff --git a/nano/nano_node/daemon.cpp b/nano/nano_node/daemon.cpp index b2dc93f33..eb58015f8 100644 --- a/nano/nano_node/daemon.cpp +++ b/nano/nano_node/daemon.cpp @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -13,26 +14,19 @@ #include -#include #include namespace { -void my_abort_signal_handler (int signum) -{ - std::signal (signum, SIG_DFL); - nano::dump_crash_stacktrace (); - nano::create_load_memory_address_files (); -} - volatile sig_atomic_t sig_int_or_term = 0; } void nano_daemon::daemon::run (boost::filesystem::path const & data_path, nano::node_flags const & flags) { // Override segmentation fault and aborting. - std::signal (SIGSEGV, &my_abort_signal_handler); - std::signal (SIGABRT, &my_abort_signal_handler); + nano::signal_manager sigman; + sigman.register_signal_handler (SIGSEGV, sigman.get_debug_files_handler (), false); + sigman.register_signal_handler (SIGABRT, sigman.get_debug_files_handler (), false); boost::filesystem::create_directories (data_path); boost::system::error_code error_chmod; @@ -142,8 +136,11 @@ void nano_daemon::daemon::run (boost::filesystem::path const & data_path, nano:: sig_int_or_term = 1; }; - std::signal (SIGINT, &nano::signal_handler); - std::signal (SIGTERM, &nano::signal_handler); + // keep trapping Ctrl-C to avoid a second Ctrl-C interrupting tasks started by the first + sigman.register_signal_handler (SIGINT, &nano::signal_handler, true); + + // sigterm is less likely to come in bunches so only trap it once + sigman.register_signal_handler (SIGTERM, &nano::signal_handler, false); runner = std::make_unique (io_ctx, node->config.io_threads); runner->join (); diff --git a/nano/nano_rpc/entry.cpp b/nano/nano_rpc/entry.cpp index 5597a7196..26f342478 100644 --- a/nano/nano_rpc/entry.cpp +++ b/nano/nano_rpc/entry.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include #include @@ -13,8 +14,6 @@ #include #include -#include - namespace { void logging_init (boost::filesystem::path const & application_path_a) @@ -47,6 +46,7 @@ void run (boost::filesystem::path const & data_path, std::vector co { logging_init (data_path); boost::asio::io_context io_ctx; + nano::signal_manager sigman; try { nano::ipc_rpc_processor ipc_rpc_processor (io_ctx, rpc_config); @@ -59,8 +59,8 @@ void run (boost::filesystem::path const & data_path, std::vector co sig_int_or_term = 1; }; - std::signal (SIGINT, &nano::signal_handler); - std::signal (SIGTERM, &nano::signal_handler); + sigman.register_signal_handler (SIGINT, &nano::signal_handler, true); + sigman.register_signal_handler (SIGTERM, &nano::signal_handler, false); runner = std::make_unique (io_ctx, rpc_config.rpc_process.io_threads); runner->join ();