From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Mon, 16 Dec 2024 14:26:46 +0000 (+0100)
Subject: rec: make quit-nicely wait on actual quit
X-Git-Tag: dnsdist-2.0.0-alpha1~96^2~12
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=19d240fd828f6b5c689d246cdf9da1b8146f6488;p=thirdparty%2Fpdns.git

rec: make quit-nicely wait on actual quit
---

diff --git a/pdns/recursordist/rec-main.cc b/pdns/recursordist/rec-main.cc
index 2c0f123617..bc0a32826d 100644
--- a/pdns/recursordist/rec-main.cc
+++ b/pdns/recursordist/rec-main.cc
@@ -279,10 +279,9 @@ int RecThreadInfo::runThreads(Logr::log_t log)
     RecThreadInfo::setThreadId(currentThreadId);
     recursorThread();
 
-    for (unsigned int thread = 0; thread < RecThreadInfo::numRecursorThreads(); thread++) {
-      if (thread == 1) {
-        continue;
-      }
+    // Skip handler thread (it might be still handling the quite-nicely) and 1, which is actually the main thread in this case
+    // hanlder thread (0) will be handled in main().
+    for (unsigned int thread = 2; thread < RecThreadInfo::numRecursorThreads(); thread++) {
       auto& tInfo = RecThreadInfo::info(thread);
       tInfo.thread.join();
       if (tInfo.exitCode != 0) {
@@ -351,6 +350,9 @@ int RecThreadInfo::runThreads(Logr::log_t log)
     info.start(currentThreadId, "web+stat", cpusMap, log);
 
     for (auto& tInfo : RecThreadInfo::infos()) {
+      if (tInfo.getName() == "web+stat") { // XXX testing for isHandler() does not work as expected!
+        continue;
+      }
       tInfo.thread.join();
       if (tInfo.exitCode != 0) {
         ret = tInfo.exitCode;
@@ -2446,8 +2448,14 @@ static void handleRCC(int fileDesc, FDMultiplexer::funcparam_t& /* var */)
     RecursorControlParser::func_t* command = nullptr;
     auto answer = RecursorControlParser::getAnswer(clientfd, msg, &command);
 
-    g_rcc.send(clientfd, answer);
+    if (command != doExitNicely) {
+      g_rcc.send(clientfd, answer);
+    }
     command();
+    if (command == doExitNicely) {
+      g_rcc.send(clientfd, answer);
+      g_rcc.~RecursorControlChannel();
+    }
   }
   catch (const std::exception& e) {
     SLOG(g_log << Logger::Error << "Error dealing with control socket request: " << e.what() << endl,
@@ -3149,6 +3157,8 @@ static void setupLogging(const string& logname)
   }
 }
 
+DoneRunning doneRunning;
+
 int main(int argc, char** argv)
 {
   g_argc = argc;
@@ -3320,6 +3330,12 @@ int main(int argc, char** argv)
     }
 
     ret = serviceMain(startupLog);
+    {
+      std::lock_guard lock(doneRunning.mutex);
+      doneRunning.done = true;
+      doneRunning.condVar.notify_one();
+    }
+    RecThreadInfo::joinThread0();
   }
   catch (const PDNSException& ae) {
     SLOG(g_log << Logger::Error << "Exception: " << ae.reason << endl,
diff --git a/pdns/recursordist/rec-main.hh b/pdns/recursordist/rec-main.hh
index 1d07b75f3d..3709e1c721 100644
--- a/pdns/recursordist/rec-main.hh
+++ b/pdns/recursordist/rec-main.hh
@@ -155,6 +155,13 @@ extern thread_local unique_ptr<FDMultiplexer> t_fdm;
 extern uint16_t g_minUdpSourcePort;
 extern uint16_t g_maxUdpSourcePort;
 extern bool g_regressionTestMode;
+struct DoneRunning
+{
+  std::mutex mutex;
+  std::condition_variable condVar;
+  std::atomic<bool> done{false};
+};
+extern DoneRunning doneRunning;
 
 // you can ask this class for a UDP socket to send a query from
 // this socket is not yours, don't even think about deleting it
@@ -539,6 +546,11 @@ public:
     mt = theMT;
   }
 
+  static void joinThread0()
+  {
+    info(0).thread.join();
+  }
+
 private:
   // FD corresponding to TCP sockets this thread is listening on.
   // These FDs are also in deferredAdds when we have one socket per
diff --git a/pdns/recursordist/rec_channel_rec.cc b/pdns/recursordist/rec_channel_rec.cc
index 76fa3a6015..f0a7ded6d7 100644
--- a/pdns/recursordist/rec_channel_rec.cc
+++ b/pdns/recursordist/rec_channel_rec.cc
@@ -1379,7 +1379,6 @@ void doExitGeneric(bool nicely)
   _exit(0); // regression test check for exit 0
 #endif
   g_log << Logger::Error << "Exiting on user request" << endl;
-  g_rcc.~RecursorControlChannel();
 
   if (!g_pidfname.empty()) {
     unlink(g_pidfname.c_str()); // we can at least try..
@@ -1387,8 +1386,14 @@ void doExitGeneric(bool nicely)
 
   if (nicely) {
     RecursorControlChannel::stop = true;
+    {
+      std::unique_lock lock(doneRunning.mutex);
+      doneRunning.condVar.wait(lock, [] { return doneRunning.done.load(); });
+    }
+    // g_rcc.~RecursorControlChannel() do not call, will be done by caller!
   }
   else {
+    g_rcc.~RecursorControlChannel();
 #if defined(__SANITIZE_ADDRESS__) && defined(HAVE_LEAK_SANITIZER_INTERFACE)
     clearLuaScript();
     pdns::coverage::dumpCoverageData();