Add ignore signal handler and cleanup

Reviewers: teon.banek, buda

Reviewed By: teon.banek

Subscribers: pullbot

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

src/memgraph_bolt.cpp

@@ -16,7 +16,7 @@
 #include "utils/flag_validation.hpp"
 #include "utils/on_scope_exit.hpp"
 #include "utils/scheduler.hpp"
-#include "utils/signals/handler.hpp"
+#include "utils/signals.hpp"
 #include "utils/stacktrace.hpp"
 #include "utils/sysinfo/memory.hpp"
 #include "utils/terminate_handler.hpp"
@@ -76,16 +76,16 @@ void InitSignalHandlers(const std::function<void()> &shutdown) {
   sigaddset(&block_shutdown_signals, SIGTERM);
   sigaddset(&block_shutdown_signals, SIGINT);
 
-  CHECK(SignalHandler::RegisterHandler(Signal::Terminate, shutdown,
-                                       block_shutdown_signals))
+  CHECK(utils::SignalHandler::RegisterHandler(utils::Signal::Terminate,
+                                              shutdown, block_shutdown_signals))
       << "Unable to register SIGTERM handler!";
-  CHECK(SignalHandler::RegisterHandler(Signal::Interupt, shutdown,
-                                       block_shutdown_signals))
+  CHECK(utils::SignalHandler::RegisterHandler(utils::Signal::Interupt, shutdown,
+                                              block_shutdown_signals))
       << "Unable to register SIGINT handler!";
 
   // Setup SIGUSR1 to be used for reopening log files, when e.g. logrotate
   // rotates our logs.
-  CHECK(SignalHandler::RegisterHandler(Signal::User1, []() {
+  CHECK(utils::SignalHandler::RegisterHandler(utils::Signal::User1, []() {
     google::CloseLogDestination(google::INFO);
   })) << "Unable to register SIGUSR1 handler!";
 }

src/utils/signals.hpp

@@ -1,3 +1,5 @@
+#pragma once
+
 #include <csignal>
 #include <functional>
 #include <iostream>
@@ -6,6 +8,8 @@
 #include <utility>
 #include <vector>
 
+namespace utils {
+
 // TODO: align bits so signals can be combined
 //       Signal::Terminate | Signal::Interupt
 enum class Signal : int {
@@ -14,10 +18,24 @@ enum class Signal : int {
   Interupt = SIGINT,
   Quit = SIGQUIT,
   Abort = SIGABRT,
+  Pipe = SIGPIPE,
   BusError = SIGBUS,
   User1 = SIGUSR1,
 };
 
+bool SignalIgnore(const Signal signal) {
+  int signal_number = static_cast<int>(signal);
+  struct sigaction action;
+  // `sa_sigaction` must be cleared before `sa_handler` is set because on some
+  // platforms the two are a union.
+  action.sa_sigaction = nullptr;
+  action.sa_handler = SIG_IGN;
+  sigemptyset(&action.sa_mask);
+  action.sa_flags = 0;
+  if (sigaction(signal_number, &action, NULL) == -1) return false;
+  return true;
+}
+
 class SignalHandler {
  private:
   static std::map<int, std::function<void()>> handlers_;
@@ -40,6 +58,9 @@ class SignalHandler {
     int signal_number = static_cast<int>(signal);
     handlers_[signal_number] = func;
     struct sigaction action;
+    // `sa_sigaction` must be cleared before `sa_handler` is set because on some
+    // platforms the two are a union.
+    action.sa_sigaction = nullptr;
     action.sa_handler = SignalHandler::Handle;
     action.sa_mask = signal_mask;
     action.sa_flags = SA_RESTART;
@@ -49,3 +70,4 @@ class SignalHandler {
 };
 
 std::map<int, std::function<void()>> SignalHandler::handlers_ = {};
+}  // namespace utils

tests/distributed/raft/example_client.cpp

@@ -10,8 +10,6 @@
 #include "io/network/endpoint.hpp"
 #include "messages.hpp"
 #include "utils/network.hpp"
-#include "utils/signals/handler.hpp"
-#include "utils/terminate_handler.hpp"
 
 using namespace communication::rpc;
 using namespace std::literals::chrono_literals;
@@ -20,8 +18,6 @@ DEFINE_string(server_interface, "127.0.0.1",
               "Server interface on which to communicate.");
 DEFINE_int32(server_port, 8010, "Server port on which to communicate.");
 
-volatile sig_atomic_t is_shutting_down = 0;
-
 int main(int argc, char **argv) {
   google::SetUsageMessage("Raft RPC Client");
 

tests/distributed/raft/example_server.cpp

@@ -7,7 +7,7 @@
 
 #include "communication/rpc/server.hpp"
 #include "messages.hpp"
-#include "utils/signals/handler.hpp"
+#include "utils/signals.hpp"
 #include "utils/terminate_handler.hpp"
 
 using namespace communication::rpc;
@@ -48,11 +48,11 @@ int main(int argc, char **argv) {
   sigaddset(&block_shutdown_signals, SIGTERM);
   sigaddset(&block_shutdown_signals, SIGINT);
 
-  CHECK(SignalHandler::RegisterHandler(Signal::Terminate, shutdown,
-                                       block_shutdown_signals))
+  CHECK(utils::SignalHandler::RegisterHandler(utils::Signal::Terminate,
+                                              shutdown, block_shutdown_signals))
       << "Unable to register SIGTERM handler!";
-  CHECK(SignalHandler::RegisterHandler(Signal::Interupt, shutdown,
-                                       block_shutdown_signals))
+  CHECK(utils::SignalHandler::RegisterHandler(utils::Signal::Interupt, shutdown,
+                                              block_shutdown_signals))
       << "Unable to register SIGINT handler!";
 
   // Example callback.

tests/manual/antlr_sigsegv.cpp

@@ -7,7 +7,7 @@
 #include <gtest/gtest.h>
 
 #include "query/frontend/opencypher/parser.hpp"
-#include "utils/signals/handler.hpp"
+#include "utils/signals.hpp"
 #include "utils/stacktrace.hpp"
 
 using namespace std::chrono_literals;
@@ -89,13 +89,13 @@ int main(int argc, char **argv) {
   ::testing::InitGoogleTest(&argc, argv);
 
   // Signal handling init.
-  SignalHandler::RegisterHandler(Signal::SegmentationFault, []() {
+  utils::SignalHandler::RegisterHandler(utils::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::RegisterHandler(Signal::Abort, []() {
+  utils::SignalHandler::RegisterHandler(utils::Signal::Abort, []() {
     // Log the stacktrace and let the abort continue.
     Stacktrace stacktrace;
     std::cerr << "Abort signal raised" << std::endl

tests/unit/signal_handler.cpp

@@ -1,23 +0,0 @@
-#include "gtest/gtest.h"
-
-#include <iostream>
-#include <string>
-#include <utility>
-
-#include "utils/signals/handler.hpp"
-#include "utils/stacktrace.hpp"
-
-TEST(SignalHandler, SegmentationFaultTest) {
-  SignalHandler::RegisterHandler(Signal::SegmentationFault, []() {
-    std::cout << "Segmentation Fault" << std::endl;
-    Stacktrace stacktrace;
-    std::cout << stacktrace.dump() << std::endl;
-  });
-
-  std::raise(SIGSEGV);
-}
-
-int main(int argc, char **argv) {
-  ::testing::InitGoogleTest(&argc, argv);
-  return RUN_ALL_TESTS();
-}

tests/unit/signals.cpp

@@ -0,0 +1,29 @@
+#include "gtest/gtest.h"
+
+#include <iostream>
+#include <string>
+#include <utility>
+
+#include "utils/signals.hpp"
+#include "utils/stacktrace.hpp"
+
+TEST(Signals, Handler) {
+  ASSERT_TRUE(utils::SignalHandler::RegisterHandler(
+      utils::Signal::SegmentationFault, []() {
+        std::cout << "Segmentation Fault" << std::endl;
+        Stacktrace stacktrace;
+        std::cout << stacktrace.dump() << std::endl;
+      }));
+
+  std::raise(SIGSEGV);
+}
+
+TEST(Signals, Ignore) {
+  ASSERT_TRUE(utils::SignalIgnore(utils::Signal::Pipe));
+  std::raise(SIGPIPE);
+}
+
+int main(int argc, char **argv) {
+  ::testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}