From f8a4fae080335d70862ce12193cc24021ac7f73f Mon Sep 17 00:00:00 2001
From: Matej Ferencevic <matej.ferencevic@memgraph.io>
Date: Fri, 30 Mar 2018 11:07:37 +0200
Subject: [PATCH] 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                         | 12 ++++----
 .../{signals/handler.hpp => signals.hpp}      | 22 ++++++++++++++
 tests/distributed/raft/example_client.cpp     |  4 ---
 tests/distributed/raft/example_server.cpp     | 10 +++----
 tests/manual/antlr_sigsegv.cpp                |  6 ++--
 tests/unit/signal_handler.cpp                 | 23 ---------------
 tests/unit/signals.cpp                        | 29 +++++++++++++++++++
 7 files changed, 65 insertions(+), 41 deletions(-)
 rename src/utils/{signals/handler.hpp => signals.hpp} (69%)
 delete mode 100644 tests/unit/signal_handler.cpp
 create mode 100644 tests/unit/signals.cpp

diff --git a/src/memgraph_bolt.cpp b/src/memgraph_bolt.cpp
index b36ec03ad..dc4c36ef4 100644
--- a/src/memgraph_bolt.cpp
+++ b/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!";
 }
diff --git a/src/utils/signals/handler.hpp b/src/utils/signals.hpp
similarity index 69%
rename from src/utils/signals/handler.hpp
rename to src/utils/signals.hpp
index d75881a7a..4b78bbd55 100644
--- a/src/utils/signals/handler.hpp
+++ b/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
diff --git a/tests/distributed/raft/example_client.cpp b/tests/distributed/raft/example_client.cpp
index 5b3389a4d..fed7620bb 100644
--- a/tests/distributed/raft/example_client.cpp
+++ b/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");
 
diff --git a/tests/distributed/raft/example_server.cpp b/tests/distributed/raft/example_server.cpp
index e7042d783..656ee4f52 100644
--- a/tests/distributed/raft/example_server.cpp
+++ b/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.
diff --git a/tests/manual/antlr_sigsegv.cpp b/tests/manual/antlr_sigsegv.cpp
index 19e98d7c9..637ea03eb 100644
--- a/tests/manual/antlr_sigsegv.cpp
+++ b/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
diff --git a/tests/unit/signal_handler.cpp b/tests/unit/signal_handler.cpp
deleted file mode 100644
index 96ab3b231..000000000
--- a/tests/unit/signal_handler.cpp
+++ /dev/null
@@ -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();
-}
diff --git a/tests/unit/signals.cpp b/tests/unit/signals.cpp
new file mode 100644
index 000000000..2e670e3c0
--- /dev/null
+++ b/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();
+}