From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Fri, 30 Apr 2021 13:51:35 +0000 (+0200)
Subject: dnsdist: Convert the dynamic blocks topN to LockGuarded
X-Git-Tag: dnsdist-1.7.0-alpha1~62^2~17
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=018309973e9af50dc13368a36ebacfd0ff3429a9;p=thirdparty%2Fpdns.git

dnsdist: Convert the dynamic blocks topN to LockGuarded
---

diff --git a/pdns/dnsdist-dynblocks.hh b/pdns/dnsdist-dynblocks.hh
index e3c1a6247a..d802d05e1e 100644
--- a/pdns/dnsdist-dynblocks.hh
+++ b/pdns/dnsdist-dynblocks.hh
@@ -404,12 +404,15 @@ private:
     std::map<std::string, std::list<std::pair<DNSName, unsigned int>>> smtData;
   };
 
-  /* Protects s_topNMGsByReason and s_topSMTsByReason. s_metricsData should only be accessed
-     by the dynamic blocks maintenance thread so it does not need a lock. */
-  static std::mutex s_topsMutex;
+  struct Tops
+  {
+    std::map<std::string, std::list<std::pair<Netmask, unsigned int>>> topNMGsByReason;
+    std::map<std::string, std::list<std::pair<DNSName, unsigned int>>> topSMTsByReason;
+  };
+
+  static LockGuarded<Tops> s_tops;
+  /* s_metricsData should only be accessed by the dynamic blocks maintenance thread so it does not need a lock */
   // need N+1 datapoints to be able to do the diff after a collection point has been reached
   static std::list<MetricsSnapshot> s_metricsData;
-  static std::map<std::string, std::list<std::pair<Netmask, unsigned int>>> s_topNMGsByReason;
-  static std::map<std::string, std::list<std::pair<DNSName, unsigned int>>> s_topSMTsByReason;
   static size_t s_topN;
 };
diff --git a/pdns/dnsdistdist/dnsdist-dynblocks.cc b/pdns/dnsdistdist/dnsdist-dynblocks.cc
index ef33e33d98..fac0624264 100644
--- a/pdns/dnsdistdist/dnsdist-dynblocks.cc
+++ b/pdns/dnsdistdist/dnsdist-dynblocks.cc
@@ -493,10 +493,9 @@ struct DynBlockEntryStat
   unsigned int lastSeenValue{0};
 };
 
-std::mutex DynBlockMaintenance::s_topsMutex;
 std::list<DynBlockMaintenance::MetricsSnapshot> DynBlockMaintenance::s_metricsData;
-std::map<std::string, std::list<std::pair<Netmask, unsigned int>>> DynBlockMaintenance::s_topNMGsByReason;
-std::map<std::string, std::list<std::pair<DNSName, unsigned int>>> DynBlockMaintenance::s_topSMTsByReason;
+
+LockGuarded<DynBlockMaintenance::Tops> DynBlockMaintenance::s_tops;
 size_t DynBlockMaintenance::s_topN{20};
 time_t DynBlockMaintenance::s_expiredDynBlocksPurgeInterval{60};
 
@@ -641,9 +640,9 @@ void DynBlockMaintenance::generateMetrics()
   }
 
   {
-    std::lock_guard<std::mutex> lock(s_topsMutex);
-    s_topNMGsByReason = std::move(topNMGs);
-    s_topSMTsByReason = std::move(topSMTs);
+    auto tops = s_tops.lock();
+    tops->topNMGsByReason = std::move(topNMGs);
+    tops->topSMTsByReason = std::move(topSMTs);
   }
 }
 
@@ -710,12 +709,10 @@ void DynBlockMaintenance::run()
 
 std::map<std::string, std::list<std::pair<Netmask, unsigned int>>> DynBlockMaintenance::getHitsForTopNetmasks()
 {
-  std::lock_guard<std::mutex> lock(s_topsMutex);
-  return s_topNMGsByReason;
+  return s_tops.lock()->topNMGsByReason;
 }
 
 std::map<std::string, std::list<std::pair<DNSName, unsigned int>>> DynBlockMaintenance::getHitsForTopSuffixes()
 {
-  std::lock_guard<std::mutex> lock(s_topsMutex);
-  return s_topSMTsByReason;
+  return s_tops.lock()->topSMTsByReason;
 }