From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Thu, 20 Jun 2024 08:21:48 +0000 (+0200)
Subject: auth: Better fix for the leak reported by LSAN in test-distributor_hh.cc
X-Git-Tag: rec-5.2.0-alpha0~5^2~1
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=681cdfb76c06fbfe89e1ae606ac629e42829cb7c;p=thirdparty%2Fpdns.git

auth: Better fix for the leak reported by LSAN in test-distributor_hh.cc

This unit test is checking that we correctly throw when too many queries
are waiting in the distribution pipe, by making the mock backend slow
on purpose. Once the distributor has been restarted as expected, we need
to wait until the mock backend has processed all queued queries, otherwise
Leak Sanitizer will rightfully report a memory leak.
---

diff --git a/pdns/test-distributor_hh.cc b/pdns/test-distributor_hh.cc
index 4f1dc4bdb4..57337cd431 100644
--- a/pdns/test-distributor_hh.cc
+++ b/pdns/test-distributor_hh.cc
@@ -6,7 +6,7 @@
 #ifdef HAVE_CONFIG_H
 #include "config.h"
 #endif
-#include <stdlib.h>
+#include <cstdlib>
 #include <unistd.h>
 #include <boost/test/unit_test.hpp>
 #include "distributor.hh"
@@ -65,8 +65,9 @@ BOOST_AUTO_TEST_CASE(test_distributor_basic) {
 
 struct BackendSlow
 {
-  std::unique_ptr<DNSPacket> question(Question&)
+  std::unique_ptr<DNSPacket> question(Question& query)
   {
+    (void)query;
     if (d_shouldSleep) {
       /* only sleep once per distributor thread, otherwise
          we are sometimes destroyed before picking up the queued
@@ -80,10 +81,10 @@ private:
   bool d_shouldSleep{true};
 };
 
-static std::atomic<int> g_receivedAnswers1;
+static std::atomic<size_t> s_receivedAnswers;
 static void report1(std::unique_ptr<DNSPacket>& /* A */, int /* B */)
 {
-  g_receivedAnswers1++;
+  s_receivedAnswers++;
 }
 
 BOOST_AUTO_TEST_CASE(test_distributor_queue) {
@@ -93,17 +94,31 @@ BOOST_AUTO_TEST_CASE(test_distributor_queue) {
   S.declare("servfail-packets","Number of times a server-failed packet was sent out");
   S.declare("timedout-packets", "timedout-packets");
 
-  auto d=Distributor<DNSPacket, Question, BackendSlow>::Create(2);
+  s_receivedAnswers.store(0);
+  auto* distributor = Distributor<DNSPacket, Question, BackendSlow>::Create(2);
 
+  size_t queued = 0;
   BOOST_CHECK_EXCEPTION( {
-    int n;
     // bound should be higher than max-queue-length
-    for(n=0; n < 2000; ++n)  {
-      Question q;
-      q.d_dt.set();
-      d->question(q, report1);
+    const size_t bound = 2000;
+    for (size_t idx = 0; idx < bound; ++idx)  {
+      Question query;
+      query.d_dt.set();
+      ++queued;
+      distributor->question(query, report1);
     }
     }, DistributorFatal, [](DistributorFatal) { return true; });
+
+  BOOST_CHECK_GT(queued, 1000);
+
+  // now we want to make sure that all queued queries have been processed
+  // otherwise LeakSanitizer will report a leak, but we are only willing to
+  // wait up to 3 seconds (3000 milliseconds)
+  size_t remainingMs = 3000;
+  while (s_receivedAnswers.load() < queued && remainingMs > 0) {
+    std::this_thread::sleep_for(std::chrono::milliseconds(10));
+    remainingMs -= 10;
+  }
 };
 
 struct BackendDies