auth: Convert the caches to LockGuarded

diff --git a/pdns/auth-packetcache.cc b/pdns/auth-packetcache.cc
index e31a0182eff7ba469117f4ee47b68576bf5a9d17..15f2bf358a4dce7878d1f9f77f24d0ec33e109ba 100644 (file)
--- a/pdns/auth-packetcache.cc
+++ b/pdns/auth-packetcache.cc
@@ -43,24 +43,10 @@ AuthPacketCache::AuthPacketCache(size_t mapsCount): d_maps(mapsCount), d_lastcle
   d_statnumentries=S.getPointer("packetcache-size");
 }
 
-AuthPacketCache::~AuthPacketCache()
-{
-  try {
-    vector<WriteLock> locks;
-    for(auto& mc : d_maps) {
-      locks.push_back(WriteLock(mc.d_mut));
-    }
-    locks.clear();
-  }
-  catch(...) {
-  }
-}
-
 void AuthPacketCache::MapCombo::reserve(size_t numberOfEntries)
 {
 #if BOOST_VERSION >= 105600
-  WriteLock wl(&d_mut);
-  d_map.get<HashTag>().reserve(numberOfEntries);
+  d_map.write_lock()->get<HashTag>().reserve(numberOfEntries);
 #endif /* BOOST_VERSION >= 105600 */
 }
 
@@ -80,13 +66,13 @@ bool AuthPacketCache::get(DNSPacket& p, DNSPacket& cached)
   time_t now = time(nullptr);
   auto& mc = getMap(p.qdomain);
   {
-    TryReadLock rl(&mc.d_mut);
-    if(!rl.gotIt()) {
+    auto map = mc.d_map.try_read_lock();
+    if (!map.owns_lock()) {
       S.inc("deferred-packetcache-lookup");
       return false;
     }
 
-    haveSomething = getEntryLocked(mc.d_map, p.getString(), hash, p.qdomain, p.qtype.getCode(), p.d_tcp, now, value);
+    haveSomething = getEntryLocked(*map, p.getString(), hash, p.qdomain, p.qtype.getCode(), p.d_tcp, now, value);
   }
 
   if (!haveSomething) {
@@ -146,13 +132,13 @@ void AuthPacketCache::insert(DNSPacket& q, DNSPacket& r, unsigned int maxTTL)
   
   auto& mc = getMap(entry.qname);
   {
-    TryWriteLock l(&mc.d_mut);
-    if (!l.gotIt()) {
+    auto map = mc.d_map.try_write_lock();
+    if (!map.owns_lock()) {
       S.inc("deferred-packetcache-inserts");
       return;
     }
 
-    auto& idx = mc.d_map.get<HashTag>();
+    auto& idx = map->get<HashTag>();
     auto range = idx.equal_range(hash);
     auto iter = range.first;
 
@@ -161,7 +147,7 @@ void AuthPacketCache::insert(DNSPacket& q, DNSPacket& r, unsigned int maxTTL)
         continue;
       }
 
-      moveCacheItemToBack<SequencedTag>(mc.d_map, iter);
+      moveCacheItemToBack<SequencedTag>(*map, iter);
       iter->value = entry.value;
       iter->ttd = now + ourttl;
       iter->created = now;
@@ -169,11 +155,11 @@ void AuthPacketCache::insert(DNSPacket& q, DNSPacket& r, unsigned int maxTTL)
     }
 
     /* no existing entry found to refresh */
-    mc.d_map.insert(std::move(entry));
+    map->insert(std::move(entry));
 
     if (*d_statnumentries >= d_maxEntries) {
       /* remove the least recently inserted or replaced entry */
-      auto& sidx = mc.d_map.get<SequencedTag>();
+      auto& sidx = map->get<SequencedTag>();
       sidx.pop_front();
     }
     else {
@@ -182,7 +168,7 @@ void AuthPacketCache::insert(DNSPacket& q, DNSPacket& r, unsigned int maxTTL)
   }
 }
 
-bool AuthPacketCache::getEntryLocked(cmap_t& map, const std::string& query, uint32_t hash, const DNSName &qname, uint16_t qtype, bool tcp, time_t now, string& value)
+bool AuthPacketCache::getEntryLocked(const cmap_t& map, const std::string& query, uint32_t hash, const DNSName &qname, uint16_t qtype, bool tcp, time_t now, string& value)
 {
   auto& idx = map.get<HashTag>();
   auto range = idx.equal_range(hash);

diff --git a/pdns/auth-packetcache.hh b/pdns/auth-packetcache.hh
index bceab073de59797980e75056d3bea546d68d5e41..618041cf17d091413dfcab3276eab0d68a7b43f2 100644 (file)
--- a/pdns/auth-packetcache.hh
+++ b/pdns/auth-packetcache.hh
@@ -48,7 +48,6 @@ class AuthPacketCache : public PacketCache
 {
 public:
   AuthPacketCache(size_t mapsCount=1024);
-  ~AuthPacketCache();
 
   void insert(DNSPacket& q, DNSPacket& r, uint32_t maxTTL);  //!< We copy the contents of *p into our cache. Do not needlessly call this to insert questions already in the cache as it wastes resources
 
@@ -116,8 +115,7 @@ private:
 
     void reserve(size_t numberOfEntries);
 
-    ReadWriteLock d_mut;
-    cmap_t d_map;
+    SharedLockGuarded<cmap_t> d_map;
   };
 
   vector<MapCombo> d_maps;
@@ -127,7 +125,7 @@ private:
   }
 
   static bool entryMatches(cmap_t::index<HashTag>::type::iterator& iter, const std::string& query, const DNSName& qname, uint16_t qtype, bool tcp);
-  bool getEntryLocked(cmap_t& map, const std::string& query, uint32_t hash, const DNSName &qname, uint16_t qtype, bool tcp, time_t now, string& entry);
+  bool getEntryLocked(const cmap_t& map, const std::string& query, uint32_t hash, const DNSName &qname, uint16_t qtype, bool tcp, time_t now, string& entry);
   void cleanupIfNeeded();
 
   AtomicCounter d_ops{0};

diff --git a/pdns/auth-querycache.cc b/pdns/auth-querycache.cc
index 7812a82e1ff694aa84d8f61cc02b1d202f2fff44..6913296d847fcca9b6b4f1ae1c836eacee6d7c1a 100644 (file)
--- a/pdns/auth-querycache.cc
+++ b/pdns/auth-querycache.cc
@@ -44,24 +44,10 @@ AuthQueryCache::AuthQueryCache(size_t mapsCount): d_maps(mapsCount), d_lastclean
   d_statnumentries=S.getPointer("query-cache-size");
 }
 
-AuthQueryCache::~AuthQueryCache()
-{
-  try {
-    vector<WriteLock> locks;
-    for(auto& mc : d_maps) {
-      locks.push_back(WriteLock(mc.d_mut));
-    }
-    locks.clear();
-  }
-  catch(...) {
-  }
-}
-
 void AuthQueryCache::MapCombo::reserve(size_t numberOfEntries)
 {
 #if BOOST_VERSION >= 105600
-  WriteLock wl(&d_mut);
-  d_map.get<HashTag>().reserve(numberOfEntries);
+  d_map.write_lock()->get<HashTag>().reserve(numberOfEntries);
 #endif /* BOOST_VERSION >= 105600 */
 }
 
@@ -74,13 +60,13 @@ bool AuthQueryCache::getEntry(const DNSName &qname, const QType& qtype, vector<D
   uint16_t qt = qtype.getCode();
   auto& mc = getMap(qname);
   {
-    TryReadLock rl(&mc.d_mut);
-    if(!rl.gotIt()) {
+    auto map = mc.d_map.try_read_lock();
+    if (!map.owns_lock()) {
       S.inc("deferred-cache-lookup");
       return false;
     }
 
-    return getEntryLocked(mc.d_map, qname, qt, value, zoneID, now);
+    return getEntryLocked(*map, qname, qt, value, zoneID, now);
   }
 }
 
@@ -103,24 +89,24 @@ void AuthQueryCache::insert(const DNSName &qname, const QType& qtype, vector<DNS
   auto& mc = getMap(val.qname);
 
   {
-    TryWriteLock l(&mc.d_mut);
-    if(!l.gotIt()) {
+    auto map = mc.d_map.try_write_lock();
+    if (!map.owns_lock()) {
       S.inc("deferred-cache-inserts"); 
       return;
     }
 
     bool inserted;
     cmap_t::iterator place;
-    tie(place, inserted) = mc.d_map.insert(val);
+    tie(place, inserted) = map->insert(val);
 
     if (!inserted) {
-      mc.d_map.replace(place, std::move(val));
-      moveCacheItemToBack<SequencedTag>(mc.d_map, place);
+      map->replace(place, std::move(val));
+      moveCacheItemToBack<SequencedTag>(*map, place);
     }
     else {
       if (*d_statnumentries >= d_maxEntries) {
         /* remove the least recently inserted or replaced entry */
-        auto& sidx = mc.d_map.get<SequencedTag>();
+        auto& sidx = map->get<SequencedTag>();
         sidx.pop_front();
       }
       else {
@@ -130,7 +116,7 @@ void AuthQueryCache::insert(const DNSName &qname, const QType& qtype, vector<DNS
   }
 }
 
-bool AuthQueryCache::getEntryLocked(cmap_t& map, const DNSName &qname, uint16_t qtype, vector<DNSZoneRecord>& value, int zoneID, time_t now)
+bool AuthQueryCache::getEntryLocked(const cmap_t& map, const DNSName &qname, uint16_t qtype, vector<DNSZoneRecord>& value, int zoneID, time_t now)
 {
   auto& idx = boost::multi_index::get<HashTag>(map);
   auto iter = idx.find(tie(qname, qtype, zoneID));
@@ -155,9 +141,9 @@ map<char,uint64_t> AuthQueryCache::getCounts()
   uint64_t queryCacheEntries=0, negQueryCacheEntries=0;
 
   for(auto& mc : d_maps) {
-    ReadLock l(&mc.d_mut);
+    auto map = mc.d_map.read_lock();
     
-    for(const auto & iter : mc.d_map) {
+    for(const auto & iter : *map) {
       if(iter.drs.empty())
         negQueryCacheEntries++;
       else

diff --git a/pdns/auth-querycache.hh b/pdns/auth-querycache.hh
index 33bd8a7cae0aa84118b09a6ac068e1f071c97858..b5ee72db123d6a1f9b7f4abf2e1bc59f2df7cd07 100644 (file)
--- a/pdns/auth-querycache.hh
+++ b/pdns/auth-querycache.hh
@@ -37,7 +37,6 @@ class AuthQueryCache : public boost::noncopyable
 {
 public:
   AuthQueryCache(size_t mapsCount=1024);
-  ~AuthQueryCache();
 
   void insert(const DNSName &qname, const QType& qtype, vector<DNSZoneRecord>&& content, uint32_t ttl, int zoneID);
 
@@ -99,8 +98,7 @@ private:
 
     void reserve(size_t numberOfEntries);
 
-    ReadWriteLock d_mut;
-    cmap_t d_map;
+    SharedLockGuarded<cmap_t> d_map;
   };
 
   vector<MapCombo> d_maps;
@@ -109,7 +107,7 @@ private:
     return d_maps[qname.hash() % d_maps.size()];
   }
 
-  bool getEntryLocked(cmap_t& map, const DNSName &content, uint16_t qtype, vector<DNSZoneRecord>& entry, int zoneID, time_t now);
+  bool getEntryLocked(const cmap_t& map, const DNSName &content, uint16_t qtype, vector<DNSZoneRecord>& entry, int zoneID, time_t now);
   void cleanupIfNeeded();
 
   AtomicCounter d_ops{0};

diff --git a/pdns/auth-zonecache.cc b/pdns/auth-zonecache.cc
index bdac5ed81ab5d3eb15f2a105921d70c0f5f7dee3..687879d0c798ce9e366a8d00bc1278cc2d26e4ff 100644 (file)
--- a/pdns/auth-zonecache.cc
+++ b/pdns/auth-zonecache.cc
@@ -42,27 +42,14 @@ AuthZoneCache::AuthZoneCache(size_t mapsCount) :
   d_statnumentries = S.getPointer("zone-cache-size");
 }
 
-AuthZoneCache::~AuthZoneCache()
-{
-  try {
-    vector<WriteLock> locks;
-    for (auto& mc : d_maps) {
-      locks.push_back(WriteLock(mc.d_mut));
-    }
-    locks.clear();
-  }
-  catch (...) {
-  }
-}
-
 bool AuthZoneCache::getEntry(const DNSName& zone, int& zoneId)
 {
   auto& mc = getMap(zone);
   bool found = false;
   {
-    ReadLock rl(mc.d_mut);
-    auto iter = mc.d_map.find(zone);
-    if (iter != mc.d_map.end()) {
+    auto map = mc.d_map.read_lock();
+    auto iter = map->find(zone);
+    if (iter != map->end()) {
       found = true;
       zoneId = iter->second.zoneId;
     }
@@ -93,7 +80,7 @@ void AuthZoneCache::replace(const vector<tuple<DNSName, int>>& zone_indices)
     return;
 
   size_t count = zone_indices.size();
-  vector<MapCombo> newMaps(d_maps.size());
+  vector<cmap_t> newMaps(d_maps.size());
 
   // build new maps
   for (const tuple<DNSName, int>& tup : zone_indices) {
@@ -101,36 +88,36 @@ void AuthZoneCache::replace(const vector<tuple<DNSName, int>>& zone_indices)
     CacheValue val;
     val.zoneId = tup.get<1>();
     auto& mc = newMaps[getMapIndex(zone)];
-    auto iter = mc.d_map.find(zone);
-    if (iter != mc.d_map.end()) {
+    auto iter = mc.find(zone);
+    if (iter != mc.end()) {
       iter->second = std::move(val);
     }
     else {
-      mc.d_map.emplace(zone, val);
+      mc.emplace(zone, val);
     }
   }
 
   {
-    WriteLock globalLock(d_mut);
-    if (d_replacePending) {
+    auto pending = d_pending.lock();
+    if (pending->d_replacePending) {
       // add/replace all zones created while data collection for replace() was already in progress.
-      for (const tuple<DNSName, int>& tup : d_pendingAdds) {
+      for (const tuple<DNSName, int>& tup : pending->d_pendingAdds) {
         const DNSName& zone = tup.get<0>();
         CacheValue val;
         val.zoneId = tup.get<1>();
         auto& mc = newMaps[getMapIndex(zone)];
-        mc.d_map[zone] = val;
+        mc[zone] = val;
       }
     }
 
     for (size_t mapIndex = 0; mapIndex < d_maps.size(); mapIndex++) {
       auto& mc = d_maps[mapIndex];
-      WriteLock mcLock(mc.d_mut);
-      mc.d_map = std::move(newMaps[mapIndex].d_map);
+      auto map = mc.d_map.write_lock();
+      *map = std::move(newMaps[mapIndex]);
     }
 
-    d_pendingAdds.clear();
-    d_replacePending = false;
+    pending->d_pendingAdds.clear();
+    pending->d_replacePending = false;
   }
 
   d_statnumentries->store(count);
@@ -142,9 +129,9 @@ void AuthZoneCache::add(const DNSName& zone, const int zoneId)
     return;
 
   {
-    WriteLock globalLock(d_mut);
-    if (d_replacePending) {
-      d_pendingAdds.push_back({zone, zoneId});
+    auto pending = d_pending.lock();
+    if (pending->d_replacePending) {
+      pending->d_pendingAdds.push_back({zone, zoneId});
     }
   }
 
@@ -154,13 +141,13 @@ void AuthZoneCache::add(const DNSName& zone, const int zoneId)
   int mapIndex = getMapIndex(zone);
   {
     auto& mc = d_maps[mapIndex];
-    WriteLock mcLock(mc.d_mut);
-    auto iter = mc.d_map.find(zone);
-    if (iter != mc.d_map.end()) {
+    auto map = mc.d_map.write_lock();
+    auto iter = map->find(zone);
+    if (iter != map->end()) {
       iter->second = std::move(val);
     }
     else {
-      mc.d_map.emplace(zone, val);
+      map->emplace(zone, val);
     }
   }
 }
@@ -171,8 +158,8 @@ void AuthZoneCache::setReplacePending()
     return;
 
   {
-    WriteLock globalLock(d_mut);
-    d_replacePending = true;
-    d_pendingAdds.clear();
+    auto pending = d_pending.lock();
+    pending->d_replacePending = true;
+    pending->d_pendingAdds.clear();
   }
 }

diff --git a/pdns/auth-zonecache.hh b/pdns/auth-zonecache.hh
index e5ace8b265e1bbdf65e0c3ec22ffe715870ef55c..e3968e1f89373056e2882805ba7038ce68a2d3ea 100644 (file)
--- a/pdns/auth-zonecache.hh
+++ b/pdns/auth-zonecache.hh
@@ -31,7 +31,6 @@ class AuthZoneCache : public boost::noncopyable
 {
 public:
   AuthZoneCache(size_t mapsCount = 1024);
-  ~AuthZoneCache();
 
   void replace(const vector<tuple<DNSName, int>>& zone);
   void add(const DNSName& zone, const int zoneId);
@@ -70,8 +69,7 @@ private:
     MapCombo(const MapCombo&) = delete;
     MapCombo& operator=(const MapCombo&) = delete;
 
-    ReadWriteLock d_mut;
-    cmap_t d_map;
+    SharedLockGuarded<cmap_t> d_map;
   };
 
   vector<MapCombo> d_maps;
@@ -90,9 +88,12 @@ private:
 
   time_t d_refreshinterval{0};
 
-  ReadWriteLock d_mut;
-  std::vector<tuple<DNSName, int>> d_pendingAdds;
-  bool d_replacePending{false};
+  struct PendingData
+  {
+    std::vector<tuple<DNSName, int>> d_pendingAdds;
+    bool d_replacePending{false};
+  };
+  LockGuarded<PendingData> d_pending;
 };
 
 extern AuthZoneCache g_zoneCache;

diff --git a/pdns/cachecleaner.hh b/pdns/cachecleaner.hh
index 55ddd48fc125ff8e2421d59d4b89cabbf610a42b..75340d388e4f4a50a0d0985f69212fa66b054024 100644 (file)
--- a/pdns/cachecleaner.hh
+++ b/pdns/cachecleaner.hh
@@ -102,12 +102,12 @@ template <typename S, typename T> uint64_t pruneLockedCollectionsVector(std::vec
   time_t now = time(nullptr);
 
   for(auto& mc : maps) {
-    WriteLock wl(&mc.d_mut);
+    auto map = mc.d_map.write_lock();
 
-    uint64_t lookAt = (mc.d_map.size() + 9) / 10; // Look at 10% of this shard
+    uint64_t lookAt = (map->size() + 9) / 10; // Look at 10% of this shard
     uint64_t erased = 0;
 
-    auto& sidx = boost::multi_index::get<S>(mc.d_map);
+    auto& sidx = boost::multi_index::get<S>(*map);
     for(auto i = sidx.begin(); i != sidx.end() && lookAt > 0; lookAt--) {
       if(i->ttd < now) {
         i = sidx.erase(i);
@@ -203,9 +203,9 @@ template <typename T> uint64_t purgeLockedCollectionsVector(std::vector<T>& maps
   uint64_t delcount=0;
 
   for(auto& mc : maps) {
-    WriteLock wl(&mc.d_mut);
-    delcount += mc.d_map.size();
-    mc.d_map.clear();
+    auto map = mc.d_map.write_lock();
+    delcount += map->size();
+    map->clear();
   }
 
   return delcount;
@@ -218,8 +218,8 @@ template <typename N, typename T> uint64_t purgeLockedCollectionsVector(std::vec
   prefix.resize(prefix.size()-1);
   DNSName dprefix(prefix);
   for(auto& mc : maps) {
-    WriteLock wl(&mc.d_mut);
-    auto& idx = boost::multi_index::get<N>(mc.d_map);
+    auto map = mc.d_map.write_lock();
+    auto& idx = boost::multi_index::get<N>(*map);
     auto iter = idx.lower_bound(dprefix);
     auto start = iter;
 
@@ -238,8 +238,8 @@ template <typename N, typename T> uint64_t purgeLockedCollectionsVector(std::vec
 template <typename N, typename T> uint64_t purgeExactLockedCollection(T& mc, const DNSName& qname)
 {
   uint64_t delcount=0;
-  WriteLock wl(&mc.d_mut);
-  auto& idx = boost::multi_index::get<N>(mc.d_map);
+  auto map = mc.d_map.write_lock();
+  auto& idx = boost::multi_index::get<N>(*map);
   auto range = idx.equal_range(qname);
   if(range.first != range.second) {
     delcount += distance(range.first, range.second);

diff --git a/pdns/lock.hh b/pdns/lock.hh
index 1b60d46a5e315bbba8b7a6498407a3983b88dd57..70fc9e764a8911a8a543863ef367d6a2572015b0 100644 (file)
--- a/pdns/lock.hh
+++ b/pdns/lock.hh
@@ -290,6 +290,11 @@ public:
     return LockGuardedHolder<T>(d_value, d_mutex);
   }
 
+  LockGuardedHolder<const T> read_only_lock() const
+  {
+    return LockGuardedHolder<const T>(d_value, d_mutex);
+  }
+
 private:
   std::mutex d_mutex;
   T d_value;

diff --git a/pdns/statbag.cc b/pdns/statbag.cc
index bbacc27b4db5b822e5b4d93a8c60f3b1df35ec33..4e9f70bd469ac0b2f01afec2965226e5301649c1 100644 (file)
--- a/pdns/statbag.cc
+++ b/pdns/statbag.cc
@@ -217,7 +217,7 @@ template<typename T, typename Comp>
 vector<pair<T, unsigned int> >StatRing<T,Comp>::get() const
 {
   map<T,unsigned int, Comp> res;
-  for(typename boost::circular_buffer<T>::const_iterator i=d_items.begin();i!=d_items.end();++i) {
+  for (typename boost::circular_buffer<T>::const_iterator i = d_items.begin(); i != d_items.end(); ++i) {
     res[*i]++;
   }
   

diff --git a/pdns/statbag.hh b/pdns/statbag.hh
index e0aa3c04e6d9b0e82e2578bcdb4cfbe798be3873..acdc3832303b0d09d2a1bb5383b792924be6b909 100644 (file)
--- a/pdns/statbag.hh
+++ b/pdns/statbag.hh
@@ -20,7 +20,6 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  */
 #pragma once
-#include <pthread.h>
 #include <map>
 #include <functional>
 #include <string>