]> git.ipfire.org Git - thirdparty/pdns.git/commitdiff
dnsdist: Explicitly update the configuration thread-local copy
authorRemi Gacogne <remi.gacogne@powerdns.com>
Thu, 17 Jul 2025 15:10:04 +0000 (17:10 +0200)
committerRemi Gacogne <remi.gacogne@powerdns.com>
Thu, 28 Aug 2025 09:24:55 +0000 (11:24 +0200)
This commits ensures that all DNSdist threads are regularly checking
if there is a new version of the runtime-modifiable configuration and
update their local copy if necessary. Regular accesses to the copy
are now fully read-only, meaning they do not invalidate the current copy.
It prevents the case where a function is invalidating the copy that the
caller is holding, and makes accessing the configuration cheaper.

Signed-off-by: Remi Gacogne <remi.gacogne@powerdns.com>
(cherry picked from commit c08ba9f1f2fe743c3e05313b2c329446e2e2d1b1)

19 files changed:
pdns/dnsdistdist/dnsdist-async.cc
pdns/dnsdistdist/dnsdist-carbon.cc
pdns/dnsdistdist/dnsdist-configuration.cc
pdns/dnsdistdist/dnsdist-configuration.hh
pdns/dnsdistdist/dnsdist-console.cc
pdns/dnsdistdist/dnsdist-discovery.cc
pdns/dnsdistdist/dnsdist-lua-network.cc
pdns/dnsdistdist/dnsdist-lua.cc
pdns/dnsdistdist/dnsdist-nghttp2.cc
pdns/dnsdistdist/dnsdist-resolver.cc
pdns/dnsdistdist/dnsdist-self-answers.cc
pdns/dnsdistdist/dnsdist-tcp.cc
pdns/dnsdistdist/dnsdist-web.cc
pdns/dnsdistdist/dnsdist-xsk.cc
pdns/dnsdistdist/dnsdist.cc
pdns/dnsdistdist/doh.cc
pdns/dnsdistdist/doh3.cc
pdns/dnsdistdist/doq.cc
pdns/sholder.hh

index 776a8f011cef95e7ad6a576534036d48048feb63..194077cdf7b9b8d1c39614bac153a957cb78761d 100644 (file)
@@ -96,6 +96,8 @@ void AsynchronousHolder::mainThread(std::shared_ptr<Data> data)
   while (true) {
     bool shouldWait = true;
     int timeout = -1;
+    dnsdist::configuration::refreshLocalRuntimeConfiguration();
+
     {
       auto content = data->d_content.lock();
       if (data->d_done) {
index b14f5359c9148aae844843c6e419d23ed7878d80..d005f9552acb46340a3fea1d949f0eab8c0cf469 100644 (file)
@@ -309,6 +309,8 @@ static void carbonHandler(const Carbon::Endpoint& endpoint)
   try {
     uint8_t consecutiveFailures = 0;
     do {
+      dnsdist::configuration::refreshLocalRuntimeConfiguration();
+
       DTime dtimer;
       dtimer.set();
       if (doOneCarbonExport(endpoint)) {
index 70875b88901166d09ca5138e037d7633fd347446..c4db8f3563d248d0f5356b829f8a669d447718b5 100644 (file)
@@ -29,15 +29,27 @@ static GlobalStateHolder<RuntimeConfiguration> s_currentRuntimeConfiguration;
 static ImmutableConfiguration s_immutableConfiguration;
 static std::atomic<bool> s_immutableConfigurationDone{false};
 
-const RuntimeConfiguration& getCurrentRuntimeConfiguration()
+static const RuntimeConfiguration& getCurrentRuntimeConfigurationInternal(bool refresh)
 {
   static thread_local auto t_threadLocalConfiguration = s_currentRuntimeConfiguration.getLocal();
-  return *t_threadLocalConfiguration;
+  return t_threadLocalConfiguration.get(!refresh);
+}
+
+const RuntimeConfiguration& getCurrentRuntimeConfiguration()
+{
+  return getCurrentRuntimeConfigurationInternal(false);
+}
+
+const RuntimeConfiguration& refreshLocalRuntimeConfiguration()
+{
+  return getCurrentRuntimeConfigurationInternal(true);
 }
 
 void updateRuntimeConfiguration(const std::function<void(RuntimeConfiguration&)>& mutator)
 {
   s_currentRuntimeConfiguration.modify(mutator);
+  /* refresh the local "cache" right away */
+  refreshLocalRuntimeConfiguration();
 }
 
 void updateImmutableConfiguration(const std::function<void(ImmutableConfiguration&)>& mutator)
index 063c69fdc6beb6025136e6bb36b3f37589bc7092..a60a5068670e13bed7764085d1da10a35b112d0d 100644 (file)
@@ -173,12 +173,14 @@ struct RuntimeConfiguration
 };
 
 /* Be careful not to hold on this for too long, it can be invalidated
-   by the next call to getCurrentRuntimeConfiguration() from the
+   by the next call to refreshLocalRuntimeConfiguration() from the
    same thread, so better be sure that any function you are not calling
-   while holding to this reference does not call getCurrentRuntimeConfiguration()
+   while holding to this reference does not call refreshLocalRuntimeConfiguration
    itself. When in doubt, better call getCurrentRuntimeConfiguration() twice.
 */
 const RuntimeConfiguration& getCurrentRuntimeConfiguration();
+const RuntimeConfiguration& refreshLocalRuntimeConfiguration();
+
 /* Get the runtime-immutable configuration */
 const ImmutableConfiguration& getImmutableConfiguration();
 /* Update the runtime-immutable part of the configuration. This function can only be called
index 626bcd3d253493783fe426338d4567ea882d1e37..db0e9735fb8c0ea8800a8a4857a07c8c28e0af2d 100644 (file)
@@ -528,6 +528,8 @@ static void controlClientThread(ConsoleConnection&& conn)
 
       line = dnsdist::crypto::authenticated::decryptSym(line, consoleKey, readingNonce);
 
+      dnsdist::configuration::refreshLocalRuntimeConfiguration();
+
       string response;
       try {
         bool withReturn = true;
@@ -648,6 +650,7 @@ void controlThread(Socket&& acceptFD)
     infolog("Accepting control connections on %s", local.toStringWithPort());
 
     while ((sock = SAccept(acceptFD.getHandle(), client)) >= 0) {
+      dnsdist::configuration::refreshLocalRuntimeConfiguration();
       const auto& consoleKey = dnsdist::configuration::getCurrentRuntimeConfiguration().d_consoleKey;
       FDWrapper socket(sock);
       if (!dnsdist::crypto::authenticated::isValidKey(consoleKey)) {
index e65d54ecb390d6e413fbb64b987eca37ecfb73e0..b3b9fff82ef726ca6d52bcc266b5d1fc9935ba5c 100644 (file)
@@ -500,6 +500,7 @@ void ServiceDiscovery::worker()
 {
   setThreadName("dnsdist/discove");
   while (true) {
+    dnsdist::configuration::refreshLocalRuntimeConfiguration();
     time_t now = time(nullptr);
 
     auto upgradeables = *(s_upgradeableBackends.lock());
index 58ad214ad5f7359e17c5ecd2d81400e9da0d16d2..e4e56e3082e235be6084c913832f8034b5830bff 100644 (file)
@@ -144,6 +144,7 @@ void NetworkListener::runOnce(ListenerData& data, timeval& now, uint32_t timeout
     return;
   }
 
+  dnsdist::configuration::refreshLocalRuntimeConfiguration();
   data.d_running = true;
   if (data.d_sockets.empty()) {
     throw runtime_error("NetworkListener started with no sockets");
index f330c2dae7c420f21b5c46b168cb027c9f7c9c83..67fc230fc8ec3cfbe43f743c480b7c623478fcb9 100644 (file)
@@ -291,6 +291,7 @@ static void LuaThread(const std::string& code)
 
   for (;;) {
     try {
+      dnsdist::configuration::refreshLocalRuntimeConfiguration();
       context.executeCode(code);
       errlog("Lua thread exited, restarting in 5 seconds");
     }
index 338858f9138bb4ac0223714383315c0b766296a2..5eea2a3429cdb234b8d2c99af87d5d88bc987d19 100644 (file)
@@ -878,6 +878,8 @@ static void dohClientThread(pdns::channel::Receiver<CrossProtocolQuery>&& receiv
     for (;;) {
       data.mplexer->run(&now, 1000);
 
+      dnsdist::configuration::refreshLocalRuntimeConfiguration();
+
       if (now.tv_sec > lastTimeoutScan) {
         lastTimeoutScan = now.tv_sec;
 
index 8e3417dc8c3eade478c406a3a90f1053e8f59c04..c9b1355e4ea0061dcba36a51c146f99be0f51d35 100644 (file)
 
 #include "dnsdist-resolver.hh"
 #include "iputils.hh"
+#include "threadname.hh"
 
 namespace dnsdist::resolver
 {
 void asynchronousResolver(const std::string& hostname, const std::function<void(const std::string& hostname, std::vector<ComboAddress>& ips)>& callback)
 {
+  setThreadName("dnsdist/resolve");
   addrinfo hints{};
   hints.ai_family = AF_UNSPEC;
   hints.ai_flags = AI_ADDRCONFIG;
index 26beb80e3432ce9e88d6adc79fcdde00fc44fe7f..ae87da5b31bbcbff31904a07f653070aa8509e3c 100644 (file)
@@ -252,7 +252,6 @@ bool generateAnswerFromRawPacket(DNSQuestion& dnsQuestion, const PacketBuffer& p
 bool removeRecordsAndSetRCode(DNSQuestion& dnsQuestion, uint8_t rcode)
 {
   auto [hadEDNS, dnssecOK] = getEDNSStatusInQuery(dnsQuestion);
-
   dnsdist::PacketMangling::editDNSHeaderFromPacket(dnsQuestion.getMutableData(), [rcode](dnsheader& header) {
     header.rcode = rcode;
     header.qr = true;
index 7e12e67c233805cfba0279ed34db707ef250213b..d58cd24f3620eee614b5a88d485a091888e9ea8d 100644 (file)
@@ -1717,6 +1717,8 @@ static void tcpClientThread(pdns::channel::Receiver<ConnectionInfo>&& queryRecei
     for (;;) {
       data.mplexer->run(&now);
 
+      dnsdist::configuration::refreshLocalRuntimeConfiguration();
+
       try {
         t_downstreamTCPConnectionsManager.cleanupClosedConnections(now);
         dnsdist::IncomingConcurrentTCPConnectionsManager::cleanup(time(nullptr));
@@ -1880,6 +1882,7 @@ void tcpAcceptorThread(const std::vector<ClientState*>& states)
     timeval now{};
     while (true) {
       mplexer->run(&now, -1);
+      dnsdist::configuration::refreshLocalRuntimeConfiguration();
     }
   }
 }
index de657f3ee0667ef2eb7d77e87da96fd1a8deec09..42885f6b5d8d84431daa9571e462a7b340e31f08 100644 (file)
@@ -1931,6 +1931,7 @@ void WebserverThread(ComboAddress listeningAddress, Socket sock)
     try {
       ComboAddress remote(listeningAddress);
       int fileDesc = SAccept(sock.getHandle(), remote);
+      dnsdist::configuration::refreshLocalRuntimeConfiguration();
 
       if (!isClientAllowedByACL(remote)) {
         vinfolog("Connection to webserver from client %s is not allowed, closing", remote.toStringWithPort());
index ff65b5c7cb503734436e2c02e1858b73be5c9036..42e74bd966426912ac63b1fb3ba873d5d4e78589 100644 (file)
@@ -42,6 +42,7 @@ void XskResponderThread(std::shared_ptr<DownstreamState> dss, std::shared_ptr<Xs
     auto pollfds = getPollFdsForWorker(*xskInfo);
     while (!dss->isStopped()) {
       poll(pollfds.data(), pollfds.size(), -1);
+      dnsdist::configuration::refreshLocalRuntimeConfiguration();
       bool needNotify = false;
       if ((pollfds[0].revents & POLLIN) != 0) {
         needNotify = true;
@@ -135,6 +136,7 @@ void XskRouter(std::shared_ptr<XskSocket> xsk)
   while (true) {
     try {
       auto ready = xsk->wait(-1);
+      dnsdist::configuration::refreshLocalRuntimeConfiguration();
       // descriptor 0 gets incoming AF_XDP packets
       if ((fds.at(0).revents & POLLIN) != 0) {
         auto packets = xsk->recv(64, &failed);
@@ -199,6 +201,7 @@ void XskClientThread(ClientState* clientState)
     while (!xskInfo->hasIncomingFrames()) {
       xskInfo->waitForXskSocket();
     }
+    dnsdist::configuration::refreshLocalRuntimeConfiguration();
     xskInfo->processIncomingFrames([&](XskPacket packet) {
       if (XskProcessQuery(*clientState, packet)) {
         packet.updatePacket();
index 81316cd68557913e1ed409d9a3a2286e9fcba79f..3396f3c1960668159c69f285d462f7221bed1880 100644 (file)
@@ -655,7 +655,7 @@ void handleResponseSent(const DNSName& qname, const QType& qtype, double udiff,
 
 static void handleResponseTC4UDPClient(DNSQuestion& dnsQuestion, uint16_t udpPayloadSize, PacketBuffer& response)
 {
-  if (udpPayloadSize > 0 && response.size() > udpPayloadSize) {
+  if (udpPayloadSize != 0 && response.size() > udpPayloadSize) {
     vinfolog("Got a response of size %d while the initial UDP payload size was %d, truncating", response.size(), udpPayloadSize);
     truncateTC(dnsQuestion.getMutableData(), dnsQuestion.getMaximumSize(), dnsQuestion.ids.qname.wirelength(), dnsdist::configuration::getCurrentRuntimeConfiguration().d_addEDNSToSelfGeneratedResponses);
     dnsdist::PacketMangling::editDNSHeaderFromPacket(dnsQuestion.getMutableData(), [](dnsheader& header) {
@@ -663,7 +663,7 @@ static void handleResponseTC4UDPClient(DNSQuestion& dnsQuestion, uint16_t udpPay
       return true;
     });
   }
-  else if (dnsQuestion.getHeader()->tc && dnsdist::configuration::getCurrentRuntimeConfiguration().d_truncateTC) {
+  else if (dnsdist::configuration::getCurrentRuntimeConfiguration().d_truncateTC && dnsQuestion.getHeader()->tc) {
     truncateTC(response, dnsQuestion.getMaximumSize(), dnsQuestion.ids.qname.wirelength(), dnsdist::configuration::getCurrentRuntimeConfiguration().d_addEDNSToSelfGeneratedResponses);
   }
 }
@@ -792,6 +792,8 @@ void responderThread(std::shared_ptr<DownstreamState> dss)
           break;
         }
 
+        dnsdist::configuration::refreshLocalRuntimeConfiguration();
+
         for (const auto& sockDesc : sockets) {
           /* allocate one more byte so we can detect truncation */
           // NOLINTNEXTLINE(bugprone-use-after-move): resizing a vector has no preconditions so it is valid to do so after moving it
@@ -2211,6 +2213,7 @@ static void udpClientThread(std::vector<ClientState*> states)
 
         packet.resize(static_cast<size_t>(got));
 
+        dnsdist::configuration::refreshLocalRuntimeConfiguration();
         processUDPQuery(*param.cs, &msgh, remote, dest, packet, nullptr, nullptr, nullptr, nullptr);
       };
 
@@ -2292,6 +2295,7 @@ static void maintThread()
   for (;;) {
     std::this_thread::sleep_for(std::chrono::seconds(interval));
 
+    dnsdist::configuration::refreshLocalRuntimeConfiguration();
     {
       auto lua = g_lua.lock();
       try {
@@ -2364,6 +2368,7 @@ static void dynBlockMaintenanceThread()
 {
   setThreadName("dnsdist/dynBloc");
 
+  dnsdist::configuration::refreshLocalRuntimeConfiguration();
   DynBlockMaintenance::run();
 }
 #endif
@@ -2374,7 +2379,7 @@ static void secPollThread()
   setThreadName("dnsdist/secpoll");
 
   for (;;) {
-    const auto& runtimeConfig = dnsdist::configuration::getCurrentRuntimeConfiguration();
+    const auto& runtimeConfig = dnsdist::configuration::refreshLocalRuntimeConfiguration();
 
     try {
       dnsdist::secpoll::doSecPoll(runtimeConfig.d_secPollSuffix);
@@ -2421,9 +2426,11 @@ static void healthChecksThread()
     }
 
     std::unique_ptr<FDMultiplexer> mplexer{nullptr};
+    const auto& runtimeConfig = dnsdist::configuration::refreshLocalRuntimeConfiguration();
+
     // this points to the actual shared_ptrs!
     // coverity[auto_causes_copy]
-    const auto servers = dnsdist::configuration::getCurrentRuntimeConfiguration().d_backends;
+    const auto servers = runtimeConfig.d_backends;
     for (const auto& dss : servers) {
       dss->updateStatisticsInfo();
 
@@ -3122,11 +3129,12 @@ static void parseParameters(int argc, char** argv, CommandLineParameters& cmdLin
 static void setupPools()
 {
   bool precompute = false;
-  if (dnsdist::configuration::getCurrentRuntimeConfiguration().d_lbPolicy->getName() == "chashed") {
+  const auto& currentConfig = dnsdist::configuration::getCurrentRuntimeConfiguration();
+  if (currentConfig.d_lbPolicy->getName() == "chashed") {
     precompute = true;
   }
   else {
-    for (const auto& entry : dnsdist::configuration::getCurrentRuntimeConfiguration().d_pools) {
+    for (const auto& entry : currentConfig.d_pools) {
       if (entry.second->policy != nullptr && entry.second->policy->getName() == "chashed") {
         precompute = true;
         break;
@@ -3136,7 +3144,7 @@ static void setupPools()
   if (precompute) {
     vinfolog("Pre-computing hashes for consistent hash load-balancing policy");
     // pre compute hashes
-    for (const auto& backend : dnsdist::configuration::getCurrentRuntimeConfiguration().d_backends) {
+    for (const auto& backend : currentConfig.d_backends) {
       if (backend->d_config.d_weight < 100) {
         vinfolog("Warning, the backend '%s' has a very low weight (%d), which will not yield a good distribution of queries with the 'chashed' policy. Please consider raising it to at least '100'.", backend->getName(), backend->d_config.d_weight);
       }
@@ -3262,11 +3270,11 @@ static void startFrontends()
     if (clientState->dohFrontend != nullptr && clientState->dohFrontend->d_library == "h2o") {
 #ifdef HAVE_DNS_OVER_HTTPS
 #ifdef HAVE_LIBH2OEVLOOP
-      std::thread dotThreadHandle(dohThread, clientState.get());
+      std::thread dohThreadHandle(dohThread, clientState.get());
       if (!clientState->cpus.empty()) {
-        mapThreadToCPUList(dotThreadHandle.native_handle(), clientState->cpus);
+        mapThreadToCPUList(dohThreadHandle.native_handle(), clientState->cpus);
       }
-      dotThreadHandle.detach();
+      dohThreadHandle.detach();
 #endif /* HAVE_LIBH2OEVLOOP */
 #endif /* HAVE_DNS_OVER_HTTPS */
       continue;
index 43f4cb3941a04b47036fb7e944cf554209286fe8..9dfaa5767acc8dbefd841562984932384c7f1ef0 100644 (file)
@@ -701,6 +701,8 @@ static void processDOHQuery(DOHUnitUniquePtr&& unit, bool inMainThread = false)
     }
   };
 
+  dnsdist::configuration::refreshLocalRuntimeConfiguration();
+
   auto& ids = unit->ids;
   uint16_t queryId = 0;
   ComboAddress remote;
@@ -1072,6 +1074,8 @@ static std::optional<ComboAddress> processForwardedForHeader(const h2o_req_t* re
  */
 static int doh_handler(h2o_handler_t *self, h2o_req_t *req)
 {
+  dnsdist::configuration::refreshLocalRuntimeConfiguration();
+
   try {
     if (req->conn->ctx->storage.size == 0) {
       return 0; // although we might was well crash on this
index a027dd4173fa468cf72b08715701893e72775583..7a8bc439f0647d6237d0e382cfa8ac725f9a6548 100644 (file)
@@ -1019,6 +1019,8 @@ void doh3Thread(ClientState* clientState)
       readyFDs.clear();
       mplexer->getAvailableFDs(readyFDs, 500);
 
+      dnsdist::configuration::refreshLocalRuntimeConfiguration();
+
       try {
         if (std::find(readyFDs.begin(), readyFDs.end(), sock.getHandle()) != readyFDs.end()) {
           handleSocketReadable(*frontend, *clientState, sock, buffer);
index 46c9c9202bda30e05cac232031766e7f83514cad..d2aa3896235f234e738430e756bbece582f50ddd 100644 (file)
@@ -806,6 +806,8 @@ void doqThread(ClientState* clientState)
       readyFDs.clear();
       mplexer->getAvailableFDs(readyFDs, 500);
 
+      dnsdist::configuration::refreshLocalRuntimeConfiguration();
+
       try {
         if (std::find(readyFDs.begin(), readyFDs.end(), sock.getHandle()) != readyFDs.end()) {
           handleSocketReadable(*frontend, *clientState, sock, buffer);
index f26dd250bdff2e67822064dff1c791b55ab57974..554a0f1b59302f7fd8d5d83071b6bffccdbca203 100644 (file)
@@ -74,6 +74,17 @@ public:
     return *operator->();
   }
 
+  // fast const-only access, see "read-only" above.
+  // if allowedOutdated is true, the current local version is returned
+  // without checking if there has been an update.
+  const T& get(bool allowedOutdated = false)
+  {
+    if (!allowedOutdated || !d_state) {
+      return *operator->();
+    }
+    return *d_state;
+  }
+
   void reset()
   {
     d_generation = 0;