fix: Fix rare crash in signal handler

If the process is signaled after SignalHandler::~SignalHandler has run
then this assert will trigger:

    ccache: SignalHandler.cpp:87: static void SignalHandler::on_signal(int): failed assertion: g_the_signal_handler

Fix this by deregistering the signal handler function before destructing
the SignalHandler object.

Fixes #1246.

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 79f0fd1c3eae5c6beef1769a6ebc34957766d180..55cfaa4113cfffdcb03138789d2d61d031ab660d 100644 (file)
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -9,7 +9,6 @@ set(
   Hash.cpp
   Logging.cpp
   ProgressBar.cpp
-  SignalHandler.cpp
   Stat.cpp
   TemporaryFile.cpp
   ThreadPool.cpp
@@ -34,6 +33,8 @@ endif()
 
 if(WIN32)
   list(APPEND source_files Win32Util.cpp)
+else()
+  list(APPEND source_files SignalHandler.cpp)
 endif()
 
 file(GLOB headers *.hpp)

diff --git a/src/SignalHandler.cpp b/src/SignalHandler.cpp
index 2d32ac6d038a929226d0a157179191e1c39fcd84..acfa2e3f65f0e87e38349509c8e2ca783d9a2549 100644 (file)
--- a/src/SignalHandler.cpp
+++ b/src/SignalHandler.cpp
@@ -1,4 +1,4 @@
-// Copyright (C) 2020-2021 Joel Rosdahl and other contributors
+// Copyright (C) 2020-2023 Joel Rosdahl and other contributors
 //
 // See doc/AUTHORS.adoc for a complete list of contributors.
 //
@@ -18,31 +18,47 @@
 
 #include "SignalHandler.hpp"
 
-#ifndef _WIN32
+#include "Context.hpp"
+#include "assertions.hpp"
 
-#  include "Context.hpp"
-#  include "assertions.hpp"
-
-#  include <signal.h> // NOLINT: sigaddset et al are defined in signal.h
-#  include <sys/types.h>
-#  include <sys/wait.h>
-#  include <unistd.h>
+#include <signal.h> // NOLINT: sigaddset et al are defined in signal.h
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
 
 namespace {
 
 SignalHandler* g_the_signal_handler = nullptr;
 sigset_t g_fatal_signal_set;
 
+const int k_handled_signals[] = {
+  SIGINT,
+  SIGTERM,
+#ifdef SIGHUP
+  SIGHUP,
+#endif
+#ifdef SIGQUIT
+  SIGQUIT,
+#endif
+};
+
 void
 register_signal_handler(int signum)
 {
-  struct sigaction act;
-  memset(&act, 0, sizeof(act));
+  struct sigaction act = {};
   act.sa_handler = SignalHandler::on_signal;
   act.sa_mask = g_fatal_signal_set;
-#  ifdef SA_RESTART
+#ifdef SA_RESTART
   act.sa_flags = SA_RESTART;
-#  endif
+#endif
+  sigaction(signum, &act, nullptr);
+}
+
+void
+deregister_signal_handler(int signum)
+{
+  struct sigaction act = {};
+  act.sa_handler = SIG_DFL;
   sigaction(signum, &act, nullptr);
 }
 
@@ -54,23 +70,13 @@ SignalHandler::SignalHandler(Context& ctx) : m_ctx(ctx)
   g_the_signal_handler = this;
 
   sigemptyset(&g_fatal_signal_set);
-  sigaddset(&g_fatal_signal_set, SIGINT);
-  sigaddset(&g_fatal_signal_set, SIGTERM);
-#  ifdef SIGHUP
-  sigaddset(&g_fatal_signal_set, SIGHUP);
-#  endif
-#  ifdef SIGQUIT
-  sigaddset(&g_fatal_signal_set, SIGQUIT);
-#  endif
-
-  register_signal_handler(SIGINT);
-  register_signal_handler(SIGTERM);
-#  ifdef SIGHUP
-  register_signal_handler(SIGHUP);
-#  endif
-#  ifdef SIGQUIT
-  register_signal_handler(SIGQUIT);
-#  endif
+  for (int signum : k_handled_signals) {
+    sigaddset(&g_fatal_signal_set, signum);
+  }
+
+  for (int signum : k_handled_signals) {
+    register_signal_handler(signum);
+  }
 
   signal(SIGPIPE, SIG_IGN); // NOLINT: This is no error, clang-tidy
 }
@@ -78,6 +84,11 @@ SignalHandler::SignalHandler(Context& ctx) : m_ctx(ctx)
 SignalHandler::~SignalHandler()
 {
   ASSERT(g_the_signal_handler);
+
+  for (int signum : k_handled_signals) {
+    deregister_signal_handler(signum);
+  }
+
   g_the_signal_handler = nullptr;
 }
 
@@ -110,34 +121,18 @@ SignalHandler::on_signal(int signum)
   kill(getpid(), signum);
 }
 
-#else // !_WIN32
-
-SignalHandler::SignalHandler(Context& ctx) : m_ctx(ctx)
-{
-}
-
-SignalHandler::~SignalHandler()
-{
-}
-
-#endif // !_WIN32
-
 void
 SignalHandler::block_signals()
 {
-#ifndef _WIN32
   sigprocmask(SIG_BLOCK, &g_fatal_signal_set, nullptr);
-#endif
 }
 
 void
 SignalHandler::unblock_signals()
 {
-#ifndef _WIN32
   sigset_t empty;
   sigemptyset(&empty);
   sigprocmask(SIG_SETMASK, &empty, nullptr);
-#endif
 }
 
 SignalHandlerBlocker::SignalHandlerBlocker()

diff --git a/src/SignalHandler.hpp b/src/SignalHandler.hpp
index 603dbe213b60e2ce3879a4f4827659a490b1fd78..b75e4667417750a9fbd0679cf49420c1858212b8 100644 (file)
--- a/src/SignalHandler.hpp
+++ b/src/SignalHandler.hpp
@@ -1,4 +1,4 @@
-// Copyright (C) 2020-2021 Joel Rosdahl and other contributors
+// Copyright (C) 2020-2023 Joel Rosdahl and other contributors
 //
 // See doc/AUTHORS.adoc for a complete list of contributors.
 //
@@ -31,7 +31,9 @@ public:
   static void unblock_signals();
 
 private:
+#ifndef _WIN32
   Context& m_ctx;
+#endif
 };
 
 class SignalHandlerBlocker
@@ -40,3 +42,21 @@ public:
   SignalHandlerBlocker();
   ~SignalHandlerBlocker();
 };
+
+#ifdef _WIN32
+inline SignalHandler::SignalHandler(Context&)
+{
+}
+
+inline SignalHandler::~SignalHandler()
+{
+}
+
+inline SignalHandlerBlocker::SignalHandlerBlocker()
+{
+}
+
+inline SignalHandlerBlocker::~SignalHandlerBlocker()
+{
+}
+#endif