From f58cb6a386a3a09d2d44859869ac09cbaca80193 Mon Sep 17 00:00:00 2001
From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Tue, 11 Aug 2020 14:46:52 +0200
Subject: [PATCH] dnsdist: Handle EINTR in DelayPipe

Otherwise the DelayPipe thread might stop because the read() operation
has been interrupted by a signal (like SIGWINCH, which is quite annoying).
Also remove the unused `read()` operation.
---
 pdns/delaypipe.cc                     | 69 +++++++++++++--------------
 pdns/delaypipe.hh                     |  3 +-
 pdns/dnsdistdist/test-delaypipe_hh.cc |  6 +--
 3 files changed, 37 insertions(+), 41 deletions(-)

diff --git a/pdns/delaypipe.cc b/pdns/delaypipe.cc
index 721b611622..167f355fd7 100644
--- a/pdns/delaypipe.cc
+++ b/pdns/delaypipe.cc
@@ -59,45 +59,42 @@ void ObjectPipe<T>::write(T& t)
   }
 }
 
-template<class T>
-bool ObjectPipe<T>::read(T* t)
-{
-  T* ptr;
-  int ret = ::read(d_fds[0], &ptr, sizeof(ptr));
-
-  if(ret < 0)
-    unixDie("read");
-  if(ret==0)
-    return false;
-  if(ret != sizeof(ptr))
-    throw std::runtime_error("Partial read, should not happen");    
-  *t=*ptr;
-  delete ptr;
-  return true;
-}
-
 template<class T>
 int ObjectPipe<T>::readTimeout(T* t, double msec)
 {
-  T* ptr;
-
-  int ret = waitForData(d_fds[0], 0, 1000*msec);
-  if(ret < 0)
-    unixDie("waiting for data in object pipe");
-  if(ret == 0) 
-    return -1;
-
-  ret = ::read(d_fds[0], &ptr, sizeof(ptr)); // this is BLOCKING!
-
-  if(ret < 0)
-    unixDie("read");
-  if(ret==0)
-    return false;
-  if(ret != sizeof(ptr))
-    throw std::runtime_error("Partial read, should not happen 2");    
-  *t=*ptr;
-  delete ptr;
-  return 1;
+  while (true) {
+    int ret = waitForData(d_fds[0], 0, 1000*msec);
+    if (ret < 0) {
+      if (errno == EINTR) {
+        continue;
+      }
+      unixDie("waiting for data in object pipe");
+    }
+    else if (ret == 0) {
+      return -1;
+    }
+
+    T* ptr = nullptr;
+    ret = ::read(d_fds[0], &ptr, sizeof(ptr)); // this is BLOCKING!
+
+    if (ret < 0) {
+      if (errno == EINTR) {
+        continue;
+      }
+      unixDie("read");
+    }
+    else if (ret == 0) {
+      return false;
+    }
+
+    if (ret != sizeof(ptr)) {
+      throw std::runtime_error("Partial read, should not happen 2");
+    }
+
+    *t = *ptr;
+    delete ptr;
+    return 1;
+  }
 }
 
 
diff --git a/pdns/delaypipe.hh b/pdns/delaypipe.hh
index b44159132b..ad1626a999 100644
--- a/pdns/delaypipe.hh
+++ b/pdns/delaypipe.hh
@@ -43,8 +43,7 @@ public:
   ObjectPipe();
   ~ObjectPipe();
   void write(T& t);
-  bool read(T* t); // returns false on EOF
-  int readTimeout(T* t, double msec); //!< -1 is timeout, 0 is no data, 1 is data. msec<0 waits infinitely wrong. msec==0 = undefined
+  int readTimeout(T* t, double msec); //!< -1 is timeout, 0 is no data, 1 is data. msec<0 waits infinitely long. msec==0 = undefined
   void close(); 
 private:
   int d_fds[2];
diff --git a/pdns/dnsdistdist/test-delaypipe_hh.cc b/pdns/dnsdistdist/test-delaypipe_hh.cc
index 3d1c1b00df..e376cb9330 100644
--- a/pdns/dnsdistdist/test-delaypipe_hh.cc
+++ b/pdns/dnsdistdist/test-delaypipe_hh.cc
@@ -15,13 +15,13 @@ BOOST_AUTO_TEST_CASE(test_object_pipe) {
 
   int i;
   for(int n=0; n < 100; ++n) {
-    bool res=op.read(&i);
-    BOOST_CHECK_EQUAL(res, true);
+    int res=op.readTimeout(&i, -1);
+    BOOST_CHECK_EQUAL(res, 1);
     BOOST_CHECK_EQUAL(n, i);
   }
 
   op.close();
-  BOOST_CHECK_EQUAL(op.read(&i), false);
+  BOOST_CHECK_EQUAL(op.readTimeout(&i, 1), 0);
 
 };
 
-- 
2.47.2