dnsdist: Fix a race when accessing a backend health status

While there should not be much risk in a data race involving a boolean
apart from getting an outdated value, it's still undefined behaviour
and it rightfully makes TSAN unhappy.
This commit makes the status atomic: hopefully using relaxed memory
ordering when reading the status will make it as cheap as a regular
non-atomic read on most platforms.

diff --git a/pdns/dnsdistdist/dnsdist-backend.cc b/pdns/dnsdistdist/dnsdist-backend.cc
index 7f5603482f155214702d814fa2735fd3e954096b..2f400cf55a90ded26e028635350615a897c1acbd 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-backend.cc
+++ b/pdns/dnsdistdist/dnsdist-backend.cc
@@ -834,7 +834,7 @@ void DownstreamState::submitHealthCheckResult(bool initial, bool newResult)
 
     currentCheckFailures++;
 
-    if (upStatus) {
+    if (upStatus.load()) {
       /* we were previously marked as "up" and failed a health-check,
          let's see if this is enough to move to the "down" state or if
          need more failed checks for that */
@@ -853,7 +853,7 @@ void DownstreamState::submitHealthCheckResult(bool initial, bool newResult)
     }
   }
 
-  if (newState != upStatus) {
+  if (newState != upStatus.load()) {
     /* we are actually moving to a new state */
     if (!IsAnyAddress(d_config.remote)) {
       infolog("Marking downstream %s as '%s'", getNameWithAddr(), newState ? "up" : "down");

diff --git a/pdns/dnsdistdist/dnsdist-lua-bindings.cc b/pdns/dnsdistdist/dnsdist-lua-bindings.cc
index e2bf8aef86961b4c96b750aab90f2939cdd02302..de53de97dba53006d3a0be26c2b7e94109eb95a4 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-lua-bindings.cc
+++ b/pdns/dnsdistdist/dnsdist-lua-bindings.cc
@@ -144,7 +144,10 @@ void setupLuaBindings(LuaContext& luaCtx, bool client, bool configCheck)
   });
   luaCtx.registerFunction<std::string (DownstreamState::*)() const>("getName", [](const DownstreamState& state) -> const std::string& { return state.getName(); });
   luaCtx.registerFunction<std::string (DownstreamState::*)() const>("getNameWithAddr", [](const DownstreamState& state) -> const std::string& { return state.getNameWithAddr(); });
-  luaCtx.registerMember("upStatus", &DownstreamState::upStatus);
+  luaCtx.registerMember<bool(DownstreamState::*)>(
+    "upStatus",
+    [](const DownstreamState& state) -> bool { return state.upStatus.load(std::memory_order_relaxed); },
+    [](DownstreamState& state, bool newStatus) { state.upStatus.store(newStatus); });
   luaCtx.registerMember<int(DownstreamState::*)>(
     "weight",
     [](const DownstreamState& state) -> int { return state.d_config.d_weight; },

diff --git a/pdns/dnsdistdist/dnsdist-web.cc b/pdns/dnsdistdist/dnsdist-web.cc
index 45c9b431e667c400a27893e5f8f1dd2b44be42d1..705646b68e575ef05387966ef30f684bd8c0eeda 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-web.cc
+++ b/pdns/dnsdistdist/dnsdist-web.cc
@@ -1078,7 +1078,7 @@ static void addServerToJSON(Json::array& servers, int identifier, const std::sha
     status = "DOWN";
   }
   else {
-    status = (backend->upStatus ? "up" : "down");
+    status = (backend->upStatus.load(std::memory_order_relaxed) ? "up" : "down");
   }
 
   Json::array pools;

diff --git a/pdns/dnsdistdist/dnsdist.hh b/pdns/dnsdistdist/dnsdist.hh
index cf4da8f6f50dc76f8a71c7896dfab27c39e18152..b643c827f31bd93142010cceb809934c9ba20057 100644 (file)
--- a/pdns/dnsdistdist/dnsdist.hh
+++ b/pdns/dnsdistdist/dnsdist.hh
@@ -903,7 +903,7 @@ public:
   uint16_t currentCheckFailures{0};
   std::atomic<bool> hashesComputed{false};
   std::atomic<bool> connected{false};
-  bool upStatus{false};
+  std::atomic<bool> upStatus{false};
 
 private:
   void handleUDPTimeout(IDState& ids);
@@ -942,7 +942,7 @@ public:
     else if (d_config.availability == Availability::Up) {
       return true;
     }
-    return upStatus;
+    return upStatus.load(std::memory_order_relaxed);
   }
 
   void setUp()
@@ -952,8 +952,8 @@ public:
 
   void setUpStatus(bool newStatus)
   {
-    upStatus = newStatus;
-    if (!upStatus) {
+    upStatus.store(newStatus);
+    if (!newStatus) {
       latencyUsec = 0.0;
       latencyUsecTCP = 0.0;
     }
@@ -999,7 +999,7 @@ public:
       status = "DOWN";
     }
     else {
-      status = (upStatus ? "up" : "down");
+      status = (upStatus.load(std::memory_order_relaxed) ? "up" : "down");
     }
     return status;
   }