Do not hold the lock while running the task.

PLus some refactoring.

diff --git a/pdns/recursordist/rec-taskqueue.cc b/pdns/recursordist/rec-taskqueue.cc
index 60e1d48e6bb2e439f46588864c13b4d1e311c46e..cc1e71f26591e7078eaa79232b3cfbbab307fbdb 100644 (file)
--- a/pdns/recursordist/rec-taskqueue.cc
+++ b/pdns/recursordist/rec-taskqueue.cc
@@ -71,7 +71,18 @@ static void resolve(const struct timeval& now, bool logErrors, const pdns::Resol
 
 void runTaskOnce(bool logErrors)
 {
-  s_taskQueue.lock()->runOnce(logErrors);
+  pdns::ResolveTask task;
+  {
+    auto lock = s_taskQueue.lock();
+    if (lock->empty()) {
+      return;
+    }
+    task = lock->pop();
+  }
+  bool expired = task.run(logErrors);
+  if (expired) {
+    s_taskQueue.lock()->incExpired();
+  }
 }
 
 void pushAlmostExpiredTask(const DNSName& qname, uint16_t qtype, time_t deadline)

diff --git a/pdns/recursordist/rec-taskqueue.hh b/pdns/recursordist/rec-taskqueue.hh
index 2e9df555233e941feb43c7a3db8bd9ce79b2580b..95e734b68dd7236287472097c97e84e218d24409 100644 (file)
--- a/pdns/recursordist/rec-taskqueue.hh
+++ b/pdns/recursordist/rec-taskqueue.hh
@@ -25,6 +25,7 @@
 
 void runTaskOnce(bool logErrors);
 void pushAlmostExpiredTask(const DNSName& qname, uint16_t qtype, time_t deadline);
+
 // General task stats
 uint64_t getTaskPushes();
 uint64_t getTaskExpired();

diff --git a/pdns/recursordist/taskqueue.cc b/pdns/recursordist/taskqueue.cc
index 817bd94e21e248510662e7caeb626b3427eebb45..cc1a4b4be7c8286f1f8e49ced1105f4c1f575e4b 100644 (file)
--- a/pdns/recursordist/taskqueue.cc
+++ b/pdns/recursordist/taskqueue.cc
@@ -28,20 +28,9 @@
 namespace pdns
 {
 
-bool TaskQueue::empty() const
-{
-  return d_queue.empty();
-}
-
-size_t TaskQueue::size() const
-{
-  return d_queue.size();
-}
-
 void TaskQueue::push(ResolveTask&& task)
 {
   // Insertion fails if it's already there, no problem since we're already scheduled
-  // and the deadline would remain the same anyway.
   auto result = d_queue.insert(std::move(task));
   if (result.second) {
     d_pushes++;
@@ -55,34 +44,23 @@ ResolveTask TaskQueue::pop()
   return ret;
 }
 
-bool TaskQueue::runOnce(bool logErrors)
+bool ResolveTask::run(bool logErrors)
 {
-  if (d_queue.empty()) {
+  if (!d_func) {
+    g_log << Logger::Debug << "TaskQueue: null task for " << d_qname.toString() << '|' << QType(d_qtype).toString() << endl;
     return false;
   }
-  ResolveTask task = pop();
-  if (task.func == nullptr) {
-    g_log << Logger::Debug << "TaskQueue: null task for " << task.d_qname.toString() << '|' << QType(task.d_qtype).toString() << endl;
-    return true;
-  }
   struct timeval now;
   Utility::gettimeofday(&now);
-  if (task.d_deadline >= now.tv_sec) {
-    task.func(now, logErrors, task);
+  if (d_deadline >= now.tv_sec) {
+    d_func(now, logErrors, *this);
   }
   else {
     // Deadline passed
-    g_log << Logger::Debug << "TaskQueue: deadline for " << task.d_qname.toString() << '|' << QType(task.d_qtype).toString() << " passed" << endl;
-    d_expired++;
-  }
-  return true;
-}
-
-void TaskQueue::runAll(bool logErrors)
-{
-  while (runOnce(logErrors)) {
-    /* empty */
+    g_log << Logger::Debug << "TaskQueue: deadline for " << d_qname.toString() << '|' << QType(d_qtype).toString() << " passed" << endl;
+    return true;
   }
+  return false;
 }
 
 } /* namespace pdns */

diff --git a/pdns/recursordist/taskqueue.hh b/pdns/recursordist/taskqueue.hh
index 3094079ae8c52227fe62e7015e316ccfaffb12ae..55c307a9021a3d579014682cb38591cf7e7d955a 100644 (file)
--- a/pdns/recursordist/taskqueue.hh
+++ b/pdns/recursordist/taskqueue.hh
@@ -47,36 +47,26 @@ struct ResolveTask
   uint16_t d_qtype;
   time_t d_deadline;
   bool d_refreshMode; // Whether to run this task in regular mode (false) or in the mode that refreshes almost expired tasks
-  void (*func)(const struct timeval&, bool logErrors, const ResolveTask&);
-};
+  std::function<void(const struct timeval& now, bool logErrors, const ResolveTask& task)> d_func;
 
-struct HashTag
-{
+  bool run(bool logErrors);
 };
-struct SequencedTag
-{
-};
-
-typedef multi_index_container<
-  ResolveTask,
-  indexed_by<
-    hashed_unique<tag<HashTag>,
-                  composite_key<ResolveTask,
-                                member<ResolveTask, DNSName, &ResolveTask::d_qname>,
-                                member<ResolveTask, uint16_t, &ResolveTask::d_qtype>,
-                                member<ResolveTask, bool, &ResolveTask::d_refreshMode>>>,
-    sequenced<tag<SequencedTag>>>>
-  queue_t;
 
 class TaskQueue
 {
 public:
-  bool empty() const;
-  size_t size() const;
+  bool empty() const
+  {
+    return d_queue.empty();
+  }
+
+  size_t size() const
+  {
+    return d_queue.size();
+  }
+
   void push(ResolveTask&& task);
   ResolveTask pop();
-  bool runOnce(bool logErrors); // Run one task if the queue is not empty
-  void runAll(bool logErrors);
 
   uint64_t getPushes()
   {
@@ -88,7 +78,29 @@ public:
     return d_expired;
   }
 
+  void incExpired()
+  {
+    d_expired++;
+  }
+
 private:
+  struct HashTag
+  {
+  };
+  struct SequencedTag
+  {
+  };
+  typedef multi_index_container<
+    ResolveTask,
+    indexed_by<
+      hashed_unique<tag<HashTag>,
+                    composite_key<ResolveTask,
+                                  member<ResolveTask, DNSName, &ResolveTask::d_qname>,
+                                  member<ResolveTask, uint16_t, &ResolveTask::d_qtype>,
+                                  member<ResolveTask, bool, &ResolveTask::d_refreshMode>>>,
+      sequenced<tag<SequencedTag>>>>
+    queue_t;
+
   queue_t d_queue;
   uint64_t d_pushes{0};
   uint64_t d_expired{0};