From c9badcb09c0901bd5075f563ada1a744d6c0745b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= <markus.makela@mariadb.com>
Date: Thu, 2 Jul 2020 07:28:45 +0300
Subject: [PATCH] Fix installation order of signal handlers

The shutdown signal handlers were installed before the workers were
initialized and weren't removed before the workers were deleted. This
would lead to a debug assertion and an eventual crash when a SIGTERM
signal was received outside of the expected scope.

The proper way to do this is to install the handlers only after the system
is up and running and to disable them as soon as the shutdown process
starts.

This mostly happened with the mxs621_unreadable_cnf test as it seemed to
receive a SIGTERM during the execution of the at-exit handlers.
---
 server/core/gateway.cc | 115 +++++++++++++++++++++++++++++++----------
 1 file changed, 88 insertions(+), 27 deletions(-)

diff --git a/server/core/gateway.cc b/server/core/gateway.cc
index 8e5637efc..dcbeb7ccc 100644
--- a/server/core/gateway.cc
+++ b/server/core/gateway.cc
@@ -1207,6 +1207,46 @@ bool disable_signals(void)
     return true;
 }
 
+bool disable_normal_signals(void)
+{
+    sigset_t sigset;
+
+    if (sigfillset(&sigset) != 0)
+    {
+        int e = errno;
+        errno = 0;
+        static const char message[] = "Failed to initialize set the signal set for MaxScale. Exiting.";
+        print_log_n_stderr(true, true, message, message, e);
+        return false;
+    }
+
+    if (!delete_signal(&sigset, SIGHUP, "SIGHUP"))
+    {
+        return false;
+    }
+
+    if (!delete_signal(&sigset, SIGUSR1, "SIGUSR1"))
+    {
+        return false;
+    }
+
+    if (!delete_signal(&sigset, SIGTERM, "SIGTERM"))
+    {
+        return false;
+    }
+
+    if (sigprocmask(SIG_SETMASK, &sigset, NULL) != 0)
+    {
+        int e = errno;
+        errno = 0;
+        static const char message[] = "Failed to set the signal set for MaxScale. Exiting";
+        print_log_n_stderr(true, true, message, message, e);
+        return false;
+    }
+
+    return true;
+}
+
 /**
  * Configures the handling of a particular signal.
  *
@@ -1234,31 +1274,12 @@ static bool configure_signal(int signum, const char* signame, void (* handler)(i
 }
 
 /**
- * Configures signal handling of MaxScale.
+ * Configure fatal signal handlers
  *
- * @return True, if all signals could be configured, false otherwise.
+ * @return True if signal handlers were installed correctly
  */
-bool configure_signals(void)
+bool configure_critical_signals(void)
 {
-    if (!configure_signal(SIGHUP, "SIGHUP", sighup_handler))
-    {
-        return false;
-    }
-
-    if (!configure_signal(SIGUSR1, "SIGUSR1", sigusr1_handler))
-    {
-        return false;
-    }
-
-    if (!configure_signal(SIGTERM, "SIGTERM", sigterm_handler))
-    {
-        return false;
-    }
-
-    if (!configure_signal(SIGINT, "SIGINT", sigint_handler))
-    {
-        return false;
-    }
 
     if (!configure_signal(SIGSEGV, "SIGSEGV", sigfatal_handler))
     {
@@ -1290,6 +1311,36 @@ bool configure_signals(void)
     return true;
 }
 
+/**
+ * Configures signal handling of MaxScale.
+ *
+ * @return True, if all signals could be configured, false otherwise.
+ */
+bool configure_normal_signals(void)
+{
+    if (!configure_signal(SIGHUP, "SIGHUP", sighup_handler))
+    {
+        return false;
+    }
+
+    if (!configure_signal(SIGUSR1, "SIGUSR1", sigusr1_handler))
+    {
+        return false;
+    }
+
+    if (!configure_signal(SIGTERM, "SIGTERM", sigterm_handler))
+    {
+        return false;
+    }
+
+    if (!configure_signal(SIGINT, "SIGINT", sigint_handler))
+    {
+        return false;
+    }
+
+    return true;
+}
+
 bool set_runtime_dirs(const char* basedir)
 {
     bool rv = true;
@@ -1860,12 +1911,10 @@ int main(int argc, char** argv)
          * the pipe. */
         close(daemon_pipe[0]);
     }
-    /*<
-     * Set signal handlers for SIGHUP, SIGTERM, SIGINT and critical signals like SIGSEGV.
-     */
-    if (!configure_signals())
+
+    if (!configure_critical_signals())
     {
-        static const char* logerr = "Failed to configure signal handlers. Exiting.";
+        static const char* logerr = "Failed to configure fatal signal handlers. Exiting.";
 
         print_log_n_stderr(true, true, logerr, logerr, 0);
         rc = MAXSCALE_INTERNALERROR;
@@ -2265,9 +2314,19 @@ int main(int argc, char** argv)
         write_child_exit_code(daemon_pipe[1], rc);
     }
 
+    if (!configure_normal_signals())
+    {
+        static const char* logerr = "Failed to configure all signal handlers. Exiting.";
+
+        print_log_n_stderr(true, true, logerr, logerr, 0);
+        rc = MAXSCALE_INTERNALERROR;
+        goto return_main;
+    }
+
     /*<
      * Run worker 0 in the main thread.
      */
+
     worker = RoutingWorker::get(RoutingWorker::MAIN);
     mxb_assert(worker);
     worker->run();
@@ -2278,6 +2337,8 @@ int main(int argc, char** argv)
     /*< Destroy all monitors */
     monitor_destroy_all();
 
+    disable_normal_signals();
+
     /*<
      * Wait for the housekeeper to finish.
      */