Remove the IDState lock.

Keeping a copy of the origFD in the response handling thread and
setting ids->age to 0 before setting ids->origFD in the UDP query
thread should prevent dropping query because of a race.

diff --git a/pdns/dnsdist.cc b/pdns/dnsdist.cc
index 5f78dceda22f29a30c126e297c70cafb9fe35780..d5858bf2b50ed635df632783982a84107b017a8e 100644 (file)
--- a/pdns/dnsdist.cc
+++ b/pdns/dnsdist.cc
@@ -157,11 +157,8 @@ void* responderThread(std::shared_ptr<DownstreamState> state)
       continue;
 
     IDState* ids = &state->idStates[dh->id];
-    int origFD;
-    {
-      ReadLock rl(&(ids->lock));
-      origFD = ids->origFD;
-    }
+    int origFD = ids->origFD;
+
     if(origFD < 0) // duplicate
       continue;
     else
@@ -222,11 +219,8 @@ void* responderThread(std::shared_ptr<DownstreamState> state)
     doAvg(g_stats.latencyAvg10000,   udiff,   10000);
     doAvg(g_stats.latencyAvg1000000, udiff, 1000000);
 
-    {
-      WriteLock wl(&(ids->lock));
-      if (ids->origFD == origFD)
-        ids->origFD = -1;
-    }
+    if (ids->origFD == origFD)
+      ids->origFD = -1;
   }
   return 0;
 }
@@ -558,30 +552,27 @@ try
       ss->queries++;
       
       unsigned int idOffset = (ss->idOffset++) % ss->idStates.size();
-      {
-        IDState* ids = &ss->idStates[idOffset];
-        WriteLock wl(&ids->lock);
-
-        if(ids->origFD < 0) // if we are reusing, no change in outstanding
-          ss->outstanding++;
-        else {
-          ss->reuseds++;
-          g_stats.downstreamTimeouts++;
-        }
-
-        ids->origFD = cs->udpFD;
-        ids->age = 0;
-        ids->origID = dh->id;
-        ids->origRemote = remote;
-        ids->sentTime.start();
-        ids->qname = qname;
-        ids->qtype = qtype;
-        ids->origDest.sin4.sin_family=0;
-        ids->delayMsec = delayMsec;
-        ids->origFlags = origFlags;
-        HarvestDestinationAddress(&msgh, &ids->origDest);
+      IDState* ids = &ss->idStates[idOffset];
+      ids->age = 0;
+
+      if(ids->origFD < 0) // if we are reusing, no change in outstanding
+        ss->outstanding++;
+      else {
+        ss->reuseds++;
+        g_stats.downstreamTimeouts++;
       }
 
+      ids->origFD = cs->udpFD;
+      ids->origID = dh->id;
+      ids->origRemote = remote;
+      ids->sentTime.start();
+      ids->qname = qname;
+      ids->qtype = qtype;
+      ids->origDest.sin4.sin_family=0;
+      ids->delayMsec = delayMsec;
+      ids->origFlags = origFlags;
+      HarvestDestinationAddress(&msgh, &ids->origDest);
+
       dh->id = idOffset;
       
       len = send(ss->fd, packet, len, 0);
@@ -682,7 +673,6 @@ void* maintThread()
       dss->prev.reuseds.store(dss->reuseds.load());
       
       for(IDState& ids  : dss->idStates) { // timeouts
-        WriteLock wl(&(ids.lock));
         if(ids.origFD >=0 && ids.age++ > 2) {
           ids.age = 0;
           ids.origFD = -1;

diff --git a/pdns/dnsdist.hh b/pdns/dnsdist.hh
index a778dd58d8ea702db6e92f1d61a43740e5d99af3..a9e25e31432cd438b85994da3b9cb7f359bd6a96 100644 (file)
--- a/pdns/dnsdist.hh
+++ b/pdns/dnsdist.hh
@@ -156,7 +156,7 @@ private:
 
 struct IDState
 {
-  IDState() : origFD(-1), delayMsec(0) { origDest.sin4.sin_family = 0; pthread_rwlock_init(&lock, 0);}
+  IDState() : origFD(-1), delayMsec(0) { origDest.sin4.sin_family = 0;}
   IDState(const IDState& orig)
   {
     origFD = orig.origFD;
@@ -165,7 +165,6 @@ struct IDState
     origDest = orig.origDest;
     delayMsec = orig.delayMsec;
     age.store(orig.age.load());
-    pthread_rwlock_init(&lock, 0);
   }
 
   int origFD;  // set to <0 to indicate this state is empty   // 4
@@ -174,7 +173,6 @@ struct IDState
   ComboAddress origDest;                                      // 28
   StopWatch sentTime;                                         // 16
   DNSName qname;                                              // 80
-  pthread_rwlock_t lock;
   std::atomic<uint16_t> age;                                  // 4
   uint16_t qtype;                                             // 2
   uint16_t origID;                                            // 2