From 17cf1b4d041ec7ff7b681c1dff06297343c6c71a Mon Sep 17 00:00:00 2001 From: Otto Moerbeek Date: Wed, 11 Nov 2020 12:02:50 +0100 Subject: [PATCH] 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) --- pdns/mtasker.hh | 2 +- pdns/pdns_recursor.cc | 5 ++++- pdns/syncres.hh | 3 ++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/pdns/mtasker.hh b/pdns/mtasker.hh index 0365e756a2..4257824ef5 100644 --- a/pdns/mtasker.hh +++ b/pdns/mtasker.hh @@ -82,7 +82,7 @@ public: EventKey key; std::shared_ptr 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 719185d443..ccb9bfa458 100644 --- a/pdns/pdns_recursor.cc +++ b/pdns/pdns_recursor.cc @@ -662,7 +662,7 @@ int asendto(const char *data, size_t len, int flags, pair 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: "<key.domain<<", "<key.remote.toString()<<", id="<key.id @@ -3431,6 +3431,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 0c9644867c..04dfe7eeda 100644 --- a/pdns/syncres.hh +++ b/pdns/syncres.hh @@ -916,7 +916,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(); } @@ -938,6 +938,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 { -- 2.47.2