From 6063f5f011ecdaeb50384b4d728708a72bfc8e00 Mon Sep 17 00:00:00 2001 From: dsiganos Date: Wed, 13 Oct 2021 14:33:33 +0100 Subject: [PATCH] Fix handling of SIGSEGV and SIGABRT in nano_node (#3502) The handling of SIGSEGV and SIGABRT did not have the desired effect of generating stack traces of the source code that caused the signal. That's because the signals were handled by the signal manager, which first switches stack context to one that is safe to execute any code. However, for SIGSEGV and SIGABRT we want to print the stacktrace of the problem and therefore we cannot switch context. So, I am setting the handling of SIGSEGV and SIGABRT to use the classic signal function, like before the signal manager was introduced. --- nano/lib/signal_manager.cpp | 8 -------- nano/lib/signal_manager.hpp | 3 --- nano/nano_node/daemon.cpp | 24 ++++++++++++++++++++---- 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/nano/lib/signal_manager.cpp b/nano/lib/signal_manager.cpp index ffaae2d4..5b1fab24 100644 --- a/nano/lib/signal_manager.cpp +++ b/nano/lib/signal_manager.cpp @@ -49,14 +49,6 @@ void nano::signal_manager::register_signal_handler (int 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) diff --git a/nano/lib/signal_manager.hpp b/nano/lib/signal_manager.hpp index b667ae56..d3e8de9d 100644 --- a/nano/lib/signal_manager.hpp +++ b/nano/lib/signal_manager.hpp @@ -33,9 +33,6 @@ public: */ 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 { diff --git a/nano/nano_node/daemon.cpp b/nano/nano_node/daemon.cpp index 8b5926ec..11eb7a2a 100644 --- a/nano/nano_node/daemon.cpp +++ b/nano/nano_node/daemon.cpp @@ -17,6 +17,16 @@ namespace { +void nano_abort_signal_handler (int signum) +{ + // create some debugging log files + nano::dump_crash_stacktrace (); + nano::create_load_memory_address_files (); + + // re-raise signal to call the default handler and exit + raise (signum); +} + volatile sig_atomic_t sig_int_or_term = 0; constexpr std::size_t OPEN_FILE_DESCRIPTORS_LIMIT = 16384; @@ -39,10 +49,14 @@ static void load_and_set_bandwidth_params (std::shared_ptr const & n void nano_daemon::daemon::run (boost::filesystem::path const & data_path, nano::node_flags const & flags) { - // Override segmentation fault and aborting. - 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); + // We catch signal SIGSEGV and SIGABRT not via the signal manager because we want these signal handlers + // to be executed in the stack of the code that caused the signal, so we can dump the stacktrace. + struct sigaction sa = {}; + sa.sa_handler = nano_abort_signal_handler; + sigemptyset (&sa.sa_mask); + sa.sa_flags = SA_RESETHAND; + sigaction (SIGSEGV, &sa, NULL); + sigaction (SIGABRT, &sa, NULL); boost::filesystem::create_directories (data_path); boost::system::error_code error_chmod; @@ -155,6 +169,8 @@ void nano_daemon::daemon::run (boost::filesystem::path const & data_path, nano:: sig_int_or_term = 1; }; + nano::signal_manager sigman; + // 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);