From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Fri, 22 Nov 2024 16:00:15 +0000 (+0100)
Subject: Avoid a data race reported by TSAN
X-Git-Tag: dnsdist-2.0.0-alpha1~95^2~30
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ff0fb2957e79898089b4024f3bf33d3c760e06a1;p=thirdparty%2Fpdns.git

Avoid a data race reported by TSAN
---

diff --git a/pdns/recursordist/rec-main.cc b/pdns/recursordist/rec-main.cc
index f554811583..bd71181692 100644
--- a/pdns/recursordist/rec-main.cc
+++ b/pdns/recursordist/rec-main.cc
@@ -111,7 +111,7 @@ std::set<ComboAddress> g_proxyProtocolExceptions;
 boost::optional<ComboAddress> g_dns64Prefix{boost::none};
 DNSName g_dns64PrefixReverse;
 unsigned int g_maxChainLength;
-std::shared_ptr<SyncRes::domainmap_t> g_initialDomainMap; // new threads needs this to be setup
+LockGuarded<std::shared_ptr<SyncRes::domainmap_t>> g_initialDomainMap; // new threads needs this to be setup
 std::shared_ptr<NetmaskGroup> g_initialAllowFrom; // new thread needs to be setup with this
 std::shared_ptr<NetmaskGroup> g_initialAllowNotifyFrom; // new threads need this to be setup
 std::shared_ptr<notifyset_t> g_initialAllowNotifyFor; // new threads need this to be setup
@@ -351,6 +351,11 @@ int RecThreadInfo::runThreads(Logr::log_t log)
     info.setHandler();
     info.start(currentThreadId, "web+stat", cpusMap, log);
 
+    if (::arg().mustDo("webserver")) {
+      extern void serveRustWeb();
+      serveRustWeb();
+    }
+
     for (auto& tInfo : RecThreadInfo::infos()) {
       tInfo.thread.join();
       if (tInfo.exitCode != 0) {
@@ -2228,7 +2233,7 @@ static int serviceMain(Logr::log_t log)
   }
   g_networkTimeoutMsec = ::arg().asNum("network-timeout");
 
-  std::tie(g_initialDomainMap, g_initialAllowNotifyFor) = parseZoneConfiguration(g_yamlSettings);
+  std::tie(*g_initialDomainMap.lock(), g_initialAllowNotifyFor) = parseZoneConfiguration(g_yamlSettings);
 
   g_latencyStatSize = ::arg().asNum("latency-statistic-size");
 
@@ -2828,7 +2833,7 @@ static void recursorThread()
     auto& threadInfo = RecThreadInfo::self();
     {
       SyncRes tmp(g_now); // make sure it allocates tsstorage before we do anything, like primeHints or so..
-      SyncRes::setDomainMap(g_initialDomainMap);
+      SyncRes::setDomainMap(*g_initialDomainMap.lock());
       t_allowFrom = g_initialAllowFrom;
       t_allowNotifyFrom = g_initialAllowNotifyFrom;
       t_allowNotifyFor = g_initialAllowNotifyFor;
@@ -3320,10 +3325,6 @@ int main(int argc, char** argv)
       g_packetCache = std::make_unique<RecursorPacketCache>(g_maxPacketCacheEntries, ::arg().asNum("packetcache-shards"));
     }
 
-    if (::arg().mustDo("webserver")) {
-      extern void serveRustWeb();
-      serveRustWeb();
-    }
     ret = serviceMain(startupLog);
   }
   catch (const PDNSException& ae) {
diff --git a/pdns/recursordist/rec-main.hh b/pdns/recursordist/rec-main.hh
index 1d07b75f3d..4effb058f2 100644
--- a/pdns/recursordist/rec-main.hh
+++ b/pdns/recursordist/rec-main.hh
@@ -228,7 +228,7 @@ extern std::atomic<bool> g_statsWanted;
 extern uint32_t g_disthashseed;
 extern int g_argc;
 extern char** g_argv;
-extern std::shared_ptr<SyncRes::domainmap_t> g_initialDomainMap; // new threads needs this to be setup
+extern LockGuarded<std::shared_ptr<SyncRes::domainmap_t>> g_initialDomainMap; // new threads needs this to be setup
 extern std::shared_ptr<NetmaskGroup> g_initialAllowFrom; // new thread needs to be setup with this
 extern std::shared_ptr<NetmaskGroup> g_initialAllowNotifyFrom; // new threads need this to be setup
 extern std::shared_ptr<notifyset_t> g_initialAllowNotifyFor; // new threads need this to be setup
diff --git a/pdns/recursordist/reczones.cc b/pdns/recursordist/reczones.cc
index f03ea75074..f892225d4e 100644
--- a/pdns/recursordist/reczones.cc
+++ b/pdns/recursordist/reczones.cc
@@ -217,8 +217,8 @@ string reloadZoneConfiguration(bool yaml)
     for (const auto& entry : oldAndNewDomains) {
       wipeCaches(entry, true, 0xffff);
     }
-    extern std::shared_ptr<SyncRes::domainmap_t> g_initialDomainMap; // XXX
-    g_initialDomainMap = newDomainMap;
+    extern LockGuarded<std::shared_ptr<SyncRes::domainmap_t>> g_initialDomainMap; // XXX
+    *g_initialDomainMap.lock() = newDomainMap;
     return "ok\n";
   }
   catch (const std::exception& e) {
diff --git a/pdns/recursordist/ws-recursor.cc b/pdns/recursordist/ws-recursor.cc
index 7a1a326198..1e112647ef 100644
--- a/pdns/recursordist/ws-recursor.cc
+++ b/pdns/recursordist/ws-recursor.cc
@@ -359,13 +359,14 @@ static void apiServerZonesPOST(HttpRequest* req, HttpResponse* resp)
   Json document = req->json();
 
   DNSName zonename = apiNameToDNSName(stringFromJson(document, "name"));
-
-  const auto& iter = g_initialDomainMap->find(zonename);
-  if (iter != g_initialDomainMap->cend()) {
-    throw ApiException("Zone already exists");
+  {
+    auto map = g_initialDomainMap.lock();
+    const auto& iter = (*map)->find(zonename);
+    if (iter != (*map)->cend()) {
+      throw ApiException("Zone already exists");
+    }
+    doCreateZone(document);
   }
-
-  doCreateZone(document);
   reloadZoneConfiguration(g_yamlSettings);
   fillZone(zonename, resp);
   resp->status = 201;
@@ -374,7 +375,8 @@ static void apiServerZonesPOST(HttpRequest* req, HttpResponse* resp)
 static void apiServerZonesGET(HttpRequest* /* req */, HttpResponse* resp)
 {
   Json::array doc;
-  for (const auto& val : *g_initialDomainMap) {
+  auto lock = g_initialDomainMap.lock();
+  for (const auto& val : **lock) {
     const SyncRes::AuthDomain& zone = val.second;
     Json::array servers;
     for (const auto& server : zone.d_servers) {