From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Wed, 15 Dec 2021 10:49:29 +0000 (+0100)
Subject: dnsdist: Add comments on reference counting around our internal pipe
X-Git-Tag: auth-4.7.0-alpha1~122^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e36f950460e7fa8e53c9b6751e244d15f6bcd24c;p=thirdparty%2Fpdns.git

dnsdist: Add comments on reference counting around our internal pipe
---

diff --git a/pdns/dnsdistdist/doh.cc b/pdns/dnsdistdist/doh.cc
index 92396fc5fd..e6f6594677 100644
--- a/pdns/dnsdistdist/doh.cc
+++ b/pdns/dnsdistdist/doh.cc
@@ -227,7 +227,15 @@ struct DOHServerConfig
    The caller should NOT release or touch the unit after calling this function */
 static void sendDoHUnitToTheMainThread(DOHUnitUniquePtr&& du, const char* description)
 {
+  /* taking a naked pointer since we are about to send that pointer over a pipe */
   auto ptr = du.release();
+  /* increasing the reference counter. This should not be strictly needed because
+     we already hold a reference and will only release it if we failed to send the
+     pointer over the pipe, but TSAN seems confused when the responder thread gets
+     a reply from a backend before the send() syscall sending the corresponding query
+     to that backend has returned in the initial thread.
+     The memory barrier needed to increase that counter seems to work around that.
+  */
   ptr->get();
   static_assert(sizeof(ptr) <= PIPE_BUF, "Writes up to PIPE_BUF are guaranteed not to be interleaved and to either fully succeed or fail");
 
@@ -241,8 +249,10 @@ static void sendDoHUnitToTheMainThread(DOHUnitUniquePtr&& du, const char* descri
       vinfolog("Unable to pass a %s to the DoH worker thread because we couldn't write to the pipe: %s", description, stringerror());
     }
 
+    /* we fail to write over the pipe so we do not need to hold to that ref anymore */
     ptr->release();
   }
+  /* we decrement the counter incremented above at the beginning of that function */
   ptr->release();
 }