From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Fri, 4 Jun 2021 14:16:41 +0000 (+0200)
Subject: auth: Convert StatBag to LockGuarded
X-Git-Tag: dnsdist-1.7.0-alpha1~11^2~11
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=42b8372ca401bfc3e9c92e3449f69fbb50993516;p=thirdparty%2Fpdns.git

auth: Convert StatBag to LockGuarded
---

diff --git a/pdns/statbag.cc b/pdns/statbag.cc
index 28f46dfefc..bbacc27b4d 100644
--- a/pdns/statbag.cc
+++ b/pdns/statbag.cc
@@ -44,10 +44,9 @@ StatBag::StatBag()
 
 void StatBag::exists(const string &key)
 {
-  if(!d_keyDescriptions.count(key))
-    {
-      throw PDNSException("Trying to deposit into unknown StatBag key '"+key+"'");
-    }
+  if (!d_keyDescriptions.count(key)) {
+    throw PDNSException("Trying to deposit into unknown StatBag key '"+key+"'");
+  }
 }
 
 string StatBag::directory(const string &prefix)
@@ -148,20 +147,12 @@ unsigned long StatBag::read(const string &key)
 {
   exists(key);
   funcstats_t::const_iterator iter = d_funcstats.find(key);
-  if(iter != d_funcstats.end())
+  if (iter != d_funcstats.end()) {
     return iter->second(iter->first);
+  }
   return *d_stats[key];
 }
 
-unsigned long StatBag::readZero(const string &key)
-{
-  exists(key);
-  unsigned long tmp=*d_stats[key];
-  d_stats[key]=nullptr;
-  return tmp;
-}
-
-
 string StatBag::getValueStr(const string &key)
 {
   ostringstream o;
@@ -169,13 +160,6 @@ string StatBag::getValueStr(const string &key)
   return o.str();
 }
 
-string StatBag::getValueStrZero(const string &key)
-{
-  ostringstream o;
-  o<<readZero(key);
-  return o.str();
-}
-
 AtomicCounter *StatBag::getPointer(const string &key)
 {
   exists(key);
@@ -192,52 +176,38 @@ StatRing<T,Comp>::StatRing(unsigned int size)
   d_items.set_capacity(size);
 }
 
-template<typename T, typename Comp>
-StatRing<T,Comp>::StatRing(const StatRing<T,Comp> &arg)
-{
-  std::lock_guard<std::mutex> thislock(d_lock);
-  std::lock_guard<std::mutex> arglock(arg.d_lock);
-  
-  d_items = arg.d_items;
-  d_help = arg.d_help;
-}
-
 template<typename T, typename Comp>
 void StatRing<T,Comp>::account(const T& t)
 {
-  std::lock_guard<std::mutex> l(d_lock);
   d_items.push_back(t);
 }
 
 template<typename T, typename Comp>
 uint64_t StatRing<T,Comp>::getSize() const
 {
-  std::lock_guard<std::mutex> l(d_lock);
   return d_items.capacity();
 }
 
 template<typename T, typename Comp>
 uint64_t StatRing<T,Comp>::getEntriesCount() const
 {
-  std::lock_guard<std::mutex> l(d_lock);
   return d_items.size();
 }
 
 template<typename T, typename Comp>
 void StatRing<T,Comp>::resize(unsigned int newsize)
 {
-  std::lock_guard<std::mutex> l(d_lock);
   d_items.set_capacity(newsize);
 }
 
 template<typename T, typename Comp>
 void StatRing<T,Comp>::setHelp(const string &str)
 {
-  d_help=str;
+  d_help = str;
 }
 
 template<typename T, typename Comp>
-string StatRing<T,Comp>::getHelp()
+string StatRing<T,Comp>::getHelp() const
 {
   return d_help;
 }
@@ -246,7 +216,6 @@ string StatRing<T,Comp>::getHelp()
 template<typename T, typename Comp>
 vector<pair<T, unsigned int> >StatRing<T,Comp>::get() const
 {
-  std::lock_guard<std::mutex> l(d_lock);
   map<T,unsigned int, Comp> res;
   for(typename boost::circular_buffer<T>::const_iterator i=d_items.begin();i!=d_items.end();++i) {
     res[*i]++;
@@ -270,39 +239,39 @@ void StatBag::registerRingStats(const string& name)
 void StatBag::declareRing(const string &name, const string &help, unsigned int size)
 {
   d_rings.emplace(name, size);
-  d_rings[name].setHelp(help);
+  d_rings[name].lock()->setHelp(help);
   registerRingStats(name);
 }
 
 void StatBag::declareComboRing(const string &name, const string &help, unsigned int size)
 {
   d_comboRings.emplace(name, size);
-  d_comboRings[name].setHelp(help);
+  d_comboRings[name].lock()->setHelp(help);
   registerRingStats(name);
 }
 
 void StatBag::declareDNSNameQTypeRing(const string &name, const string &help, unsigned int size)
 {
   d_dnsnameqtyperings.emplace(name, size);
-  d_dnsnameqtyperings[name].setHelp(help);
+  d_dnsnameqtyperings[name].lock()->setHelp(help);
   registerRingStats(name);
 }
 
 vector<pair<string, unsigned int> > StatBag::getRing(const string &name)
 {
-  if(d_rings.count(name)) {
-    return d_rings[name].get();
+  if (d_rings.count(name)) {
+    return d_rings[name].lock()->get();
   }
   vector<pair<string, unsigned int> > ret;
 
   if (d_comboRings.count(name)) {
     typedef pair<SComboAddress, unsigned int> stor_t;
-    vector<stor_t> raw =d_comboRings[name].get();
+    vector<stor_t> raw =d_comboRings[name].lock()->get();
     for(const stor_t& stor :  raw) {
       ret.push_back(make_pair(stor.first.ca.toString(), stor.second));
     }
-  } else if(d_dnsnameqtyperings.count(name)) {
-    auto raw = d_dnsnameqtyperings[name].get();
+  } else if (d_dnsnameqtyperings.count(name)) {
+    auto raw = d_dnsnameqtyperings[name].lock()->get();
     for (auto const &e : raw) {
       ret.push_back(make_pair(std::get<0>(e.first).toLogString() + "/" + std::get<1>(e.first).toString(), e.second));
     }
@@ -313,65 +282,64 @@ vector<pair<string, unsigned int> > StatBag::getRing(const string &name)
 template<typename T, typename Comp>
 void StatRing<T,Comp>::reset()
 {
-  std::lock_guard<std::mutex> l(d_lock);
   d_items.clear();
 }
 
 void StatBag::resetRing(const string &name)
 {
   if(d_rings.count(name))
-    d_rings[name].reset();
+    d_rings[name].lock()->reset();
   if(d_comboRings.count(name))
-    d_comboRings[name].reset();
+    d_comboRings[name].lock()->reset();
   if(d_dnsnameqtyperings.count(name))
-    d_dnsnameqtyperings[name].reset();
+    d_dnsnameqtyperings[name].lock()->reset();
 }
 
 void StatBag::resizeRing(const string &name, unsigned int newsize)
 {
   if(d_rings.count(name))
-    d_rings[name].resize(newsize);
+    d_rings[name].lock()->resize(newsize);
   if(d_comboRings.count(name))
-    d_comboRings[name].resize(newsize);
+    d_comboRings[name].lock()->resize(newsize);
   if(d_dnsnameqtyperings.count(name))
-    return d_dnsnameqtyperings[name].resize(newsize);
+    return d_dnsnameqtyperings[name].lock()->resize(newsize);
 }
 
 
 uint64_t StatBag::getRingSize(const string &name)
 {
   if(d_rings.count(name))
-    return d_rings[name].getSize();
+    return d_rings[name].lock()->getSize();
   if(d_comboRings.count(name))
-    return d_comboRings[name].getSize();
+    return d_comboRings[name].lock()->getSize();
   if(d_dnsnameqtyperings.count(name))
-    return d_dnsnameqtyperings[name].getSize();
+    return d_dnsnameqtyperings[name].lock()->getSize();
   return 0;
 }
 
 uint64_t StatBag::getRingEntriesCount(const string &name)
 {
   if(d_rings.count(name))
-    return d_rings[name].getEntriesCount();
+    return d_rings[name].lock()->getEntriesCount();
   if(d_comboRings.count(name))
-    return d_comboRings[name].getEntriesCount();
+    return d_comboRings[name].lock()->getEntriesCount();
   if(d_dnsnameqtyperings.count(name))
-    return d_dnsnameqtyperings[name].getEntriesCount();
+    return d_dnsnameqtyperings[name].lock()->getEntriesCount();
   return 0;
 }
 
 string StatBag::getRingTitle(const string &name)
 {
   if(d_rings.count(name))
-    return d_rings[name].getHelp();
+    return d_rings[name].lock()->getHelp();
   if(d_comboRings.count(name))
-    return d_comboRings[name].getHelp();
+    return d_comboRings[name].lock()->getHelp();
   if(d_dnsnameqtyperings.count(name))
-    return d_dnsnameqtyperings[name].getHelp();
+    return d_dnsnameqtyperings[name].lock()->getHelp();
   return "";
 }
 
-vector<string>StatBag::listRings()
+vector<string>StatBag::listRings() const
 {
   vector<string> ret;
   for(auto & d_ring : d_rings)
@@ -384,7 +352,7 @@ vector<string>StatBag::listRings()
   return ret;
 }
 
-bool StatBag::ringExists(const string &name)
+bool StatBag::ringExists(const string &name) const
 {
   return d_rings.count(name) || d_comboRings.count(name) || d_dnsnameqtyperings.count(name);
 }
diff --git a/pdns/statbag.hh b/pdns/statbag.hh
index ce5030dbaa..e0aa3c04e6 100644
--- a/pdns/statbag.hh
+++ b/pdns/statbag.hh
@@ -22,7 +22,6 @@
 #pragma once
 #include <pthread.h>
 #include <map>
-#include <mutex>
 #include <functional>
 #include <string>
 #include <vector>
@@ -31,15 +30,15 @@
 #include "iputils.hh"
 #include "circular_buffer.hh"
 
-
 template<typename T, typename Comp=std::less<T> >
 class StatRing
 {
 public:
   StatRing(unsigned int size=10000);
-  // Some older C++ libs have trouble emplacing without a copy-constructor, so provide one
-  StatRing(const StatRing &);
-  StatRing & operator=(const StatRing &) = delete;
+  StatRing(const StatRing&) = delete;
+  StatRing& operator=(const StatRing&) = delete;
+  StatRing& operator=(StatRing&&) = delete;
+  StatRing(StatRing&&) = default;
   
   void account(const T &item);
 
@@ -48,7 +47,7 @@ public:
   void resize(unsigned int newsize);  
   void reset();
   void setHelp(const string &str);
-  string getHelp();
+  string getHelp() const;
 
   vector<pair<T, unsigned int> > get() const;
 private:
@@ -58,7 +57,6 @@ private:
   }
 
   boost::circular_buffer<T> d_items;
-  mutable std::mutex d_lock;
   string d_help;
 };
 
@@ -73,9 +71,9 @@ class StatBag
   map<string, std::unique_ptr<AtomicCounter>> d_stats;
   map<string, string> d_keyDescriptions;
   map<string, StatType> d_statTypes;
-  map<string,StatRing<string, CIStringCompare> >d_rings;
-  map<string,StatRing<SComboAddress> >d_comboRings;
-  map<string,StatRing<std::tuple<DNSName, QType> > >d_dnsnameqtyperings;
+  map<string, LockGuarded<StatRing<string, CIStringCompare> > > d_rings;
+  map<string, LockGuarded<StatRing<SComboAddress> > > d_comboRings;
+  map<string, LockGuarded<StatRing<std::tuple<DNSName, QType> > > > d_dnsnameqtyperings;
   typedef boost::function<uint64_t(const std::string&)> func_t;
   typedef map<string, func_t> funcstats_t;
   funcstats_t d_funcstats;
@@ -98,27 +96,33 @@ public:
   string getRingTitle(const string &name);
   void ringAccount(const char* name, const string &item)
   {
-    if(d_doRings)  {
-      if(!d_rings.count(name))
+    if (d_doRings)  {
+      auto it = d_rings.find(name);
+      if (it == d_rings.end()) {
 	throw runtime_error("Attempting to account to non-existent ring '"+std::string(name)+"'");
+      }
 
-      d_rings[name].account(item);
+      it->second.lock()->account(item);
     }
   }
   void ringAccount(const char* name, const ComboAddress &item)
   {
-    if(d_doRings) {
-      if(!d_comboRings.count(name))
+    if (d_doRings) {
+      auto it = d_comboRings.find(name);
+      if (it == d_comboRings.end()) {
 	throw runtime_error("Attempting to account to non-existent comboRing '"+std::string(name)+"'");
-      d_comboRings[name].account(item);
+      }
+      it->second.lock()->account(item);
     }
   }
   void ringAccount(const char* name, const DNSName &dnsname, const QType &qtype)
   {
-    if(d_doRings) {
-      if(!d_dnsnameqtyperings.count(name))
+    if (d_doRings) {
+      auto it = d_dnsnameqtyperings.find(name);
+      if (it == d_dnsnameqtyperings.end()) {
 	throw runtime_error("Attempting to account to non-existent dnsname+qtype ring '"+std::string(name)+"'");
-      d_dnsnameqtyperings[name].account(std::make_tuple(dnsname, qtype));
+      }
+      it->second.lock()->account(std::make_tuple(dnsname, qtype));
     }
   }
 
@@ -127,8 +131,8 @@ public:
     d_doRings=true;
   }
 
-  vector<string>listRings();
-  bool ringExists(const string &name);
+  vector<string>listRings() const;
+  bool ringExists(const string &name) const;
   void resetRing(const string &name);
   void resizeRing(const string &name, unsigned int newsize);
   uint64_t getRingSize(const string &name);
@@ -143,10 +147,8 @@ public:
   inline void inc(const string &key); //!< increase this key's value by one
   void set(const string &key, unsigned long value); //!< set this key's value
   unsigned long read(const string &key); //!< read the value behind this key
-  unsigned long readZero(const string &key); //!< read the value behind this key, and zero it afterwards
   AtomicCounter *getPointer(const string &key); //!< get a direct pointer to the value behind a key. Use this for high performance increments
   string getValueStr(const string &key); //!< read a value behind a key, and return it as a string
-  string getValueStrZero(const string &key); //!< read a value behind a key, and return it as a string, and zero afterwards
   void blacklist(const string &str);
 
   bool d_allowRedeclare; // only set this true during tests, never in production code
@@ -161,5 +163,5 @@ inline void StatBag::deposit(const string &key, int value)
 
 inline void StatBag::inc(const string &key)
 {
-  deposit(key,1);
+  deposit(key, 1);
 }