From 07d4834a60067aa75759afddf02979c2592322b0 Mon Sep 17 00:00:00 2001
From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Tue, 14 Sep 2021 09:51:49 +0200
Subject: [PATCH] dnsdist: Convert the TLS session cache to LockGuarded

---
 pdns/dnsdistdist/dnsdist-nghttp2.hh       |  3 +++
 pdns/dnsdistdist/dnsdist-session-cache.cc | 22 +++++++++++-----------
 pdns/dnsdistdist/dnsdist-session-cache.hh | 15 ++++++++++-----
 3 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/pdns/dnsdistdist/dnsdist-nghttp2.hh b/pdns/dnsdistdist/dnsdist-nghttp2.hh
index d36c9959f0..f57e4e46fa 100644
--- a/pdns/dnsdistdist/dnsdist-nghttp2.hh
+++ b/pdns/dnsdistdist/dnsdist-nghttp2.hh
@@ -47,6 +47,9 @@ private:
   struct DoHWorkerThread;
 
   std::mutex d_mutex;
+  /* we only alter that vector at configuration time, and then
+     it is never modified at runtime, so we don't take a lock
+     after the configuration phase */
   std::vector<DoHWorkerThread> d_clientThreads;
   pdns::stat_t d_pos{0};
   uint64_t d_numberOfThreads{0};
diff --git a/pdns/dnsdistdist/dnsdist-session-cache.cc b/pdns/dnsdistdist/dnsdist-session-cache.cc
index 18d03d8d84..b227efd2c6 100644
--- a/pdns/dnsdistdist/dnsdist-session-cache.cc
+++ b/pdns/dnsdistdist/dnsdist-session-cache.cc
@@ -27,31 +27,31 @@ time_t TLSSessionCache::s_cleanupDelay{60};
 time_t TLSSessionCache::s_sessionValidity{600};
 uint16_t TLSSessionCache::s_maxSessionsPerBackend{20};
 
-void TLSSessionCache::cleanup(time_t now, const std::lock_guard<std::mutex>& lock)
+void TLSSessionCache::cleanup(time_t now, LockGuardedHolder<TLSSessionCache::CacheData>& data)
 {
   time_t cutOff = now + s_sessionValidity;
 
-  for (auto it = d_sessions.begin(); it != d_sessions.end();) {
+  for (auto it = data->d_sessions.begin(); it != data->d_sessions.end();) {
     if (it->second.d_lastUsed > cutOff || it->second.d_sessions.size() == 0) {
-      it = d_sessions.erase(it);
+      it = data->d_sessions.erase(it);
     }
     else {
       ++it;
     }
   }
 
-  d_nextCleanup = now + s_cleanupDelay;
+  data->d_nextCleanup = now + s_cleanupDelay;
 }
 
 void TLSSessionCache::putSessions(const boost::uuids::uuid& backendID, time_t now, std::vector<std::unique_ptr<TLSSession>>&& sessions)
 {
-  std::lock_guard<decltype(d_lock)> lock(d_lock);
-  if (d_nextCleanup == 0 || now > d_nextCleanup) {
-    cleanup(now, lock);
+  auto data = d_data.lock();
+  if (data->d_nextCleanup == 0 || now > data->d_nextCleanup) {
+    cleanup(now, data);
   }
 
   for (auto& session : sessions) {
-    auto& entry = d_sessions[backendID];
+    auto& entry = data->d_sessions[backendID];
     if (entry.d_sessions.size() >= s_maxSessionsPerBackend) {
       entry.d_sessions.pop_back();
     }
@@ -61,9 +61,9 @@ void TLSSessionCache::putSessions(const boost::uuids::uuid& backendID, time_t no
 
 std::unique_ptr<TLSSession> TLSSessionCache::getSession(const boost::uuids::uuid& backendID, time_t now)
 {
-  std::lock_guard<decltype(d_lock)> lock(d_lock);
-  auto it = d_sessions.find(backendID);
-  if (it == d_sessions.end()) {
+  auto data = d_data.lock();
+  auto it = data->d_sessions.find(backendID);
+  if (it == data->d_sessions.end()) {
     return nullptr;
   }
 
diff --git a/pdns/dnsdistdist/dnsdist-session-cache.hh b/pdns/dnsdistdist/dnsdist-session-cache.hh
index 0c523b1b6a..47d76bc478 100644
--- a/pdns/dnsdistdist/dnsdist-session-cache.hh
+++ b/pdns/dnsdistdist/dnsdist-session-cache.hh
@@ -24,6 +24,7 @@
 #include <deque>
 #include <map>
 
+#include "lock.hh"
 #include "tcpiohandler.hh"
 #include "uuid-utils.hh"
 
@@ -33,7 +34,6 @@ public:
   TLSSessionCache()
   {
   }
-  void cleanup(time_t now, const std::lock_guard<std::mutex>& lock);
 
   void putSessions(const boost::uuids::uuid& backendID, time_t now, std::vector<std::unique_ptr<TLSSession>>&& sessions);
   std::unique_ptr<TLSSession> getSession(const boost::uuids::uuid& backendID, time_t now);
@@ -64,10 +64,15 @@ private:
     time_t d_lastUsed{0};
   };
 
-  std::map<boost::uuids::uuid, BackendEntry> d_sessions;
-  // do we need to shard this?
-  std::mutex d_lock;
-  time_t d_nextCleanup{0};
+  struct CacheData
+  {
+    // do we need to shard this?
+    std::map<boost::uuids::uuid, BackendEntry> d_sessions;
+    time_t d_nextCleanup{0};
+  };
+  LockGuarded<CacheData> d_data;
+
+  void cleanup(time_t now, LockGuardedHolder<CacheData>& data);
 };
 
 extern TLSSessionCache g_sessionCache;
-- 
2.47.2