Merge pull request #592 from capnproto/poll-eintr

Fix wrong poll/epoll timeout after EINTR.

c++/src/kj/async-unix-test.c++

@@ -35,6 +35,8 @@
 #include <pthread.h>
 #include <algorithm>
 #include <sys/wait.h>
+#include <sys/time.h>
+#include <errno.h>
 
 namespace kj {
 namespace {
@@ -576,6 +578,51 @@ TEST(AsyncUnixTest, SteadyTimers) {
   }
 }
 
+bool dummySignalHandlerCalled = false;
+void dummySignalHandler(int) {
+  dummySignalHandlerCalled = true;
+}
+
+TEST(AsyncUnixTest, InterruptedTimer) {
+  captureSignals();
+  UnixEventPort port;
+  EventLoop loop(port);
+  WaitScope waitScope(loop);
+
+#if __linux__
+  // Linux timeslices are 1ms.
+  constexpr auto OS_SLOWNESS_FACTOR = 1;
+#else
+  // OSX timeslices are 10ms, so we need longer timeouts to avoid flakiness.
+  // In fact, even 10x doesn't make OSX happy so we're pushing it to 100x.
+  // To be safe we'll assume other OS's are similar.
+  constexpr auto OS_SLOWNESS_FACTOR = 100;
+#endif
+
+  // Schedule a timer event in 10ms.
+  auto& timer = port.getTimer();
+  auto start = timer.now();
+  constexpr auto timeout = 10 * MILLISECONDS * OS_SLOWNESS_FACTOR;
+
+  // Arrange SIGALRM to be delivered in 5ms, handled in an empty signal handler. This will cause
+  // our wait to be interrupted with EINTR. We should nevertheless continue waiting for the right
+  // amount of time.
+  dummySignalHandlerCalled = false;
+  if (signal(SIGALRM, &dummySignalHandler) == SIG_ERR) {
+    KJ_FAIL_SYSCALL("signal(SIGALRM)", errno);
+  }
+  struct itimerval itv;
+  memset(&itv, 0, sizeof(itv));
+  itv.it_value.tv_usec = 5000 * OS_SLOWNESS_FACTOR;  // signal after 5ms
+  setitimer(ITIMER_REAL, &itv, nullptr);
+
+  timer.afterDelay(timeout).wait(waitScope);
+
+  KJ_EXPECT(dummySignalHandlerCalled);
+  KJ_EXPECT(timer.now() - start >= timeout);
+  KJ_EXPECT(timer.now() - start <= timeout + (timeout / 5));  // allow 2ms error
+}
+
 TEST(AsyncUnixTest, Wake) {
   captureSignals();
   UnixEventPort port;

c++/src/kj/async-unix.c++

@@ -542,8 +542,18 @@ bool UnixEventPort::doEpollWait(int timeout) {
   }
 
   struct epoll_event events[16];
-  int n;
-  KJ_SYSCALL(n = epoll_wait(epollFd, events, kj::size(events), timeout));
+  int n = epoll_wait(epollFd, events, kj::size(events), timeout);
+  if (n < 0) {
+    int error = errno;
+    if (error == EINTR) {
+      // We can't simply restart the epoll call because we need to recompute the timeout. Instead,
+      // we pretend epoll_wait() returned zero events. This will cause the event loop to spin once,
+      // decide it has nothing to do, recompute timeouts, then return to waiting.
+      n = 0;
+    } else {
+      KJ_FAIL_SYSCALL("epoll_wait()", error);
+    }
+  }
 
   bool woken = false;
 
@@ -724,13 +734,16 @@ public:
   }
 
   void run(int timeout) {
-    do {
-      pollResult = ::poll(pollfds.begin(), pollfds.size(), timeout);
-      pollError = pollResult < 0 ? errno : 0;
-
-      // EINTR should only happen if we received a signal *other than* the ones registered via
-      // the UnixEventPort, so we don't care about that case.
-    } while (pollError == EINTR);
+    pollResult = ::poll(pollfds.begin(), pollfds.size(), timeout);
+    pollError = pollResult < 0 ? errno : 0;
+
+    if (pollError == EINTR) {
+      // We can't simply restart the poll call because we need to recompute the timeout. Instead,
+      // we pretend poll() returned zero events. This will cause the event loop to spin once,
+      // decide it has nothing to do, recompute timeouts, then return to waiting.
+      pollResult = 0;
+      pollError = 0;
+    }
   }
 
   void processResults() {