Merge pull request PowerDNS#15196 from rgacogne/rec-fix-secpoll-tsan

rec: Fix a race reported by TSAN around the secpoll status

pdns/recursordist/metrics_table.py

@@ -1,4 +1,4 @@
-# From this table all metrics related files are generated by the metric.py script:
+# From this table all metrics related files are generated by the metrics.py script:
 #
 # - RECURSOR-MIB.txt
 # - docs/rec-metrics-gen.rst
@@ -569,7 +569,7 @@
         'name': 'noping-outqueries', # XXX obsolete?
         'lambda': '[] { return g_Counters.sum(rec::Counter::noPingOutQueries); }',
         'snmp': 73,
-        'desc': 'Number of outgoing queries without ping', 
+        'desc': 'Number of outgoing queries without ping',
     },
     {
         'name': 'noedns-outqueries',

pdns/recursordist/rec_channel_rec.cc

@@ -90,7 +90,6 @@ bool PrefixDashNumberCompare::operator()(const std::string& a, const std::string
   return aa < bb;
 }
 
-static map<string, const uint32_t*> d_get32bitpointers;
 static map<string, const pdns::stat_t*> d_getatomics;
 static map<string, std::function<uint64_t()>> d_get64bitmembers;
 static map<string, std::function<StatsMap()>> d_getmultimembers;
@@ -125,14 +124,6 @@ void disableStats(StatComponent component, const string& stats)
   }
 }
 
-static void addGetStat(const string& name, const uint32_t* place)
-{
-  if (!d_get32bitpointers.emplace(name, place).second) {
-    cerr << "addGetStat: double def " << name << endl;
-    _exit(1);
-  }
-}
-
 static void addGetStat(const string& name, const pdns::stat_t* place)
 {
   if (!d_getatomics.emplace(name, place).second) {
@@ -190,12 +181,12 @@ static std::optional<uint64_t> get(const string& name)
 {
   std::optional<uint64_t> ret;
 
-  if (d_get32bitpointers.count(name))
-    return *d_get32bitpointers.find(name)->second;
-  if (d_getatomics.count(name))
+  if (d_getatomics.count(name) != 0) {
     return d_getatomics.find(name)->second->load();
-  if (d_get64bitmembers.count(name))
+  }
+  if (d_get64bitmembers.count(name) != 0) {
     return d_get64bitmembers.find(name)->second();
+  }
 
   {
     auto dm = d_dynmetrics.lock();
@@ -226,11 +217,6 @@ StatsMap getAllStatsMap(StatComponent component)
   StatsMap ret;
   const auto& disabledlistMap = s_disabledStats.at(component);
 
-  for (const auto& the32bits : d_get32bitpointers) {
-    if (disabledlistMap.count(the32bits.first) == 0) {
-      ret.emplace(the32bits.first, StatsMapEntry{getPrometheusName(the32bits.first), std::to_string(*the32bits.second)});
-    }
-  }
   for (const auto& atomic : d_getatomics) {
     if (disabledlistMap.count(atomic.first) == 0) {
       ret.emplace(atomic.first, StatsMapEntry{getPrometheusName(atomic.first), std::to_string(atomic.second->load())});

pdns/recursordist/secpoll-recursor.cc

@@ -14,8 +14,7 @@
 #define PACKAGEVERSION getPDNSVersion()
 #endif
 
-uint32_t g_security_status;
-string g_security_message;
+pdns::stat_t g_security_status;
 
 void doSecPoll(time_t* last_secpoll, Logr::log_t log)
 {
@@ -87,19 +86,17 @@ void doSecPoll(time_t* last_secpoll, Logr::log_t log)
     return;
   }
 
-  g_security_message = std::move(security_message);
-
-  auto rlog = vlog->withValues("securitymessage", Logging::Loggable(g_security_message), "status", Logging::Loggable(security_status));
+  auto rlog = vlog->withValues("securitymessage", Logging::Loggable(security_message), "status", Logging::Loggable(security_status));
   if (g_security_status != 1 && security_status == 1) {
-    SLOG(g_log << Logger::Warning << "Polled security status of version " << pkgv << ", no known issues reported: " << g_security_message << endl,
+    SLOG(g_log << Logger::Warning << "Polled security status of version " << pkgv << ", no known issues reported: " << security_message << endl,
          rlog->info(Logr::Notice, "Polled security status of version, no known issues reported"));
   }
   if (security_status == 2) {
-    SLOG(g_log << Logger::Error << "PowerDNS Security Update Recommended: " << g_security_message << endl,
+    SLOG(g_log << Logger::Error << "PowerDNS Security Update Recommended: " << security_message << endl,
          rlog->info(Logr::Error, "PowerDNS Security Update Recommended"));
   }
   if (security_status == 3) {
-    SLOG(g_log << Logger::Error << "PowerDNS Security Update Mandatory: " << g_security_message << endl,
+    SLOG(g_log << Logger::Error << "PowerDNS Security Update Mandatory: " << security_message << endl,
          rlog->info(Logr::Error, "PowerDNS Security Update Mandatory"));
   }
 

pdns/recursordist/secpoll-recursor.hh

@@ -23,8 +23,8 @@
 #include <time.h>
 #include "namespaces.hh"
 #include "logr.hh"
+#include "stat_t.hh"
 #include <stdint.h>
 
 void doSecPoll(time_t*, Logr::log_t);
-extern uint32_t g_security_status;
-extern std::string g_security_message;
+extern pdns::stat_t g_security_status;