Prevent double termination signals causing crashes

Summary:
Use sigaction to register signal handlers.
This is preferred over `signal` function, according to `man 3p signal`.
Add global sig_atomic_t flag when shutting down.
Block other signal handlers when shutting down.

Reviewers: mislav.bradac, mferencevic

Reviewed By: mferencevic

Subscribers: pullbot

Differential Revision: https://phabricator.memgraph.io/D943

src/memgraph_bolt.cpp

@@ -1,3 +1,4 @@
+#include <csignal>
 #include <experimental/filesystem>
 #include <iostream>
 
@@ -40,6 +41,13 @@ DEFINE_uint64(memory_warning_threshold, 1024,
               "less available RAM available it will log a warning. Set to 0 to "
               "disable.");
 
+// Needed to correctly handle memgraph destruction from a signal handler.
+// Without having some sort of a flag, it is possible that a signal is handled
+// when we are exiting main, inside destructors of GraphDb and similar. The
+// signal handler may then initiate another shutdown on memgraph which is in
+// half destructed state, causing invalid memory access and crash.
+volatile sig_atomic_t is_shutting_down = 0;
+
 int main(int argc, char **argv) {
   google::SetUsageMessage("Memgraph database server");
   gflags::SetVersionString(version_string);
@@ -56,20 +64,6 @@ int main(int argc, char **argv) {
   // Unhandled exception handler init.
   std::set_terminate(&terminate_handler);
 
-  // Signal handling init.
-  SignalHandler::register_handler(Signal::SegmentationFault, []() {
-    // Log that we got SIGSEGV and abort the program, because returning from
-    // SIGSEGV handler is undefined behaviour.
-    std::cerr << "SegmentationFault signal raised" << std::endl;
-    std::abort();  // This will continue into our SIGABRT handler.
-  });
-  SignalHandler::register_handler(Signal::Abort, []() {
-    // Log the stacktrace and let the abort continue.
-    Stacktrace stacktrace;
-    std::cerr << "Abort signal raised" << std::endl
-              << stacktrace.dump() << std::endl;
-  });
-
   // Initialize bolt session data (GraphDb and Interpreter).
   SessionData session_data;
 
@@ -85,17 +79,30 @@ int main(int argc, char **argv) {
   // Initialize server.
   ServerT server(endpoint, session_data);
 
+  // Handler for regular termination signals
   auto shutdown = [&server, &session_data]() {
+    if (is_shutting_down) return;
+    is_shutting_down = 1;
     // Server needs to be shutdown first and then the database. This prevents a
     // race condition when a transaction is accepted during server shutdown.
     server.Shutdown();
     session_data.db.Shutdown();
   };
-  // register SIGTERM handler
-  SignalHandler::register_handler(Signal::Terminate, shutdown);
 
-  // register SIGINT handler
-  SignalHandler::register_handler(Signal::Interupt, shutdown);
+  // Prevent handling shutdown inside a shutdown. For example, SIGINT handler
+  // being interrupted by SIGTERM before is_shutting_down is set, thus causing
+  // double shutdown.
+  sigset_t block_shutdown_signals;
+  sigemptyset(&block_shutdown_signals);
+  sigaddset(&block_shutdown_signals, SIGTERM);
+  sigaddset(&block_shutdown_signals, SIGINT);
+
+  CHECK(SignalHandler::RegisterHandler(Signal::Terminate, shutdown,
+                                       block_shutdown_signals))
+      << "Unable to register SIGTERM handler!";
+  CHECK(SignalHandler::RegisterHandler(Signal::Interupt, shutdown,
+                                       block_shutdown_signals))
+      << "Unable to register SIGINT handler!";
 
   // Start memory warning logger.
   Scheduler mem_log_scheduler;

src/utils/signals/handler.hpp

@@ -23,22 +23,30 @@ class SignalHandler {
  private:
   static std::map<int, std::function<void()>> handlers_;
 
-  static void handle(int signal) { handlers_[signal](); }
+  static void Handle(int signal) { handlers_[signal](); }
 
  public:
-  static void register_handler(Signal signal, Function func) {
-    int signal_number = static_cast<int>(signal);
-    handlers_[signal_number] = func;
-    std::signal(signal_number, SignalHandler::handle);
+  /// Install a signal handler.
+  static bool RegisterHandler(Signal signal, Function func) {
+    sigset_t signal_mask;
+    sigemptyset(&signal_mask);
+    return RegisterHandler(signal, func, signal_mask);
   }
 
-  // TODO possible changes if signelton needed later
-  /*
-    static SignalHandler& instance() {
-      static SignalHandler instance;
-      return instance;
-    }
-  */
+  /// Like RegisterHandler, but takes a `signal_mask` argument for blocking
+  /// signals during execution of the handler. `signal_mask` should be created
+  /// using `sigemptyset` and `sigaddset` functions from `<signal.h>`.
+  static bool RegisterHandler(Signal signal, Function func,
+                              sigset_t signal_mask) {
+    int signal_number = static_cast<int>(signal);
+    handlers_[signal_number] = func;
+    struct sigaction action;
+    action.sa_handler = SignalHandler::Handle;
+    action.sa_mask = signal_mask;
+    action.sa_flags = SA_RESTART;
+    if (sigaction(signal_number, &action, NULL) == -1) return false;
+    return true;
+  }
 };
 
 std::map<int, std::function<void()>> SignalHandler::handlers_ = {};

tests/manual/antlr_sigsegv.cpp

@@ -89,13 +89,13 @@ int main(int argc, char **argv) {
   ::testing::InitGoogleTest(&argc, argv);
 
   // Signal handling init.
-  SignalHandler::register_handler(Signal::SegmentationFault, []() {
+  SignalHandler::RegisterHandler(Signal::SegmentationFault, []() {
     // Log that we got SIGSEGV and abort the program, because returning from
     // SIGSEGV handler is undefined behaviour.
     std::cerr << "SegmentationFault signal raised" << std::endl;
     std::abort();  // This will continue into our SIGABRT handler.
   });
-  SignalHandler::register_handler(Signal::Abort, []() {
+  SignalHandler::RegisterHandler(Signal::Abort, []() {
     // Log the stacktrace and let the abort continue.
     Stacktrace stacktrace;
     std::cerr << "Abort signal raised" << std::endl

tests/unit/signal_handler.cpp

@@ -8,7 +8,7 @@
 #include "utils/stacktrace.hpp"
 
 TEST(SignalHandler, SegmentationFaultTest) {
-  SignalHandler::register_handler(Signal::SegmentationFault, []() {
+  SignalHandler::RegisterHandler(Signal::SegmentationFault, []() {
     std::cout << "Segmentation Fault" << std::endl;
     Stacktrace stacktrace;
     std::cout << stacktrace.dump() << std::endl;