From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Tue, 18 May 2021 15:30:36 +0000 (+0200)
Subject: dnsdist: Add a suppression for a race in TCP stats reported by TSAN
X-Git-Tag: auth-4.5.0-beta1~24^2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F10423%2Fhead;p=thirdparty%2Fpdns.git

dnsdist: Add a suppression for a race in TCP stats reported by TSAN

We might indeed be off by a few connections in the "maximum connections
seen" counter, but we don't care that much.
---

diff --git a/pdns/dnsdist.hh b/pdns/dnsdist.hh
index e2448fab46..a3b220d132 100644
--- a/pdns/dnsdist.hh
+++ b/pdns/dnsdist.hh
@@ -1033,6 +1033,8 @@ struct DownstreamState
     qps.addHit();
   }
 
+  void incCurrentConnectionsCount();
+
 private:
   std::string name;
   std::string nameWithAddr;
diff --git a/pdns/dnsdistdist/dnsdist-backend.cc b/pdns/dnsdistdist/dnsdist-backend.cc
index 091839b414..52074b90b8 100644
--- a/pdns/dnsdistdist/dnsdist-backend.cc
+++ b/pdns/dnsdistdist/dnsdist-backend.cc
@@ -186,3 +186,11 @@ DownstreamState::~DownstreamState()
     tid.detach();
   }
 }
+
+void DownstreamState::incCurrentConnectionsCount()
+{
+  auto currentConnectionsCount = ++tcpCurrentConnections;
+  if (currentConnectionsCount > tcpMaxConcurrentConnections) {
+    tcpMaxConcurrentConnections.store(currentConnectionsCount);
+  }
+}
diff --git a/pdns/dnsdistdist/dnsdist-tcp-downstream.cc b/pdns/dnsdistdist/dnsdist-tcp-downstream.cc
index 9436c881b0..a43ca1d37c 100644
--- a/pdns/dnsdistdist/dnsdist-tcp-downstream.cc
+++ b/pdns/dnsdistdist/dnsdist-tcp-downstream.cc
@@ -351,10 +351,7 @@ bool TCPConnectionToBackend::reconnect()
       d_queries = 0;
 
       d_handler = std::move(handler);
-      ++d_ds->tcpCurrentConnections;
-      if (d_ds->tcpCurrentConnections > d_ds->tcpMaxConcurrentConnections) {
-        d_ds->tcpMaxConcurrentConnections = d_ds->tcpCurrentConnections;
-      }
+      d_ds->incCurrentConnectionsCount();
       return true;
     }
     catch (const std::runtime_error& e) {
diff --git a/pdns/dnsdistdist/tsan.supp b/pdns/dnsdistdist/tsan.supp
index 0899dde574..2f9978b21f 100644
--- a/pdns/dnsdistdist/tsan.supp
+++ b/pdns/dnsdistdist/tsan.supp
@@ -4,6 +4,7 @@ race:doLatencyStats
 race:handleStats
 race:ClientState::updateTCPMetrics
 race:DownstreamState::updateTCPMetrics
+race:DownstreamState::incCurrentConnectionsCount
 # There is a race when we update the status of a backend,
 # but eventual consistency is fine there
 race:DownstreamState::setDown