From aa21f612393a047222f1881cb17f5baf1b6b5bc5 Mon Sep 17 00:00:00 2001
From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Wed, 12 Feb 2025 10:17:30 +0100
Subject: [PATCH] Clearly separate signal handling from rec_control
 quit(-nicely) path

Also: as the bulk and regression tests now use quit-nicely, we don't need to do special processing
for the SAN case anymore on signals.

rec_control quit still does some SAN work and exits 1 (legacy beheviour).
---
 pdns/recursordist/rec-main.cc        |  6 +-----
 pdns/recursordist/rec_channel.hh     |  2 --
 pdns/recursordist/rec_channel_rec.cc | 12 ++++++------
 3 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/pdns/recursordist/rec-main.cc b/pdns/recursordist/rec-main.cc
index 00b1b8f67b..8ad01ea68f 100644
--- a/pdns/recursordist/rec-main.cc
+++ b/pdns/recursordist/rec-main.cc
@@ -952,7 +952,7 @@ static void daemonize(Logr::log_t log)
 
 static void termIntHandler([[maybe_unused]] int arg)
 {
-  doExit();
+  _exit(1);
 }
 
 static void usr1Handler([[maybe_unused]] int arg)
@@ -1962,10 +1962,6 @@ static int initForks(Logr::log_t log)
     signal(SIGTERM, termIntHandler);
     signal(SIGINT, termIntHandler);
   }
-#if defined(__SANITIZE_THREAD__) || (defined(__SANITIZE_ADDRESS__) && defined(HAVE_LEAK_SANITIZER_INTERFACE))
-  // If san is wanted, we dump the info ourselves
-  signal(SIGTERM, termIntHandler);
-#endif
 
   signal(SIGUSR1, usr1Handler);
   signal(SIGUSR2, usr2Handler);
diff --git a/pdns/recursordist/rec_channel.hh b/pdns/recursordist/rec_channel.hh
index f9977e5892..f86092107c 100644
--- a/pdns/recursordist/rec_channel.hh
+++ b/pdns/recursordist/rec_channel.hh
@@ -139,8 +139,6 @@ void disableStats(StatComponent component, const string& stats);
 
 void registerAllStats();
 
-void doExitGeneric(bool nicely);
-void doExit();
 void doExitNicely();
 RecursorControlChannel::Answer doQueueReloadLuaScript(vector<string>::const_iterator begin, vector<string>::const_iterator end);
 RecursorControlChannel::Answer luaconfig(bool broadcast);
diff --git a/pdns/recursordist/rec_channel_rec.cc b/pdns/recursordist/rec_channel_rec.cc
index bf0b64a5af..08d6000890 100644
--- a/pdns/recursordist/rec_channel_rec.cc
+++ b/pdns/recursordist/rec_channel_rec.cc
@@ -1373,15 +1373,13 @@ static auto clearLuaScript()
   return doQueueReloadLuaScript(empty.begin(), empty.end());
 }
 
-void doExitGeneric(bool nicely)
+// This code SHOUD *NOT* BE CALLED BY SIGNAL HANDLERS anymore
+static void doExitGeneric(bool nicely)
 {
 #if defined(__SANITIZE_THREAD__)
   _exit(0); // regression test check for exit 0
 #endif
-  // Not safe from a signal handler!
-  // g_slog->withName("runtime")->info(Logr::Notice, "Exiting on user request")
-  static const string msg("Exiting on user request\n");
-  (void)write(STDERR_FILENO, msg.data(), msg.size());
+  g_slog->withName("runtime")->info(Logr::Notice, "Exiting on user request", "nicely", Logging::Loggable(nicely));
 
   if (!g_pidfname.empty()) {
     unlink(g_pidfname.c_str()); // we can at least try..
@@ -1394,8 +1392,10 @@ void doExitGeneric(bool nicely)
       g_doneRunning.condVar.wait(lock, [] { return g_doneRunning.done.load(); });
     }
     // g_rcc.~RecursorControlChannel() do not call, caller still needs it!
+    // Caller will continue doing the orderly shutdown
   }
   else {
+    // rec_control quit case. Is that still used by test code? bulktests and regression test use quit-nicely
     g_rcc.~RecursorControlChannel();
 #if defined(__SANITIZE_ADDRESS__) && defined(HAVE_LEAK_SANITIZER_INTERFACE)
     clearLuaScript();
@@ -1409,7 +1409,7 @@ void doExitGeneric(bool nicely)
   }
 }
 
-void doExit()
+static void doExit()
 {
   doExitGeneric(false);
 }
-- 
2.47.2