From 86f1ce3b61794ef3f509970f492a9fe98aa491f0 Mon Sep 17 00:00:00 2001
From: Otto <otto.moerbeek@open-xchange.com>
Date: Fri, 20 Aug 2021 13:49:37 +0200
Subject: [PATCH] One way of solving the race. Still needs operator[] though,
 since policyName can vary.

---
 pdns/pdns_recursor.cc              | 2 +-
 pdns/rec_channel_rec.cc            | 6 +++---
 pdns/recursordist/docs/metrics.rst | 2 +-
 pdns/recursordist/rec_metrics.hh   | 2 +-
 pdns/syncres.cc                    | 2 +-
 pdns/syncres.hh                    | 2 +-
 6 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc
index fc76562901..158675c76d 100644
--- a/pdns/pdns_recursor.cc
+++ b/pdns/pdns_recursor.cc
@@ -1161,7 +1161,7 @@ static PolicyResult handlePolicyHit(const DNSFilterEngine::Policy& appliedPolicy
   /* don't account truncate actions for TCP queries, since they are not applied */
   if (appliedPolicy.d_kind != DNSFilterEngine::PolicyKind::Truncate || !dc->d_tcp) {
     ++g_stats.policyResults[appliedPolicy.d_kind];
-    ++g_stats.policyHits[appliedPolicy.getName()];
+    ++(g_stats.policyHits.lock()->operator[](appliedPolicy.getName()));
   }
 
   if (sr.doLog() &&  appliedPolicy.d_type != DNSFilterEngine::PolicyType::None) {
diff --git a/pdns/rec_channel_rec.cc b/pdns/rec_channel_rec.cc
index eb562f5c2e..360b1f54c8 100644
--- a/pdns/rec_channel_rec.cc
+++ b/pdns/rec_channel_rec.cc
@@ -1153,13 +1153,13 @@ static StatsMap toCPUStatsMap(const string& name)
   return entries;
 }
 
-static StatsMap toRPZStatsMap(const string& name, const std::unordered_map<std::string, std::atomic<uint64_t>>& map)
+static StatsMap toRPZStatsMap(const string& name, LockGuarded<std::unordered_map<std::string, std::atomic<uint64_t>>>& map)
 {
   const string pbasename = getPrometheusName(name);
   StatsMap entries;
 
   uint64_t total = 0;
-  for (const auto& entry: map) {
+  for (const auto& entry: *map.lock()) {
     auto &key = entry.first;
     auto count = entry.second.load();
     std::string sname, pname;
@@ -1168,7 +1168,7 @@ static StatsMap toRPZStatsMap(const string& name, const std::unordered_map<std::
       pname = pbasename + "{type=\"filter\"}";
     } else {
       sname = name + "-rpz-" + key;
-      pname = pbasename + "{type=\"rpz\",policyname=\"" + entry.first + "\"}";
+      pname = pbasename + "{type=\"rpz\",policyname=\"" + key + "\"}";
     }
     entries.emplace(sname, StatsMapEntry{pname, std::to_string(count)});
     total += count;
diff --git a/pdns/recursordist/docs/metrics.rst b/pdns/recursordist/docs/metrics.rst
index c1f7255483..a3a83c338c 100644
--- a/pdns/recursordist/docs/metrics.rst
+++ b/pdns/recursordist/docs/metrics.rst
@@ -597,7 +597,7 @@ packets dropped because of (Lua) policy decision
 policy-hits
 ^^^^^^^^^^^
 Number of policy decisions based on Lua (``type = "filter"``), or RPZ (``type = "rpz"``). RPZ hits include the :ref:`rpz-policyName`.
-These metrics are useful for Prometheus and not listed other outputs by default.
+These metrics are useful for Prometheus and not listed in other outputs by default.
 
 policy-result-noaction
 ^^^^^^^^^^^^^^^^^^^^^^
diff --git a/pdns/recursordist/rec_metrics.hh b/pdns/recursordist/rec_metrics.hh
index 1e6d875f28..1881ce12c4 100644
--- a/pdns/recursordist/rec_metrics.hh
+++ b/pdns/recursordist/rec_metrics.hh
@@ -26,7 +26,7 @@
 #include <map>
 
 // Metric types for Prometheus
-enum class PrometheusMetricType
+enum class PrometheusMetricType : uint8_t
 {
   counter,
   gauge,
diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index e484736429..1b1910554e 100644
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -2184,7 +2184,7 @@ void SyncRes::handlePolicyHit(const std::string& prefix, const DNSName& qname, c
   /* don't account truncate actions for TCP queries, since they are not applied */
   if (d_appliedPolicy.d_kind != DNSFilterEngine::PolicyKind::Truncate || !d_queryReceivedOverTCP) {
     ++g_stats.policyResults[d_appliedPolicy.d_kind];
-    ++g_stats.policyHits[d_appliedPolicy.getName()];
+    ++(g_stats.policyHits.lock()->operator[](d_appliedPolicy.getName()));
   }
 
   if (d_appliedPolicy.d_type != DNSFilterEngine::PolicyType::None) {
diff --git a/pdns/syncres.hh b/pdns/syncres.hh
index 7bbadc4993..4b1f752c27 100644
--- a/pdns/syncres.hh
+++ b/pdns/syncres.hh
@@ -1072,7 +1072,7 @@ struct RecursorStats
   std::map<vState, std::atomic<uint64_t> > dnssecResults;
   std::map<vState, std::atomic<uint64_t> > xdnssecResults;
   std::map<DNSFilterEngine::PolicyKind, std::atomic<uint64_t> > policyResults;
-  std::unordered_map<std::string, std::atomic<uint64_t>> policyHits;
+  LockGuarded<std::unordered_map<std::string, std::atomic<uint64_t>>> policyHits;
   std::atomic<uint64_t> rebalancedQueries{0};
   std::atomic<uint64_t> proxyProtocolInvalidCount{0};
   std::atomic<uint64_t> nodLookupsDroppedOversize{0};
-- 
2.47.2