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.

diff --git a/pdns/rec_channel.cc b/pdns/rec_channel.cc
index 9381736f713efefe05616f96d69d1292a8dc671d..17d408398a2087a64adfa530ee4d24f5a98a1640 100644 (file)
--- 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 0b1610827e483e68ff28b82251e3f07d91cfab9c..d6ca9fbcc27c5ea6209435c9f12e5ea4b8695806 100644 (file)
--- 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 44913e44fbfe3d5226cbdc0ca55670f114aba837..08ca90b8a91e958b3bf82d98e93c1d2c9e1f50b5 100644 (file)
--- 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 c61076257c39602343cffd296cba1751ba28b00c..9e05a49739c841ded18520aa23e0e4ae438fcfaa 100644 (file)
--- 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 469d43df0a87d8cbe2e949dc931976e16464adbf..0fe249f1708bc29500e236d1347d20b22ddb0963 100644 (file)
--- 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 72c164b39d746cf460704084dafce3b2b78cef32..c1030a6af66edb25b38e6e9b7dd2c3dc1b2dcfbf 100644 (file)
--- 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 db360eb47eaeb4cf28393f5f7aae32a3c04b7622..7e07b9e08d1878fca997c28608246963360a681e 100644 (file)
--- 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
   {