rec: Move the record caches to LockGuarded (WIP: size() should not need a lock)

diff --git a/pdns/cachecleaner.hh b/pdns/cachecleaner.hh
index 4962ff63576be733e34eb1d119dd0f4741b9fd5d..fecdbc1ab2d2a44f916952b06395b9c45e0dab0b 100644 (file)
--- a/pdns/cachecleaner.hh
+++ b/pdns/cachecleaner.hh
@@ -21,7 +21,6 @@
  */
 #pragma once
 
-#include <mutex>
 #include <boost/multi_index_container.hpp>
 
 #include "dnsname.hh"
@@ -50,7 +49,6 @@ template <typename S, typename C, typename T> void pruneCollection(C& container,
 
   for (auto iter = sidx.begin(); iter != sidx.end() && tried < lookAt ; ++tried) {
     if (iter->getTTD() < now) {
-      container.preRemoval(*iter);
       iter = sidx.erase(iter);
       erased++;
     }
@@ -72,7 +70,6 @@ template <typename S, typename C, typename T> void pruneCollection(C& container,
   // just lob it off from the beginning
   auto iter = sidx.begin();
   for (size_t i = 0; i < toTrim && iter != sidx.end(); i++) {
-    container.preRemoval(*iter);
     iter = sidx.erase(iter);
   }
 }
@@ -145,17 +142,17 @@ template <typename S, typename C, typename T> uint64_t pruneMutexCollectionsVect
   if (maps_size == 0)
       return 0;
 
-  for (auto& mc : maps) {
-    const typename C::lock l(mc);
-    mc.invalidate();
-    auto& sidx = boost::multi_index::get<S>(mc.d_map);
+  for (auto& lockGuardedMap : maps) {
+    auto mc = C::lock(lockGuardedMap);
+    mc->invalidate();
+    auto& sidx = boost::multi_index::get<S>(mc->d_map);
     uint64_t erased = 0, lookedAt = 0;
     for (auto i = sidx.begin(); i != sidx.end(); lookedAt++) {
       if (i->getTTD() < now) {
-        container.preRemoval(*i);
+        container.preRemoval(*mc, *i);
         i = sidx.erase(i);
         erased++;
-        mc.d_entriesCount--;
+        mc->d_entriesCount--;
       } else {
         ++i;
       }
@@ -179,15 +176,15 @@ template <typename S, typename C, typename T> uint64_t pruneMutexCollectionsVect
 
     while (true) {
     size_t pershard = toTrim / maps_size + 1;
-    for (auto& mc : maps) {
-      const typename C::lock l(mc);
-      mc.invalidate();
-      auto& sidx = boost::multi_index::get<S>(mc.d_map);
+    for (auto& lockGuardedMap : maps) {
+      auto mc = C::lock(lockGuardedMap);
+      mc->invalidate();
+      auto& sidx = boost::multi_index::get<S>(mc->d_map);
       size_t removed = 0;
       for (auto i = sidx.begin(); i != sidx.end() && removed < pershard; removed++) {
-        container.preRemoval(*i);
+        container.preRemoval(*mc, *i);
         i = sidx.erase(i);
-        mc.d_entriesCount--;
+        mc->d_entriesCount--;
         totErased++;
         toTrim--;
         if (toTrim == 0) {

diff --git a/pdns/dnsseckeeper.hh b/pdns/dnsseckeeper.hh
index cf821a6d076e49316277ac6ab279e4b16cde1a96..edae2152f7a3954b453afda26277a7abbec49fe2 100644 (file)
--- a/pdns/dnsseckeeper.hh
+++ b/pdns/dnsseckeeper.hh
@@ -304,14 +304,6 @@ private:
   static AtomicCounter s_ops;
   static time_t s_last_prune;
   static size_t s_maxEntries;
-
-public:
-  void preRemoval(const KeyCacheEntry&)
-  {
-  }
-  void preRemoval(const METACacheEntry&)
-  {
-  }
 };
 
 class DNSPacket;

diff --git a/pdns/lock.hh b/pdns/lock.hh
index fbf94bf8369e79f547d5242d78ba0bd5e34044cf..d49bb7a3f73da3f400d05d395ee133f4cc529e92 100644 (file)
--- a/pdns/lock.hh
+++ b/pdns/lock.hh
@@ -200,6 +200,11 @@ public:
     return d_lock.owns_lock();
   }
 
+  void lock()
+  {
+    d_lock.lock();
+  }
+
 private:
   std::unique_lock<std::mutex> d_lock;
   T& d_value;

diff --git a/pdns/recpacketcache.hh b/pdns/recpacketcache.hh
index de6e3c719bf782e82fea8958385ac658cfb2c623..7ef8a99e9d3e0681cba0b82a973235ae8942d206 100644 (file)
--- a/pdns/recpacketcache.hh
+++ b/pdns/recpacketcache.hh
@@ -128,9 +128,4 @@ private:
 
   static bool qrMatch(const packetCache_t::index<HashTag>::type::iterator& iter, const std::string& queryPacket, const DNSName& qname, uint16_t qtype, uint16_t qclass);
   bool checkResponseMatches(std::pair<packetCache_t::index<HashTag>::type::iterator, packetCache_t::index<HashTag>::type::iterator> range, const std::string& queryPacket, const DNSName& qname, uint16_t qtype, uint16_t qclass, time_t now, std::string* responsePacket, uint32_t* age, vState* valState, OptPBData* pbdata);
-
-public:
-  void preRemoval(const Entry& entry)
-  {
-  }
 };

diff --git a/pdns/recursor_cache.cc b/pdns/recursor_cache.cc
index 76e702fba1065960af59f6c17eb840ea3d1d24fe..972b6010a6607cfc0a33092b1c2b25a9341146ba 100644 (file)
--- a/pdns/recursor_cache.cc
+++ b/pdns/recursor_cache.cc
@@ -22,8 +22,9 @@ MemRecursorCache::MemRecursorCache(size_t mapsCount) : d_maps(mapsCount)
 size_t MemRecursorCache::size()
 {
   size_t count = 0;
-  for (auto& map : d_maps) {
-    count += map.d_entriesCount;
+  for (auto& lockGuardedMap : d_maps) {
+    auto map = lock(lockGuardedMap);
+    count += map->d_entriesCount;
   }
   return count;
 }
@@ -31,10 +32,10 @@ size_t MemRecursorCache::size()
 pair<uint64_t,uint64_t> MemRecursorCache::stats()
 {
   uint64_t c = 0, a = 0;
-  for (auto& map : d_maps) {
-    const lock l(map);
-    c += map.d_contended_count;
-    a += map.d_acquired_count;
+  for (auto& lockGuardedMap : d_maps) {
+    auto map = lock(lockGuardedMap);
+    c += map->d_contended_count;
+    a += map->d_acquired_count;
   }
   return pair<uint64_t,uint64_t>(c, a);
 }
@@ -44,8 +45,8 @@ size_t MemRecursorCache::ecsIndexSize()
   // XXX!
   size_t count = 0;
   for (auto& map : d_maps) {
-    const lock l(map);
-    count += map.d_ecsIndex.size();
+    auto m = lock(map);
+    count += m->d_ecsIndex.size();
   }
   return count;
 }
@@ -55,8 +56,8 @@ size_t MemRecursorCache::bytes()
 {
   size_t ret = 0;
   for (auto& map : d_maps) {
-    const lock l(map);
-    for (const auto& i : map.d_map) {
+    auto m = lock(map);
+    for (const auto& i : m->d_map) {
       ret += sizeof(struct CacheEntry);
       ret += i.d_qname.toString().length();
       for (const auto& record : i.d_records) {
@@ -272,22 +273,22 @@ time_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType qt, b
     *wasAuth = true;
   }
 
-  auto& map = getMap(qname);
-  const lock l(map);
+  auto& lockGuardedMap = getMap(qname);
+  auto map = lock(lockGuardedMap);
 
   /* If we don't have any netmask-specific entries at all, let's just skip this
      to be able to use the nice d_cachecache hack. */
-  if (qtype != QType::ANY && !map.d_ecsIndex.empty() && !routingTag) {
+  if (qtype != QType::ANY && !map->d_ecsIndex.empty() && !routingTag) {
     if (qtype == QType::ADDR) {
       time_t ret = -1;
 
-      auto entryA = getEntryUsingECSIndex(map, now, qname, QType::A, requireAuth, who);
-      if (entryA != map.d_map.end()) {
-        ret = handleHit(map, entryA, qname, origTTL, res, signatures, authorityRecs, variable, cachedState, wasAuth, fromAuthZone);
+      auto entryA = getEntryUsingECSIndex(*map, now, qname, QType::A, requireAuth, who);
+      if (entryA != map->d_map.end()) {
+        ret = handleHit(*map, entryA, qname, origTTL, res, signatures, authorityRecs, variable, cachedState, wasAuth, fromAuthZone);
       }
-      auto entryAAAA = getEntryUsingECSIndex(map, now, qname, QType::AAAA, requireAuth, who);
-      if (entryAAAA != map.d_map.end()) {
-        time_t ttdAAAA = handleHit(map, entryAAAA, qname, origTTL, res, signatures, authorityRecs, variable, cachedState, wasAuth, fromAuthZone);
+      auto entryAAAA = getEntryUsingECSIndex(*map, now, qname, QType::AAAA, requireAuth, who);
+      if (entryAAAA != map->d_map.end()) {
+        time_t ttdAAAA = handleHit(*map, entryAAAA, qname, origTTL, res, signatures, authorityRecs, variable, cachedState, wasAuth, fromAuthZone);
         if (ret > 0) {
           ret = std::min(ret, ttdAAAA);
         } else {
@@ -302,9 +303,9 @@ time_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType qt, b
       return ret > 0 ? (ret - now) : ret;
     }
     else {
-      auto entry = getEntryUsingECSIndex(map, now, qname, qtype, requireAuth, who);
-      if (entry != map.d_map.end()) {
-        time_t ret = handleHit(map, entry, qname, origTTL, res, signatures, authorityRecs, variable, cachedState, wasAuth, fromAuthZone);
+      auto entry = getEntryUsingECSIndex(*map, now, qname, qtype, requireAuth, who);
+      if (entry != map->d_map.end()) {
+        time_t ret = handleHit(*map, entry, qname, origTTL, res, signatures, authorityRecs, variable, cachedState, wasAuth, fromAuthZone);
         if (state && cachedState) {
           *state = *cachedState;
         }
@@ -315,17 +316,17 @@ time_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType qt, b
   }
 
   if (routingTag) {
-    auto entries = getEntries(map, qname, qt, routingTag);
+    auto entries = getEntries(*map, qname, qt, routingTag);
     bool found = false;
     time_t ttd;
 
     if (entries.first != entries.second) {
       OrderedTagIterator_t firstIndexIterator;
       for (auto i=entries.first; i != entries.second; ++i) {
-        firstIndexIterator = map.d_map.project<OrderedTag>(i);
+        firstIndexIterator = map->d_map.project<OrderedTag>(i);
 
         if (i->d_ttd <= now) {
-          moveCacheItemToFront<SequencedTag>(map.d_map, firstIndexIterator);
+          moveCacheItemToFront<SequencedTag>(map->d_map, firstIndexIterator);
           continue;
         }
 
@@ -333,7 +334,7 @@ time_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType qt, b
           continue;
         }
         found = true;
-        ttd = handleHit(map, firstIndexIterator, qname, origTTL, res, signatures, authorityRecs, variable, cachedState, wasAuth, fromAuthZone);
+        ttd = handleHit(*map, firstIndexIterator, qname, origTTL, res, signatures, authorityRecs, variable, cachedState, wasAuth, fromAuthZone);
 
         if (qt != QType::ANY && qt != QType::ADDR) { // normally if we have a hit, we are done
           break;
@@ -350,7 +351,7 @@ time_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType qt, b
     }
   }
   // Try (again) without tag
-  auto entries = getEntries(map, qname, qt, boost::none);
+  auto entries = getEntries(*map, qname, qt, boost::none);
 
   if (entries.first != entries.second) {
     OrderedTagIterator_t firstIndexIterator;
@@ -358,10 +359,10 @@ time_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType qt, b
     time_t ttd;
 
     for (auto i=entries.first; i != entries.second; ++i) {
-      firstIndexIterator = map.d_map.project<OrderedTag>(i);
+      firstIndexIterator = map->d_map.project<OrderedTag>(i);
 
       if (i->d_ttd <= now) {
-        moveCacheItemToFront<SequencedTag>(map.d_map, firstIndexIterator);
+        moveCacheItemToFront<SequencedTag>(map->d_map, firstIndexIterator);
         continue;
       }
 
@@ -370,7 +371,7 @@ time_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType qt, b
       }
 
       found = true;
-      ttd = handleHit(map, firstIndexIterator, qname, origTTL, res, signatures, authorityRecs, variable, cachedState, wasAuth, fromAuthZone);
+      ttd = handleHit(*map, firstIndexIterator, qname, origTTL, res, signatures, authorityRecs, variable, cachedState, wasAuth, fromAuthZone);
 
       if (qt != QType::ANY && qt != QType::ADDR) { // normally if we have a hit, we are done
         break;
@@ -388,10 +389,10 @@ time_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType qt, b
 
 void MemRecursorCache::replace(time_t now, const DNSName &qname, const QType qt, const vector<DNSRecord>& content, const vector<shared_ptr<RRSIGRecordContent>>& signatures, const std::vector<std::shared_ptr<DNSRecord>>& authorityRecs, bool auth, const DNSName& authZone, boost::optional<Netmask> ednsmask, const OptTag& routingTag, vState state, boost::optional<ComboAddress> from)
 {
-  auto& map = getMap(qname);
-  const lock l(map);
+  auto& lockGuardedMap = getMap(qname);
+  auto map = lock(lockGuardedMap);
 
-  map.d_cachecachevalid = false;
+  map->d_cachecachevalid = false;
   if (ednsmask) {
     ednsmask = ednsmask->getNormalized();
   }
@@ -400,10 +401,10 @@ void MemRecursorCache::replace(time_t now, const DNSName &qname, const QType qt,
   // We only store an ednsmask if we do not have a tag and we do have a mask.
   auto key = boost::make_tuple(qname, qt.getCode(), ednsmask ? routingTag : boost::none, (ednsmask && !routingTag) ? *ednsmask : Netmask());
   bool isNew = false;
-  cache_t::iterator stored = map.d_map.find(key);
-  if (stored == map.d_map.end()) {
-    stored = map.d_map.insert(CacheEntry(key, auth)).first;
-    map.d_entriesCount++;
+  cache_t::iterator stored = map->d_map.find(key);
+  if (stored == map->d_map.end()) {
+    stored = map->d_map.insert(CacheEntry(key, auth)).first;
+    map->d_entriesCount++;
     isNew = true;
   }
 
@@ -416,9 +417,9 @@ void MemRecursorCache::replace(time_t now, const DNSName &qname, const QType qt,
     /* don't bother building an ecsIndex if we don't have any netmask-specific entries */
     if (!routingTag && ednsmask && !ednsmask->empty()) {
       auto ecsIndexKey = boost::make_tuple(qname, qt.getCode());
-      auto ecsIndex = map.d_ecsIndex.find(ecsIndexKey);
-      if (ecsIndex == map.d_ecsIndex.end()) {
-        ecsIndex = map.d_ecsIndex.insert(ECSIndexEntry(qname, qt.getCode())).first;
+      auto ecsIndex = map->d_ecsIndex.find(ecsIndexKey);
+      if (ecsIndex == map->d_ecsIndex.end()) {
+        ecsIndex = map->d_ecsIndex.insert(ECSIndexEntry(qname, qt.getCode())).first;
       }
       ecsIndex->addMask(*ednsmask);
     }
@@ -480,10 +481,10 @@ void MemRecursorCache::replace(time_t now, const DNSName &qname, const QType qt,
   }
 
   if (!isNew) {
-    moveCacheItemToBack<SequencedTag>(map.d_map, stored);
+    moveCacheItemToBack<SequencedTag>(map->d_map, stored);
   }
   ce.d_submitted = false;
-  map.d_map.replace(stored, ce);
+  map->d_map.replace(stored, ce);
 }
 
 size_t MemRecursorCache::doWipeCache(const DNSName& name, bool sub, const QType qtype)
@@ -491,50 +492,50 @@ size_t MemRecursorCache::doWipeCache(const DNSName& name, bool sub, const QType
   size_t count = 0;
 
   if (!sub) {
-    auto& map = getMap(name);
-    const lock l(map);
-    map.d_cachecachevalid = false;
-    auto& idx = map.d_map.get<OrderedTag>();
+    auto& lockGuardedMap = getMap(name);
+    auto map = lock(lockGuardedMap);
+    map->d_cachecachevalid = false;
+    auto& idx = map->d_map.get<OrderedTag>();
     auto range = idx.equal_range(name);
     auto i = range.first;
     while (i != range.second) {
       if (i->d_qtype == qtype || qtype == 0xffff) {
         i = idx.erase(i);
         count++;
-        map.d_entriesCount--;
+        map->d_entriesCount--;
       } else {
         ++i;
       }
     }
 
     if (qtype == 0xffff) {
-      auto& ecsIdx = map.d_ecsIndex.get<OrderedTag>();
+      auto& ecsIdx = map->d_ecsIndex.get<OrderedTag>();
       auto ecsIndexRange = ecsIdx.equal_range(name);
       ecsIdx.erase(ecsIndexRange.first, ecsIndexRange.second);
     }
     else {
-      auto& ecsIdx = map.d_ecsIndex.get<HashedTag>();
+      auto& ecsIdx = map->d_ecsIndex.get<HashedTag>();
       auto ecsIndexRange = ecsIdx.equal_range(tie(name, qtype));
       ecsIdx.erase(ecsIndexRange.first, ecsIndexRange.second);
     }
   }
   else {
-    for (auto& map : d_maps) {
-      const lock l(map);
-      map.d_cachecachevalid = false;
-      auto& idx = map.d_map.get<OrderedTag>();
+    for (auto& lockGuardedMap : d_maps) {
+      auto map = lock(lockGuardedMap);
+      map->d_cachecachevalid = false;
+      auto& idx = map->d_map.get<OrderedTag>();
       for (auto i = idx.lower_bound(name); i != idx.end(); ) {
         if (!i->d_qname.isPartOf(name))
           break;
         if (i->d_qtype == qtype || qtype == 0xffff) {
           count++;
           i = idx.erase(i);
-          map.d_entriesCount--;
+          map->d_entriesCount--;
         } else {
           ++i;
         }
       }
-      auto& ecsIdx = map.d_ecsIndex.get<OrderedTag>();
+      auto& ecsIdx = map->d_ecsIndex.get<OrderedTag>();
       for (auto i = ecsIdx.lower_bound(name); i != ecsIdx.end(); ) {
         if (!i->d_qname.isPartOf(name))
           break;
@@ -552,10 +553,10 @@ size_t MemRecursorCache::doWipeCache(const DNSName& name, bool sub, const QType
 // Name should be doLimitTime or so
 bool MemRecursorCache::doAgeCache(time_t now, const DNSName& name, const QType qtype, uint32_t newTTL)
 {
-  auto& map = getMap(name);
-  const lock l(map);
-  cache_t::iterator iter = map.d_map.find(tie(name, qtype));
-  if (iter == map.d_map.end()) {
+  auto& lockGuardedMap = getMap(name);
+  auto map = lock(lockGuardedMap);
+  cache_t::iterator iter = map->d_map.find(tie(name, qtype));
+  if (iter == map->d_map.end()) {
     return false;
   }
 
@@ -565,13 +566,13 @@ bool MemRecursorCache::doAgeCache(time_t now, const DNSName& name, const QType q
 
   uint32_t maxTTL = static_cast<uint32_t>(ce.d_ttd - now);
   if (maxTTL > newTTL) {
-    map.d_cachecachevalid = false;
+    map->d_cachecachevalid = false;
 
     time_t newTTD = now + newTTL;
 
     if (ce.d_ttd > newTTD) {
       ce.d_ttd = newTTD;
-      map.d_map.replace(iter, ce);
+      map->d_map.replace(iter, ce);
     }
     return true;
   }
@@ -588,13 +589,13 @@ bool MemRecursorCache::updateValidationStatus(time_t now, const DNSName &qname,
     throw std::runtime_error("Trying to update the DNSSEC validation status of several (via ADDR) records for " + qname.toLogString());
   }
 
-  auto& map = getMap(qname);
-  const lock l(map);
+  auto& lockGuardedMap = getMap(qname);
+  auto map = lock(lockGuardedMap);
 
   bool updated = false;
-  if (!map.d_ecsIndex.empty() && !routingTag) {
-    auto entry = getEntryUsingECSIndex(map, now, qname, qtype, requireAuth, who);
-    if (entry == map.d_map.end()) {
+  if (!map->d_ecsIndex.empty() && !routingTag) {
+    auto entry = getEntryUsingECSIndex(*map, now, qname, qtype, requireAuth, who);
+    if (entry == map->d_map.end()) {
       return false;
     }
 
@@ -605,10 +606,10 @@ bool MemRecursorCache::updateValidationStatus(time_t now, const DNSName &qname,
     return true;
   }
 
-  auto entries = getEntries(map, qname, qt, routingTag);
+  auto entries = getEntries(*map, qname, qt, routingTag);
 
   for(auto i = entries.first; i != entries.second; ++i) {
-    auto firstIndexIterator = map.d_map.project<OrderedTag>(i);
+    auto firstIndexIterator = map->d_map.project<OrderedTag>(i);
 
     if (!entryMatches(firstIndexIterator, qtype, requireAuth, who)) {
       continue;
@@ -641,9 +642,9 @@ uint64_t MemRecursorCache::doDump(int fd)
   fprintf(fp.get(), "; main record cache dump follows\n;\n");
   uint64_t count = 0;
 
-  for (auto& map : d_maps) {
-    const lock l(map);
-    const auto& sidx = map.d_map.get<SequencedTag>();
+  for (auto& lockGuardedMap : d_maps) {
+    auto map = lock(lockGuardedMap);
+    const auto& sidx = map->d_map.get<SequencedTag>();
 
     time_t now = time(nullptr);
     for (const auto& i : sidx) {

diff --git a/pdns/recursor_cache.hh b/pdns/recursor_cache.hh
index 0fe249f1708bc29500e236d1347d20b22ddb0963..dbe74354004efa7442ed8aa52ae98f97c3e0dbf9 100644 (file)
--- a/pdns/recursor_cache.hh
+++ b/pdns/recursor_cache.hh
@@ -22,7 +22,6 @@
 #pragma once
 #include <string>
 #include <set>
-#include <mutex>
 #include "dns.hh"
 #include "qtype.hh"
 #include "misc.hh"
@@ -38,6 +37,7 @@
 #include <boost/multi_index/sequenced_index.hpp>
 #include <boost/version.hpp>
 #include "iputils.hh"
+#include "lock.hh"
 #include "validate.hh"
 #undef max
 
@@ -212,7 +212,6 @@ private:
     DNSName d_cachedqname;
     OptTag d_cachedrtag;
     Entries d_cachecache;
-    std::mutex mutex;
     bool d_cachecachevalid{false};
     std::atomic<uint64_t> d_entriesCount{0};
     uint64_t d_contended_count{0};
@@ -224,10 +223,10 @@ private:
     }
   };
 
-  vector<MapCombo> d_maps;
-  MapCombo& getMap(const DNSName &qname)
+  vector<LockGuarded<MapCombo>> d_maps;
+  LockGuarded<MapCombo>& getMap(const DNSName &qname)
   {
-    return d_maps[qname.hash() % d_maps.size()];
+    return d_maps.at(qname.hash() % d_maps.size());
   }
 
   static time_t fakeTTD(OrderedTagIterator_t& entry, const DNSName& qname, QType qtype, time_t ret, time_t now, uint32_t origTTL, bool refresh);
@@ -239,30 +238,24 @@ private:
   time_t handleHit(MapCombo& map, OrderedTagIterator_t& entry, const DNSName& qname, uint32_t& origTTL, vector<DNSRecord>* res, vector<std::shared_ptr<RRSIGRecordContent>>* signatures, std::vector<std::shared_ptr<DNSRecord>>* authorityRecs, bool* variable, boost::optional<vState>& state, bool* wasAuth, DNSName* authZone);
 
 public:
-  struct lock {
-    lock(MapCombo& map) : m(map.mutex)
-    {
-      if (!m.try_lock()) {
-        m.lock();
-        map.d_contended_count++;
-      }
-      map.d_acquired_count++;
-    }
-    ~lock() {
-      m.unlock();
+  static LockGuardedTryHolder<MapCombo> lock(LockGuarded<MapCombo>& map)
+  {
+    auto locked = map.try_lock();
+    if (!locked.owns_lock()) {
+      locked.lock();
+      locked->d_contended_count++;
     }
-  private:
-    std::mutex &m;
-  };
+    ++locked->d_acquired_count;
+    return locked;
+  }
 
-  void preRemoval(const CacheEntry& entry)
+  void preRemoval(MapCombo& map, const CacheEntry& entry)
   {
     if (entry.d_netmask.empty()) {
       return;
     }
 
     auto key = tie(entry.d_qname, entry.d_qtype);
-    auto& map = getMap(entry.d_qname);
     auto ecsIndexEntry = map.d_ecsIndex.find(key);
     if (ecsIndexEntry != map.d_ecsIndex.end()) {
       ecsIndexEntry->removeNetmask(entry.d_netmask);

diff --git a/pdns/recursordist/negcache.cc b/pdns/recursordist/negcache.cc
index 53dcb6f76abbd3a4fe390512f341d54d92b23581..717ce75b90bc2febd28fa54c628170de59e0e16d 100644 (file)
--- a/pdns/recursordist/negcache.cc
+++ b/pdns/recursordist/negcache.cc
@@ -31,11 +31,12 @@ NegCache::NegCache(size_t mapsCount) :
 {
 }
 
-size_t NegCache::size() const
+size_t NegCache::size()
 {
   size_t count = 0;
-  for (const auto& map : d_maps) {
-    count += map.d_entriesCount;
+  for (auto& lockGuardedMap : d_maps) {
+    auto map = lock(lockGuardedMap);
+    count += map->d_entriesCount;
   }
   return count;
 }
@@ -59,19 +60,19 @@ bool NegCache::getRootNXTrust(const DNSName& qname, const struct timeval& now, N
   static const QType qtnull(0);
   DNSName lastLabel = qname.getLastLabel();
 
-  auto& map = getMap(lastLabel);
-  const lock l(map);
+  auto& lockGuardedMap = getMap(lastLabel);
+  auto map = lock(lockGuardedMap);
 
-  negcache_t::const_iterator ni = map.d_map.find(tie(lastLabel, qtnull));
+  negcache_t::const_iterator ni = map->d_map.find(tie(lastLabel, qtnull));
 
-  while (ni != map.d_map.end() && ni->d_name == lastLabel && ni->d_auth.isRoot() && ni->d_qtype == qtnull) {
+  while (ni != map->d_map.end() && ni->d_name == lastLabel && ni->d_auth.isRoot() && ni->d_qtype == qtnull) {
     // We have something
     if (now.tv_sec < ni->d_ttd) {
       ne = *ni;
-      moveCacheItemToBack<SequenceTag>(map.d_map, ni);
+      moveCacheItemToBack<SequenceTag>(map->d_map, ni);
       return true;
     }
-    moveCacheItemToFront<SequenceTag>(map.d_map, ni);
+    moveCacheItemToFront<SequenceTag>(map->d_map, ni);
     ++ni;
   }
   return false;
@@ -88,10 +89,10 @@ bool NegCache::getRootNXTrust(const DNSName& qname, const struct timeval& now, N
  */
 bool NegCache::get(const DNSName& qname, const QType& qtype, const struct timeval& now, NegCacheEntry& ne, bool typeMustMatch)
 {
-  auto& map = getMap(qname);
-  const lock l(map);
+  auto& lockGuardedMap = getMap(qname);
+  auto map = lock(lockGuardedMap);
 
-  const auto& idx = map.d_map.get<NegCacheEntry>();
+  const auto& idx = map->d_map.get<NegCacheEntry>();
   auto range = idx.equal_range(qname);
   auto ni = range.first;
 
@@ -99,16 +100,16 @@ bool NegCache::get(const DNSName& qname, const QType& qtype, const struct timeva
     // We have an entry
     if ((!typeMustMatch && ni->d_qtype.getCode() == 0) || ni->d_qtype == qtype) {
       // We match the QType or the whole name is denied
-      auto firstIndexIterator = map.d_map.project<CompositeKey>(ni);
+      auto firstIndexIterator = map->d_map.project<CompositeKey>(ni);
 
       if (now.tv_sec < ni->d_ttd) {
         // Not expired
         ne = *ni;
-        moveCacheItemToBack<SequenceTag>(map.d_map, firstIndexIterator);
+        moveCacheItemToBack<SequenceTag>(map->d_map, firstIndexIterator);
         return true;
       }
       // expired
-      moveCacheItemToFront<SequenceTag>(map.d_map, firstIndexIterator);
+      moveCacheItemToFront<SequenceTag>(map->d_map, firstIndexIterator);
     }
     ++ni;
   }
@@ -122,11 +123,11 @@ bool NegCache::get(const DNSName& qname, const QType& qtype, const struct timeva
  */
 void NegCache::add(const NegCacheEntry& ne)
 {
-  auto& map = getMap(ne.d_name);
-  const lock l(map);
-  bool inserted = lruReplacingInsert<SequenceTag>(map.d_map, ne);
+  auto& lockGuardedMap = getMap(ne.d_name);
+  auto map = lock(lockGuardedMap);
+  bool inserted = lruReplacingInsert<SequenceTag>(map->d_map, ne);
   if (inserted) {
-    map.d_entriesCount++;
+    ++map->d_entriesCount;
   }
 }
 
@@ -139,9 +140,9 @@ void NegCache::add(const NegCacheEntry& ne)
  */
 void NegCache::updateValidationStatus(const DNSName& qname, const QType& qtype, const vState newState, boost::optional<time_t> capTTD)
 {
-  auto& map = getMap(qname);
-  const lock l(map);
-  auto range = map.d_map.equal_range(tie(qname, qtype));
+  auto& lockGuardedMap = getMap(qname);
+  auto map = lock(lockGuardedMap);
+  auto range = map->d_map.equal_range(tie(qname, qtype));
 
   if (range.first != range.second) {
     range.first->d_validationState = newState;
@@ -156,11 +157,11 @@ void NegCache::updateValidationStatus(const DNSName& qname, const QType& qtype,
  *
  * \param qname The name of the entries to be counted
  */
-size_t NegCache::count(const DNSName& qname) const
+size_t NegCache::count(const DNSName& qname)
 {
-  const auto& map = getMap(qname);
-  const lock l(map);
-  return map.d_map.count(tie(qname));
+  auto& lockGuardedMap = getMap(qname);
+  auto map = lock(lockGuardedMap);
+  return map->d_map.count(tie(qname));
 }
 
 /*!
@@ -169,11 +170,11 @@ size_t NegCache::count(const DNSName& qname) const
  * \param qname The name of the entries to be counted
  * \param qtype The type of the entries to be counted
  */
-size_t NegCache::count(const DNSName& qname, const QType qtype) const
+size_t NegCache::count(const DNSName& qname, const QType qtype)
 {
-  const auto& map = getMap(qname);
-  const lock l(map);
-  return map.d_map.count(tie(qname, qtype));
+  auto& lockGuardedMap = getMap(qname);
+  auto map = lock(lockGuardedMap);
+  return map->d_map.count(tie(qname, qtype));
 }
 
 /*!
@@ -187,27 +188,27 @@ size_t NegCache::wipe(const DNSName& name, bool subtree)
 {
   size_t ret = 0;
   if (subtree) {
-    for (auto& m : d_maps) {
-      const lock l(m);
-      for (auto i = m.d_map.lower_bound(tie(name)); i != m.d_map.end();) {
+    for (auto& map : d_maps) {
+      auto m = lock(map);
+      for (auto i = m->d_map.lower_bound(tie(name)); i != m->d_map.end();) {
         if (!i->d_name.isPartOf(name))
           break;
-        i = m.d_map.erase(i);
+        i = m->d_map.erase(i);
         ret++;
-        m.d_entriesCount--;
+        --m->d_entriesCount;
       }
     }
     return ret;
   }
 
-  auto& map = getMap(name);
-  const lock l(map);
-  auto range = map.d_map.equal_range(tie(name));
+  auto& lockGuardedMap = getMap(name);
+  auto map = lock(lockGuardedMap);
+  auto range = map->d_map.equal_range(tie(name));
   auto i = range.first;
   while (i != range.second) {
-    i = map.d_map.erase(i);
+    i = map->d_map.erase(i);
     ret++;
-    map.d_entriesCount--;
+    --map->d_entriesCount;
   }
   return ret;
 }
@@ -217,10 +218,10 @@ size_t NegCache::wipe(const DNSName& name, bool subtree)
  */
 void NegCache::clear()
 {
-  for (auto& m : d_maps) {
-    const lock l(m);
-    m.d_map.clear();
-    m.d_entriesCount = 0;
+  for (auto& map : d_maps) {
+    auto m = lock(map);
+    m->d_map.clear();
+    m->d_entriesCount = 0;
   }
 }
 
@@ -240,13 +241,13 @@ void NegCache::prune(size_t maxEntries)
  *
  * \param fp A pointer to an open FILE object
  */
-size_t NegCache::dumpToFile(FILE* fp, const struct timeval& now) const
+size_t NegCache::dumpToFile(FILE* fp, const struct timeval& now)
 {
   size_t ret = 0;
 
-  for (const auto& m : d_maps) {
-    const lock l(m);
-    auto& sidx = m.d_map.get<SequenceTag>();
+  for (auto& map : d_maps) {
+    auto m = lock(map);
+    auto& sidx = m->d_map.get<SequenceTag>();
     for (const NegCacheEntry& ne : sidx) {
       ret++;
       int64_t ttl = ne.d_ttd - now.tv_sec;

diff --git a/pdns/recursordist/negcache.hh b/pdns/recursordist/negcache.hh
index 7e07b9e08d1878fca997c28608246963360a681e..1a875e5960635212df38aaa54ce0c5149124fab3 100644 (file)
--- a/pdns/recursordist/negcache.hh
+++ b/pdns/recursordist/negcache.hh
@@ -21,7 +21,6 @@
  */
 #pragma once
 
-#include <mutex>
 #include <vector>
 #include <boost/multi_index_container.hpp>
 #include <boost/multi_index/hashed_index.hpp>
@@ -29,6 +28,7 @@
 #include "dnsparser.hh"
 #include "dnsname.hh"
 #include "dns.hh"
+#include "lock.hh"
 #include "validate.hh"
 
 using namespace ::boost::multi_index;
@@ -71,17 +71,13 @@ public:
   void updateValidationStatus(const DNSName& qname, const QType& qtype, const vState newState, boost::optional<time_t> capTTD);
   bool get(const DNSName& qname, const QType& qtype, const struct timeval& now, NegCacheEntry& ne, bool typeMustMatch = false);
   bool getRootNXTrust(const DNSName& qname, const struct timeval& now, NegCacheEntry& ne);
-  size_t count(const DNSName& qname) const;
-  size_t count(const DNSName& qname, const QType qtype) const;
+  size_t count(const DNSName& qname);
+  size_t count(const DNSName& qname, const QType qtype);
   void prune(size_t maxEntries);
   void clear();
-  size_t dumpToFile(FILE* fd, const struct timeval& now) const;
+  size_t dumpToFile(FILE* fd, const struct timeval& now);
   size_t wipe(const DNSName& name, bool subtree = false);
-  size_t size() const;
-
-  void preRemoval(const NegCacheEntry& entry)
-  {
-  }
+  size_t size();
 
 private:
   struct CompositeKey
@@ -111,42 +107,36 @@ private:
     MapCombo(const MapCombo&) = delete;
     MapCombo& operator=(const MapCombo&) = delete;
     negcache_t d_map;
-    mutable std::mutex mutex;
     std::atomic<uint64_t> d_entriesCount{0};
-    mutable uint64_t d_contended_count{0};
-    mutable uint64_t d_acquired_count{0};
+    uint64_t d_contended_count{0};
+    uint64_t d_acquired_count{0};
     void invalidate() {}
   };
 
-  vector<MapCombo> d_maps;
+  vector<LockGuarded<MapCombo>> d_maps;
 
-  MapCombo& getMap(const DNSName& qname)
+  LockGuarded<MapCombo>& getMap(const DNSName& qname)
   {
-    return d_maps[qname.hash() % d_maps.size()];
+    return d_maps.at(qname.hash() % d_maps.size());
   }
-  const MapCombo& getMap(const DNSName& qname) const
+  const LockGuarded<MapCombo>& getMap(const DNSName& qname) const
   {
-    return d_maps[qname.hash() % d_maps.size()];
+    return d_maps.at(qname.hash() % d_maps.size());
   }
 
 public:
-  struct lock
+  static LockGuardedTryHolder<MapCombo> lock(LockGuarded<MapCombo>& map)
   {
-    lock(const MapCombo& map) :
-      m(map.mutex)
-    {
-      if (!m.try_lock()) {
-        m.lock();
-        map.d_contended_count++;
-      }
-      map.d_acquired_count++;
-    }
-    ~lock()
-    {
-      m.unlock();
+    auto locked = map.try_lock();
+    if (!locked.owns_lock()) {
+      locked.lock();
+      locked->d_contended_count++;
     }
+    ++locked->d_acquired_count;
+    return locked;
+  }
 
-  private:
-    std::mutex& m;
-  };
+  void preRemoval(MapCombo& map, const NegCacheEntry& entry)
+  {
+  }
 };