From 364a54918a76998b92426476eca6bfb90df1bb4a Mon Sep 17 00:00:00 2001
From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Wed, 7 Apr 2021 16:00:57 +0200
Subject: [PATCH] rec: Fix a few cases discoverd by tsan:

- The NegCache and MemRecursorCache destructors were not deadlock free
  when running from testrunner. The purpose of the code in the dts
  is also unclear, so delete them.

- quit-nicely uses a volatile sig_atomic_t, which is not thread-safe
  according to tsan. Replace by atomic.
---
 pdns/rec_channel.cc           |  2 +-
 pdns/rec_channel.hh           |  2 +-
 pdns/rec_channel_rec.cc       |  2 +-
 pdns/recursor_cache.cc        | 13 -------------
 pdns/recursor_cache.hh        |  1 -
 pdns/recursordist/negcache.cc | 13 -------------
 pdns/recursordist/negcache.hh |  1 -
 7 files changed, 3 insertions(+), 31 deletions(-)

diff --git a/pdns/rec_channel.cc b/pdns/rec_channel.cc
index 9381736f71..17d408398a 100644
--- a/pdns/rec_channel.cc
+++ b/pdns/rec_channel.cc
@@ -17,7 +17,7 @@
 
 #include "namespaces.hh"
 
-volatile sig_atomic_t RecursorControlChannel::stop = 0;
+std::atomic<bool> RecursorControlChannel::stop = false;
 
 RecursorControlChannel::RecursorControlChannel()
 {
diff --git a/pdns/rec_channel.hh b/pdns/rec_channel.hh
index 0b1610827e..d6ca9fbcc2 100644
--- a/pdns/rec_channel.hh
+++ b/pdns/rec_channel.hh
@@ -69,7 +69,7 @@ public:
   RecursorControlChannel::Answer recv(std::string* remote = nullptr, unsigned int timeout = 5);
 
   int d_fd;
-  static volatile sig_atomic_t stop;
+  static std::atomic<bool> stop;
 
 private:
   struct sockaddr_un d_local;
diff --git a/pdns/rec_channel_rec.cc b/pdns/rec_channel_rec.cc
index 44913e44fb..08ca90b8a9 100644
--- a/pdns/rec_channel_rec.cc
+++ b/pdns/rec_channel_rec.cc
@@ -1325,7 +1325,7 @@ void doExitGeneric(bool nicely)
   if(!s_pidfname.empty())
     unlink(s_pidfname.c_str()); // we can at least try..
   if(nicely) {
-    RecursorControlChannel::stop = 1;
+    RecursorControlChannel::stop = true;
   } else {
     _exit(1);
   }
diff --git a/pdns/recursor_cache.cc b/pdns/recursor_cache.cc
index c61076257c..9e05a49739 100644
--- a/pdns/recursor_cache.cc
+++ b/pdns/recursor_cache.cc
@@ -19,19 +19,6 @@ MemRecursorCache::MemRecursorCache(size_t mapsCount) : d_maps(mapsCount)
 {
 }
 
-MemRecursorCache::~MemRecursorCache()
-{
-  try {
-    typedef std::unique_ptr<lock> lock_t;
-    vector<lock_t> locks;
-    for (auto& map : d_maps) {
-      locks.push_back(lock_t(new lock(map)));
-    }
-  }
-  catch(...) {
-  }
-}
-
 size_t MemRecursorCache::size()
 {
   size_t count = 0;
diff --git a/pdns/recursor_cache.hh b/pdns/recursor_cache.hh
index 469d43df0a..0fe249f170 100644
--- a/pdns/recursor_cache.hh
+++ b/pdns/recursor_cache.hh
@@ -48,7 +48,6 @@ class MemRecursorCache : public boost::noncopyable //  : public RecursorCache
 {
 public:
   MemRecursorCache(size_t mapsCount = 1024);
-  ~MemRecursorCache();
 
   size_t size();
   size_t bytes();
diff --git a/pdns/recursordist/negcache.cc b/pdns/recursordist/negcache.cc
index 72c164b39d..c1030a6af6 100644
--- a/pdns/recursordist/negcache.cc
+++ b/pdns/recursordist/negcache.cc
@@ -31,19 +31,6 @@ NegCache::NegCache(size_t mapsCount) :
 {
 }
 
-NegCache::~NegCache()
-{
-  try {
-    typedef std::unique_ptr<lock> lock_t;
-    vector<lock_t> locks;
-    for (auto& map : d_maps) {
-      locks.push_back(lock_t(new lock(map)));
-    }
-  }
-  catch (...) {
-  }
-}
-
 size_t NegCache::size() const
 {
   size_t count = 0;
diff --git a/pdns/recursordist/negcache.hh b/pdns/recursordist/negcache.hh
index db360eb47e..7e07b9e08d 100644
--- a/pdns/recursordist/negcache.hh
+++ b/pdns/recursordist/negcache.hh
@@ -51,7 +51,6 @@ class NegCache : public boost::noncopyable
 {
 public:
   NegCache(size_t mapsCount = 1024);
-  ~NegCache();
 
   struct NegCacheEntry
   {
-- 
2.47.2