Handle signals with boost asio signal handler

Currently the nano_node has 2 problems:
* it uses the function signal which is not very portable
* it calls possibly blocking code from signal handler context

These problems will become even more crucial when we start using SIGHUP to
reload config. In addition, we plan to introduce more processes and it
makes sense to have a common signal framework for all the processes.

This commit introduces the signal manager, which fixes all the problems
listed above.

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

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 <nano/lib/signal_manager.hpp>
+
+#include <gtest/gtest.h>
+
+#include <boost/format.hpp>
+
+#include <csignal>
+#include <iostream>
+
+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<void (int)> 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<void (int)> 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$");
+}

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

nano/lib/signal_manager.cpp

@@ -0,0 +1,93 @@
+#include <nano/lib/signal_manager.hpp>
+#include <nano/lib/utility.hpp>
+
+#include <boost/asio.hpp>
+#include <boost/asio/signal_set.hpp>
+#include <boost/bind.hpp>
+#include <boost/format.hpp>
+
+#include <iostream>
+
+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<boost::asio::signal_set> sigset_a, signal_manager & sigman_a, std::function<void (int)> 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<void (int)> handler, bool repeat)
+{
+	// create a signal set to hold the mapping between signals and signal handlers
+	auto sigset = std::make_shared<boost::asio::signal_set> (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<void (int)> 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 ()));
+	}
+}

nano/lib/signal_manager.hpp

@@ -0,0 +1,82 @@
+#pragma once
+
+#include <nano/lib/utility.hpp>
+
+#include <boost/asio.hpp>
+#include <boost/system/error_code.hpp>
+#include <boost/thread.hpp>
+
+#include <iostream>
+#include <memory>
+#include <string>
+#include <vector>
+
+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<void (int)> handler, bool repeat);
+
+	/** returns a signal handler that prints a stacktrace and creates some debug files */
+	std::function<void (int)> get_debug_files_handler (void);
+
+private:
+	struct signal_descriptor final
+	{
+		signal_descriptor (std::shared_ptr<boost::asio::signal_set> sigset_a, signal_manager & sigman_a, std::function<void (int)> 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<boost::asio::signal_set> 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<void (int)> 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<boost::asio::io_context::executor_type> work;
+
+	/** a list of descriptors to hold data contexts needed by the asyncronous handlers */
+	std::vector<signal_descriptor> descriptor_list;
+
+	/** thread to service the signal manager io context */
+	boost::thread smthread;
+};
+
+}

nano/nano_node/daemon.cpp

@@ -1,4 +1,5 @@
 #include <nano/boost/process/child.hpp>
+#include <nano/lib/signal_manager.hpp>
 #include <nano/lib/threading.hpp>
 #include <nano/lib/utility.hpp>
 #include <nano/nano_node/daemon.hpp>
@@ -13,26 +14,19 @@
 
 #include <boost/format.hpp>
 
-#include <csignal>
 #include <iostream>
 
 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<nano::thread_runner> (io_ctx, node->config.io_threads);
 				runner->join ();

nano/nano_rpc/entry.cpp

@@ -1,5 +1,6 @@
 #include <nano/lib/cli.hpp>
 #include <nano/lib/errors.hpp>
+#include <nano/lib/signal_manager.hpp>
 #include <nano/lib/threading.hpp>
 #include <nano/lib/utility.hpp>
 #include <nano/node/cli.hpp>
@@ -13,8 +14,6 @@
 #include <boost/log/utility/setup/file.hpp>
 #include <boost/program_options.hpp>
 
-#include <csignal>
-
 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<std::string> 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<std::string> 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<nano::thread_runner> (io_ctx, rpc_config.rpc_process.io_threads);
 			runner->join ();