Do not add request to a wait chain that's already processed or being processed.

The following scenario can occur. Multiple concurrent clients doing the same query A
are needed to trigger it:

1. Incoming request A, which has a need for request X
2. Add request X to chain because we already have an identical outstanding request
3. We receive the reply for X
4. We process the chain
5. In the meantime a new request for X that's identical is added to the chain
6. The added id in step 5 is not being processed anymore -> timeout

This can happen if request X has TTL 0, otherwise the record cache would have a hit.

(cherry picked from commit c647a254a0f863aabeaea9d33f673afa26c60457)

diff --git a/pdns/mtasker.hh b/pdns/mtasker.hh
index bff0eca0ff1c5f77e88b1e4a2a8f1a0cf1214c67..e6194f9cd2cd071538edf549858a4a2196fda391 100644 (file)
--- a/pdns/mtasker.hh
+++ b/pdns/mtasker.hh
@@ -81,7 +81,7 @@ public:
     EventKey key;
     std::shared_ptr<pdns_ucontext_t> context;
     struct timeval ttd;
-    int tid;    
+    int tid;
   };
 
   typedef multi_index_container<

diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc
index de1eb6d78f9304dc97461d7835ed7985dc013055..df6750b622e40d631eee520f4da45a94858acced 100644 (file)
--- a/pdns/pdns_recursor.cc
+++ b/pdns/pdns_recursor.cc
@@ -644,7 +644,7 @@ int asendto(const char *data, size_t len, int flags,
   pair<MT_t::waiters_t::iterator, MT_t::waiters_t::iterator> chain=MT->d_waiters.equal_range(pident, PacketIDBirthdayCompare());
 
   for(; chain.first != chain.second; chain.first++) {
-    if(chain.first->key.fd > -1) { // don't chain onto existing chained waiter!
+    if(chain.first->key.fd > -1 && !chain.first->key.closed) { // don't chain onto existing chained waiter or a chain already processed
       /*
       cerr<<"Orig: "<<pident.domain<<", "<<pident.remote.toString()<<", id="<<id<<endl;
       cerr<<"Had hit: "<< chain.first->key.domain<<", "<<chain.first->key.remote.toString()<<", id="<<chain.first->key.id
@@ -3727,6 +3727,9 @@ static void handleTCPClientWritable(int fd, FDMultiplexer::funcparam_t& var)
 // resend event to everybody chained onto it
 static void doResends(MT_t::waiters_t::iterator& iter, PacketID resend, const string& content)
 {
+  // We close the chain for new entries, since they won't be processed anyway
+  iter->key.closed = true;
+
   if(iter->key.chain.empty())
     return;
   //  cerr<<"doResends called!\n";

diff --git a/pdns/syncres.hh b/pdns/syncres.hh
index 1d2c3a97b09a61e6058ee0a4de8a0f0af900c4b4..83de8d1ca2d04eac99d60b358123734c1bc66f16 100644 (file)
--- a/pdns/syncres.hh
+++ b/pdns/syncres.hh
@@ -926,7 +926,7 @@ int arecvtcp(string& data, size_t len, Socket* sock, bool incompleteOkay);
 
 struct PacketID
 {
-  PacketID() : id(0), type(0), sock(0), inNeeded(0), inIncompleteOkay(false), outPos(0), nearMisses(0), fd(-1)
+  PacketID() : id(0), type(0), sock(0), inNeeded(0), inIncompleteOkay(false), outPos(0), nearMisses(0), fd(-1), closed(false)
   {
     remote.reset();
   }
@@ -948,6 +948,7 @@ struct PacketID
   mutable chain_t chain;
   mutable uint32_t nearMisses; // number of near misses - host correct, id wrong
   int fd;
+  mutable bool closed; // Processing already started, don't accept new chained ids
 
   bool operator<(const PacketID& b) const
   {