From 71a265f50e373c0216ab455d460ab35ca04e81fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Sat, 13 Apr 2024 20:51:32 +0200 Subject: [PATCH 1/3] Signal handlers cleanup --- nano/load_test/entry.cpp | 10 ++++++---- nano/nano_node/daemon.cpp | 7 +++---- nano/nano_rpc/entry.cpp | 7 +++---- nano/secure/utility.cpp | 13 ------------- nano/secure/utility.hpp | 3 --- 5 files changed, 12 insertions(+), 28 deletions(-) diff --git a/nano/load_test/entry.cpp b/nano/load_test/entry.cpp index c47989fd63..a70ccab4d1 100644 --- a/nano/load_test/entry.cpp +++ b/nano/load_test/entry.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -595,13 +596,14 @@ int main (int argc, char * const * argv) std::shared_ptr ioc_shared = std::make_shared (); boost::asio::io_context & ioc{ *ioc_shared }; - debug_assert (!nano::signal_handler_impl); - nano::signal_handler_impl = [&ioc] () { + nano::signal_manager sigman; + + auto signal_handler = [&ioc] (int signum) { ioc.stop (); }; - std::signal (SIGINT, &nano::signal_handler); - std::signal (SIGTERM, &nano::signal_handler); + sigman.register_signal_handler (SIGINT, signal_handler, true); + sigman.register_signal_handler (SIGTERM, signal_handler, false); tcp::resolver resolver{ ioc }; auto const primary_node_results = resolver.resolve ("::1", std::to_string (rpc_port_start)); diff --git a/nano/nano_node/daemon.cpp b/nano/nano_node/daemon.cpp index 0ed9ee7053..9d260e4369 100644 --- a/nano/nano_node/daemon.cpp +++ b/nano/nano_node/daemon.cpp @@ -193,8 +193,7 @@ void nano::daemon::run (std::filesystem::path const & data_path, nano::node_flag } } - debug_assert (!nano::signal_handler_impl); - nano::signal_handler_impl = [this, io_ctx_w = std::weak_ptr{ io_ctx }] () { + auto signal_handler = [this, io_ctx_w = std::weak_ptr{ io_ctx }] (int signum) { logger.warn (nano::log::type::daemon, "Interrupt signal received, stopping..."); if (auto io_ctx_l = io_ctx_w.lock ()) @@ -207,10 +206,10 @@ void nano::daemon::run (std::filesystem::path const & data_path, nano::node_flag 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); + sigman.register_signal_handler (SIGINT, 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); + sigman.register_signal_handler (SIGTERM, 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 b198d20820..30dedb3492 100644 --- a/nano/nano_rpc/entry.cpp +++ b/nano/nano_rpc/entry.cpp @@ -58,8 +58,7 @@ void run (std::filesystem::path const & data_path, std::vector cons auto rpc = nano::get_rpc (io_ctx, rpc_config, ipc_rpc_processor); rpc->start (); - debug_assert (!nano::signal_handler_impl); - nano::signal_handler_impl = [io_ctx_w = std::weak_ptr{ io_ctx }] () { + auto signal_handler = [io_ctx_w = std::weak_ptr{ io_ctx }] (int signum) { logger.warn (nano::log::type::daemon, "Interrupt signal received, stopping..."); if (auto io_ctx_l = io_ctx_w.lock ()) @@ -69,8 +68,8 @@ void run (std::filesystem::path const & data_path, std::vector cons sig_int_or_term = 1; }; - sigman.register_signal_handler (SIGINT, &nano::signal_handler, true); - sigman.register_signal_handler (SIGTERM, &nano::signal_handler, false); + sigman.register_signal_handler (SIGINT, signal_handler, true); + sigman.register_signal_handler (SIGTERM, signal_handler, false); runner = std::make_unique (io_ctx, rpc_config.rpc_process.io_threads); runner->join (); diff --git a/nano/secure/utility.cpp b/nano/secure/utility.cpp index 07d6472462..bec147eae1 100644 --- a/nano/secure/utility.cpp +++ b/nano/secure/utility.cpp @@ -74,16 +74,3 @@ void nano::remove_temporary_directories () } } } - -namespace nano -{ -/** A wrapper for handling signals */ -std::function signal_handler_impl; -void signal_handler (int sig) -{ - if (signal_handler_impl != nullptr) - { - signal_handler_impl (); - } -} -} diff --git a/nano/secure/utility.hpp b/nano/secure/utility.hpp index 96f93d6edd..8cd446c90a 100644 --- a/nano/secure/utility.hpp +++ b/nano/secure/utility.hpp @@ -13,7 +13,4 @@ std::filesystem::path working_path (nano::networks network = nano::network_const std::filesystem::path unique_path (nano::networks network = nano::network_constants::active_network); // Remove all unique tmp directories created by the process void remove_temporary_directories (); -// Generic signal handler declarations -extern std::function signal_handler_impl; -void signal_handler (int sig); } From 602ecd43f023bac8e1c68853afe36fa728a50d34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Sun, 14 Apr 2024 18:44:29 +0200 Subject: [PATCH 2/3] Signal manager logging --- nano/lib/logging_enums.hpp | 1 + nano/lib/signal_manager.cpp | 17 +++++++++-------- nano/lib/signal_manager.hpp | 9 +++------ 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/nano/lib/logging_enums.hpp b/nano/lib/logging_enums.hpp index 79f4df8dec..7392320bc8 100644 --- a/nano/lib/logging_enums.hpp +++ b/nano/lib/logging_enums.hpp @@ -75,6 +75,7 @@ enum class type rep_tiers, syn_cookies, thread_runner, + signal_manager, // bootstrap bulk_pull_client, diff --git a/nano/lib/signal_manager.cpp b/nano/lib/signal_manager.cpp index 365b37142c..83e48a9224 100644 --- a/nano/lib/signal_manager.cpp +++ b/nano/lib/signal_manager.cpp @@ -46,14 +46,16 @@ void nano::signal_manager::register_signal_handler (int signum, std::functionclear (); } - - 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 ())); + logger.debug (nano::log::type::signal_manager, "Signal error: {} ({})", ec.message (), to_signal_name (signum)); } } diff --git a/nano/lib/signal_manager.hpp b/nano/lib/signal_manager.hpp index 5012de52fb..a62f619af0 100644 --- a/nano/lib/signal_manager.hpp +++ b/nano/lib/signal_manager.hpp @@ -1,5 +1,6 @@ #pragma once +#include #include #include @@ -51,18 +52,14 @@ class signal_manager final 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, boost::system::error_code const & error, int signum); + nano::logger logger; + /** boost asio context to use */ boost::asio::io_context ioc; From 29e7218959b8ea8166a44b305ae30597b6c3426b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Sun, 14 Apr 2024 18:24:29 +0200 Subject: [PATCH 3/3] Daemon latch --- nano/lib/signal_manager.cpp | 18 ++++++++++ nano/lib/signal_manager.hpp | 1 + nano/nano_node/daemon.cpp | 68 ++++++++++++++++++------------------- nano/nano_rpc/entry.cpp | 39 ++++++++++----------- 4 files changed, 72 insertions(+), 54 deletions(-) diff --git a/nano/lib/signal_manager.cpp b/nano/lib/signal_manager.cpp index 83e48a9224..74f466e788 100644 --- a/nano/lib/signal_manager.cpp +++ b/nano/lib/signal_manager.cpp @@ -82,3 +82,21 @@ void nano::signal_manager::base_handler (nano::signal_manager::signal_descriptor logger.debug (nano::log::type::signal_manager, "Signal error: {} ({})", ec.message (), to_signal_name (signum)); } } + +std::string nano::to_signal_name (int signum) +{ + switch (signum) + { + case SIGINT: + return "SIGINT"; + case SIGTERM: + return "SIGTERM"; + case SIGSEGV: + return "SIGSEGV"; + case SIGABRT: + return "SIGABRT"; + case SIGILL: + return "SIGILL"; + } + return std::to_string (signum); +} diff --git a/nano/lib/signal_manager.hpp b/nano/lib/signal_manager.hpp index a62f619af0..9c0fb81e36 100644 --- a/nano/lib/signal_manager.hpp +++ b/nano/lib/signal_manager.hpp @@ -73,4 +73,5 @@ class signal_manager final boost::thread smthread; }; +std::string to_signal_name (int signum); } diff --git a/nano/nano_node/daemon.cpp b/nano/nano_node/daemon.cpp index 9d260e4369..da66fc8e4f 100644 --- a/nano/nano_node/daemon.cpp +++ b/nano/nano_node/daemon.cpp @@ -14,11 +14,11 @@ #include #include -#include #include #include #include +#include #include @@ -56,8 +56,6 @@ void install_abort_signal_handler () #endif } -volatile sig_atomic_t sig_int_or_term = 0; - constexpr std::size_t OPEN_FILE_DESCRIPTORS_LIMIT = 16384; } @@ -146,16 +144,28 @@ void nano::daemon::run (std::filesystem::path const & data_path, nano::node_flag logger.info (nano::log::type::daemon, "Database backend: {}", node->store.vendor_get ()); logger.info (nano::log::type::daemon, "Start time: {:%c} UTC", fmt::gmtime (dateTime)); + // IO context runner should be started first and stopped last to allow asio handlers to execute during node start/stop + runner = std::make_unique (io_ctx, node->config.io_threads); + node->start (); - nano::ipc::ipc_server ipc_server (*node, config.rpc); + std::atomic stopped{ false }; + + std::unique_ptr ipc_server = std::make_unique (*node, config.rpc); std::unique_ptr rpc_process; - std::shared_ptr rpc; std::unique_ptr rpc_handler; + std::shared_ptr rpc; + if (config.rpc_enable) { if (!config.rpc.child_process.enable) { + auto stop_callback = [this, &stopped] () { + logger.warn (nano::log::type::daemon, "RPC stop request received, stopping..."); + stopped = true; + stopped.notify_all (); + }; + // Launch rpc in-process nano::rpc_config rpc_config{ config.node.network_params.network }; auto error = nano::read_rpc_config_toml (data_path, rpc_config, flags.rpc_config_overrides); @@ -166,16 +176,7 @@ void nano::daemon::run (std::filesystem::path const & data_path, nano::node_flag } rpc_config.tls_config = tls_config; - rpc_handler = std::make_unique (*node, ipc_server, config.rpc, - [&ipc_server, &workers = node->workers, io_ctx_w = std::weak_ptr{ io_ctx }] () { - ipc_server.stop (); - workers.add_timed_task (std::chrono::steady_clock::now () + std::chrono::seconds (3), [io_ctx_w] () { - if (auto io_ctx_l = io_ctx_w.lock ()) - { - io_ctx_l->stop (); - } - }); - }); + rpc_handler = std::make_unique (*node, *ipc_server, config.rpc, stop_callback); rpc = nano::get_rpc (io_ctx, rpc_config, *rpc_handler); rpc->start (); } @@ -191,38 +192,35 @@ void nano::daemon::run (std::filesystem::path const & data_path, nano::node_flag rpc_process = std::make_unique (config.rpc.child_process.rpc_path, "--daemon", "--data_path", data_path.string (), "--network", network); } + debug_assert (rpc || rpc_process); } - auto signal_handler = [this, io_ctx_w = std::weak_ptr{ io_ctx }] (int signum) { - logger.warn (nano::log::type::daemon, "Interrupt signal received, stopping..."); - - if (auto io_ctx_l = io_ctx_w.lock ()) - { - io_ctx_l->stop (); - } - sig_int_or_term = 1; + auto signal_handler = [this, &stopped] (int signum) { + logger.warn (nano::log::type::daemon, "Interrupt signal received ({}), stopping...", to_signal_name (signum)); + stopped = true; + stopped.notify_all (); }; 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, signal_handler, true); - // sigterm is less likely to come in bunches so only trap it once sigman.register_signal_handler (SIGTERM, signal_handler, false); - runner = std::make_unique (io_ctx, node->config.io_threads); - runner->join (); + // Keep running until stopped flag is set + stopped.wait (false); - if (sig_int_or_term == 1) + logger.info (nano::log::type::daemon, "Stopping..."); + + if (rpc) { - ipc_server.stop (); - node->stop (); - if (rpc) - { - rpc->stop (); - } + rpc->stop (); } + ipc_server->stop (); + node->stop (); + io_ctx->stop (); + runner->join (); + if (rpc_process) { rpc_process->wait (); @@ -243,5 +241,5 @@ void nano::daemon::run (std::filesystem::path const & data_path, nano::node_flag logger.critical (nano::log::type::daemon, "Error deserializing config: {}", error.get_message ()); } - logger.info (nano::log::type::daemon, "Daemon exiting"); + logger.info (nano::log::type::daemon, "Daemon stopped"); } diff --git a/nano/nano_rpc/entry.cpp b/nano/nano_rpc/entry.cpp index 30dedb3492..6f1c6080b2 100644 --- a/nano/nano_rpc/entry.cpp +++ b/nano/nano_rpc/entry.cpp @@ -15,10 +15,10 @@ #include #include +#include + namespace { -volatile sig_atomic_t sig_int_or_term = 0; - nano::logger logger{ "rpc_daemon" }; void run (std::filesystem::path const & data_path, std::vector const & config_overrides) @@ -41,7 +41,7 @@ void run (std::filesystem::path const & data_path, std::vector cons error = nano::read_tls_config_toml (data_path, *tls_config, logger); if (error) { - logger.critical (nano::log::type::daemon, "Error reading RPC TLS config: {}", error.get_message ()); + logger.critical (nano::log::type::daemon_rpc, "Error reading RPC TLS config: {}", error.get_message ()); std::exit (1); } else @@ -51,42 +51,43 @@ void run (std::filesystem::path const & data_path, std::vector cons std::shared_ptr io_ctx = std::make_shared (); - nano::signal_manager sigman; + runner = std::make_unique (io_ctx, rpc_config.rpc_process.io_threads); + try { nano::ipc_rpc_processor ipc_rpc_processor (*io_ctx, rpc_config); auto rpc = nano::get_rpc (io_ctx, rpc_config, ipc_rpc_processor); rpc->start (); - auto signal_handler = [io_ctx_w = std::weak_ptr{ io_ctx }] (int signum) { - logger.warn (nano::log::type::daemon, "Interrupt signal received, stopping..."); + std::atomic stopped{ false }; - if (auto io_ctx_l = io_ctx_w.lock ()) - { - io_ctx_l->stop (); - } - sig_int_or_term = 1; + auto signal_handler = [&stopped] (int signum) { + logger.warn (nano::log::type::daemon_rpc, "Interrupt signal received ({}), stopping...", nano::to_signal_name (signum)); + stopped = true; + stopped.notify_all (); }; + nano::signal_manager sigman; sigman.register_signal_handler (SIGINT, signal_handler, true); sigman.register_signal_handler (SIGTERM, signal_handler, false); - runner = std::make_unique (io_ctx, rpc_config.rpc_process.io_threads); - runner->join (); + // Keep running until stopped flag is set + stopped.wait (false); + + logger.info (nano::log::type::daemon_rpc, "Stopping..."); - if (sig_int_or_term == 1) - { - rpc->stop (); - } + rpc->stop (); + io_ctx->stop (); + runner->join (); } catch (std::runtime_error const & e) { - logger.critical (nano::log::type::daemon, "Error while running RPC: {}", e.what ()); + logger.critical (nano::log::type::daemon_rpc, "Error while running RPC: {}", e.what ()); } } else { - logger.critical (nano::log::type::daemon, "Error deserializing config: {}", error.get_message ()); + logger.critical (nano::log::type::daemon_rpc, "Error deserializing config: {}", error.get_message ()); } logger.info (nano::log::type::daemon_rpc, "Daemon stopped (RPC)");