From 3e76cf5f2cd71d8d67a2ed529a86febffa43fe73 Mon Sep 17 00:00:00 2001
From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Fri, 10 May 2019 10:26:15 +0200
Subject: [PATCH] Use a condition variable and restore the loop to be more like
 the original code. Separating slave and master comms threads can come later.

---
 pdns/communicator.cc       | 27 +++++++++++++++++++++------
 pdns/communicator.hh       |  4 +++-
 pdns/mastercommunicator.cc |  2 +-
 pdns/slavecommunicator.cc  |  4 ++--
 pdns/unix_semaphore.cc     |  6 ------
 pdns/utility.hh            |  3 ---
 6 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/pdns/communicator.cc b/pdns/communicator.cc
index 3e2ce98bb..836f07e59 100644
--- a/pdns/communicator.cc
+++ b/pdns/communicator.cc
@@ -113,19 +113,34 @@ void CommunicatorClass::mainloop(void)
     d_tickinterval=::arg().asNum("slave-cycle-interval");
     makeNotifySockets();
 
-    for (;;) {
+    for(;;) {
       slaveRefresh(&P);
       masterUpdateCheck(&P);
 
       time_t tick = doNotifications(); // this processes any notification acknowledgements and actually send out our own notifications
       tick = min(tick, d_tickinterval);
-      struct timespec abs_time = {.tv_sec = time(0) + tick, .tv_nsec = 0};
+      time_t next = time(0) + tick;
 
-      // Wait for a post for a max time. We might get EINTR, oh well
-      d_any_sem.timedWait(abs_time);
+      while (time(0) < next) {
+        std::unique_lock<std::mutex> lk(d_any_mutex);
+        auto rc = d_any_condvar.wait_for(lk, std::chrono::seconds(1));
+        lk.unlock();
 
-      while (d_any_sem.tryWait() == 0) {
-       // eat up remaining posts, will do next iteration shortly
+        if (rc == std::cv_status::timeout) {
+          bool extraSlaveRefresh = false;
+          {
+            Lock l(&d_lock);
+            if (d_tocheck.size())
+              extraSlaveRefresh = true;
+          }
+          if (extraSlaveRefresh)
+            slaveRefresh(&P);
+        }
+        else {
+          break; // something happened
+        }
+        // this gets executed at least once every second
+        doNotifications();
       }
     }
   }
diff --git a/pdns/communicator.hh b/pdns/communicator.hh
index c63534dc2..d7d7178a7 100644
--- a/pdns/communicator.hh
+++ b/pdns/communicator.hh
@@ -25,6 +25,7 @@
 #include <pthread.h>
 #include <string>
 #include <semaphore.h>
+#include <condition_variable>
 #include <queue>
 #include <list>
 #include <limits>
@@ -205,7 +206,8 @@ private:
   set<DNSName> d_inprogress;
   
   Semaphore d_suck_sem;
-  Semaphore d_any_sem;
+  std::condition_variable d_any_condvar;
+  std::mutex d_any_mutex;
   time_t d_tickinterval;
   set<DomainInfo> d_tocheck;
   struct cmp {
diff --git a/pdns/mastercommunicator.cc b/pdns/mastercommunicator.cc
index 048034749..5d6c97459 100644
--- a/pdns/mastercommunicator.cc
+++ b/pdns/mastercommunicator.cc
@@ -303,5 +303,5 @@ void CommunicatorClass::makeNotifySockets()
 void CommunicatorClass::notify(const DNSName &domain, const string &ip)
 {
   d_nq.add(domain, ip);
-  d_any_sem.post();
+  d_any_condvar.notify_one();
 }
diff --git a/pdns/slavecommunicator.cc b/pdns/slavecommunicator.cc
index 190488faf..50cf01c0b 100644
--- a/pdns/slavecommunicator.cc
+++ b/pdns/slavecommunicator.cc
@@ -727,7 +727,7 @@ void CommunicatorClass::addSlaveCheckRequest(const DomainInfo& di, const ComboAd
   }
   d_tocheck.erase(di);
   d_tocheck.insert(ours);
-  d_any_sem.post(); // kick the loop!
+  d_any_condvar.notify_one(); // kick the loop!
 }
 
 void CommunicatorClass::addTrySuperMasterRequest(DNSPacket *p)
@@ -735,7 +735,7 @@ void CommunicatorClass::addTrySuperMasterRequest(DNSPacket *p)
   Lock l(&d_lock);
   DNSPacket ours = *p;
   if(d_potentialsupermasters.insert(ours).second)
-    d_any_sem.post(); // kick the loop!
+    d_any_condvar.notify_one(); // kick the loop!
 }
 
 void CommunicatorClass::slaveRefresh(PacketHandler *P)
diff --git a/pdns/unix_semaphore.cc b/pdns/unix_semaphore.cc
index bac933c0e..a4b25b6dd 100644
--- a/pdns/unix_semaphore.cc
+++ b/pdns/unix_semaphore.cc
@@ -168,12 +168,6 @@ int Semaphore::wait()
   while (ret == -1 && errno == EINTR);
   return ret;
 }
-
-int Semaphore::timedWait(const struct timespec& abs_timeout)
-{
- return sem_timedwait(m_pSemaphore, &abs_timeout);
-}
-
 int Semaphore::tryWait()
 {
   return sem_trywait(m_pSemaphore);
diff --git a/pdns/utility.hh b/pdns/utility.hh
index 30ea93230..024fc089f 100644
--- a/pdns/utility.hh
+++ b/pdns/utility.hh
@@ -78,9 +78,6 @@ public:
   //! Waits for a semaphore.
   int wait( void );
 
-  //! Waits for a semaphore with a timeout
-  int timedWait(const struct timespec &abs_timeout);
-
   //! Tries to wait for a semaphore.
   int tryWait( void );
 
-- 
2.50.0