From: Otto <otto.moerbeek@open-xchange.com>
Date: Mon, 2 Aug 2021 12:15:53 +0000 (+0200)
Subject: Use explicit PacketIDCompare
X-Git-Tag: dnsdist-1.7.0-alpha1~74^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0662c2d3c60bca6d99b0edd269630b23e913bdde;p=thirdparty%2Fpdns.git

Use explicit PacketIDCompare
---

diff --git a/pdns/mtasker.cc b/pdns/mtasker.cc
index 8776a4e33f..c02e27781f 100644
--- a/pdns/mtasker.cc
+++ b/pdns/mtasker.cc
@@ -170,7 +170,7 @@ int main()
     \return returns -1 in case of error, 0 in case of timeout, 1 in case of an answer 
 */
 
-template<class EventKey, class EventVal>int MTasker<EventKey,EventVal>::waitEvent(EventKey &key, EventVal *val, unsigned int timeoutMsec, const struct timeval* now)
+template<class EventKey, class EventVal, class Cmp>int MTasker<EventKey,EventVal,Cmp>::waitEvent(EventKey &key, EventVal *val, unsigned int timeoutMsec, const struct timeval* now)
 {
   if(d_waiters.count(key)) { // there was already an exact same waiter
     return -1;
@@ -219,7 +219,7 @@ template<class EventKey, class EventVal>int MTasker<EventKey,EventVal>::waitEven
 //! yields control to the kernel or other threads
 /** Hands over control to the kernel, allowing other processes to run, or events to arrive */
 
-template<class Key, class Val>void MTasker<Key,Val>::yield()
+template<class Key, class Val, class Cmp>void MTasker<Key,Val,Cmp>::yield()
 {
   d_runQueue.push(d_tid);
   notifyStackSwitchToKernel();
@@ -235,7 +235,7 @@ template<class Key, class Val>void MTasker<Key,Val>::yield()
 
     WARNING: when passing val as zero, d_waitval is undefined, and hence waitEvent will return undefined!
 */
-template<class EventKey, class EventVal>int MTasker<EventKey,EventVal>::sendEvent(const EventKey& key, const EventVal* val)
+template<class EventKey, class EventVal, class Cmp>int MTasker<EventKey,EventVal,Cmp>::sendEvent(const EventKey& key, const EventVal* val)
 {
   typename waiters_t::iterator waiter=d_waiters.find(key);
 
@@ -262,7 +262,7 @@ template<class EventKey, class EventVal>int MTasker<EventKey,EventVal>::sendEven
     \param start Pointer to the function which will form the start of the thread
     \param val A void pointer that can be used to pass data to the thread
 */
-template<class Key, class Val>void MTasker<Key,Val>::makeThread(tfunc_t *start, void* val)
+template<class Key, class Val, class Cmp>void MTasker<Key,Val,Cmp>::makeThread(tfunc_t *start, void* val)
 {
   auto uc=std::make_shared<pdns_ucontext_t>();
   
@@ -303,7 +303,7 @@ template<class Key, class Val>void MTasker<Key,Val>::makeThread(tfunc_t *start,
     \return Returns if there is more work scheduled and recalling schedule now would be useful
       
 */
-template<class Key, class Val>bool MTasker<Key,Val>::schedule(const struct timeval*  now)
+template<class Key, class Val, class Cmp>bool MTasker<Key,Val,Cmp>::schedule(const struct timeval*  now)
 {
   if(!d_runQueue.empty()) {
     d_tid=d_runQueue.front();
@@ -359,7 +359,7 @@ template<class Key, class Val>bool MTasker<Key,Val>::schedule(const struct timev
 /** Call this to check if no processes are running anymore
     \return true if no processes are left
  */
-template<class Key, class Val>bool MTasker<Key,Val>::noProcesses() const
+template<class Key, class Val, class Cmp>bool MTasker<Key,Val,Cmp>::noProcesses() const
 {
   return d_threadsCount == 0;
 }
@@ -368,7 +368,7 @@ template<class Key, class Val>bool MTasker<Key,Val>::noProcesses() const
 /** Call this to perhaps limit activities if too many threads are running
     \return number of processes running
  */
-template<class Key, class Val>unsigned int MTasker<Key,Val>::numProcesses() const
+template<class Key, class Val, class Cmp>unsigned int MTasker<Key,Val,Cmp>::numProcesses() const
 {
   return d_threadsCount;
 }
@@ -380,7 +380,7 @@ template<class Key, class Val>unsigned int MTasker<Key,Val>::numProcesses() cons
 
     \param events Vector which is to be filled with keys threads are waiting for
 */
-template<class Key, class Val>void MTasker<Key,Val>::getEvents(std::vector<Key>& events)
+template<class Key, class Val, class Cmp>void MTasker<Key,Val,Cmp>::getEvents(std::vector<Key>& events)
 {
   events.clear();
   for(typename waiters_t::const_iterator i=d_waiters.begin();i!=d_waiters.end();++i) {
@@ -392,19 +392,19 @@ template<class Key, class Val>void MTasker<Key,Val>::getEvents(std::vector<Key>&
 /** Processes can call this to get a numerical representation of their current thread ID.
     This can be useful for logging purposes.
 */
-template<class Key, class Val>int MTasker<Key,Val>::getTid() const
+template<class Key, class Val, class Cmp>int MTasker<Key,Val,Cmp>::getTid() const
 {
   return d_tid;
 }
 
 //! Returns the maximum stack usage so far of this MThread
-template<class Key, class Val>uint64_t MTasker<Key,Val>::getMaxStackUsage()
+template<class Key, class Val, class Cmp>uint64_t MTasker<Key,Val,Cmp>::getMaxStackUsage()
 {
   return d_threads[d_tid].startOfStack - d_threads[d_tid].highestStackSeen;
 }
 
 //! Returns the maximum stack usage so far of this MThread
-template<class Key, class Val>unsigned int MTasker<Key,Val>::getUsec()
+template<class Key, class Val, class Cmp>unsigned int MTasker<Key,Val,Cmp>::getUsec()
 {
 #ifdef MTASKERTIMING
   return d_threads[d_tid].totTime + d_threads[d_tid].dt.ndiff()/1000;
diff --git a/pdns/mtasker.hh b/pdns/mtasker.hh
index fa1ede4127..62bf590987 100644
--- a/pdns/mtasker.hh
+++ b/pdns/mtasker.hh
@@ -46,7 +46,7 @@ struct KeyTag {};
     \note The EventKey needs to have an operator< defined because it is used as the key of an associative array
 */
 
-template<class EventKey=int, class EventVal=int> class MTasker
+template<class EventKey=int, class EventVal=int, class Cmp = std::less<EventKey>> class MTasker
 {
 private:
   pdns_ucontext_t d_kernel;
@@ -87,10 +87,10 @@ public:
   typedef multi_index_container<
     Waiter,
     indexed_by <
-                ordered_unique<member<Waiter,EventKey,&Waiter::key> >,
-                ordered_non_unique<tag<KeyTag>, member<Waiter,struct timeval,&Waiter::ttd> >
-               >
-  > waiters_t;
+      ordered_unique<member<Waiter,EventKey,&Waiter::key>, Cmp>,
+      ordered_non_unique<tag<KeyTag>, member<Waiter,struct timeval,&Waiter::ttd> >
+      >
+    > waiters_t;
 
   waiters_t d_waiters;
 
diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc
index 64d5ede01b..172421a34a 100644
--- a/pdns/pdns_recursor.cc
+++ b/pdns/pdns_recursor.cc
@@ -5456,7 +5456,7 @@ try
     t_bogusqueryring = std::unique_ptr<boost::circular_buffer<pair<DNSName, uint16_t> > >(new boost::circular_buffer<pair<DNSName, uint16_t> >());
     t_bogusqueryring->set_capacity(ringsize);
   }
-  MT=std::unique_ptr<MTasker<std::shared_ptr<PacketID>,PacketBuffer> >(new MTasker<std::shared_ptr<PacketID>,PacketBuffer>(::arg().asNum("stack-size")));
+  MT = std::make_unique<MT_t>(::arg().asNum("stack-size"));
   threadInfo.mt = MT.get();
 
   /* start protobuf export threads if needed */
diff --git a/pdns/syncres.hh b/pdns/syncres.hh
index ae5a7e5dd5..acca4f9951 100644
--- a/pdns/syncres.hh
+++ b/pdns/syncres.hh
@@ -967,21 +967,15 @@ struct PacketID
 
   bool operator<(const PacketID& b) const
   {
-    if (tie(remote, tcpsock, type) < tie(b.remote, b.tcpsock, b.type)) {
-      return true;
-    }
-    if (tie(remote, tcpsock, type) > tie(b.remote, b.tcpsock, b.type)) {
-      return false;
-    }
-
-    return tie(domain, fd, id) < tie(b.domain, b.fd, b.id);
+    // We don't want explcit PacketID compare here, but always via predicate classes below
+    assert(0);
   }
 };
 
 inline ostream& operator<<(ostream & os, const PacketID& pid)
 {
   return os << "PacketID(id=" << pid.id << ",remote=" << pid.remote.toString() << ",type=" << pid.type << ",tcpsock=" <<
-    pid.tcpsock << "fd=" << pid.fd << ',' << pid.domain << ')';
+    pid.tcpsock << ",fd=" << pid.fd << ',' << pid.domain << ')';
 }
 
 inline ostream& operator<<(ostream & os, const shared_ptr<PacketID>& pid)
@@ -989,10 +983,20 @@ inline ostream& operator<<(ostream & os, const shared_ptr<PacketID>& pid)
   return os << *pid;
 }
 
-inline bool operator<(const std::shared_ptr<PacketID>& a, const std::shared_ptr<PacketID>& b)
+struct PacketIDCompare
 {
-  return a->operator<(*b);
-}
+  bool operator()(const std::shared_ptr<PacketID>& a, const std::shared_ptr<PacketID>& b) const
+  {
+    if (tie(a->remote, a->tcpsock, a->type) < tie(b->remote, b->tcpsock, b->type)) {
+      return true;
+    }
+    if (tie(a->remote, a->tcpsock, a->type) > tie(b->remote, b->tcpsock, b->type)) {
+      return false;
+    }
+
+    return tie(a->domain, a->fd, a->id) < tie(b->domain, b->fd, b->id);
+  }
+};
 
 struct PacketIDBirthdayCompare
 {
@@ -1009,7 +1013,7 @@ struct PacketIDBirthdayCompare
 };
 extern std::unique_ptr<MemRecursorCache> g_recCache;
 extern thread_local std::unique_ptr<RecursorPacketCache> t_packetCache;
-typedef MTasker<std::shared_ptr<PacketID>,PacketBuffer> MT_t;
+typedef MTasker<std::shared_ptr<PacketID>, PacketBuffer, PacketIDCompare> MT_t;
 MT_t* getMT();
 
 struct RecursorStats