[PATCH] poll: wait until timeout on pipes (win32)

[Edward Thomson]

Hello-

I'm very interested in seeing the poll(2) compatibility function wait
until the timeout has elapsed before returning on win32.  The manual
indicates that "under Windows, when passing a pipe, Gnulib's poll
replacement might return 0 even before the timeout has passed."

Callers expecting poll(2) to honor the timeout may devolve into
pathological behavior when this expectation is violated.  For example,
git uses poll's timeout to send a keep-alive packet to the remote
socket:

https://github.com/git/git/blob/master/upload-pack.c#L170 and
https://github.com/git/git/blob/master/upload-pack.c#L234

When the gnulib compatibility poll is used in Git for Windows and
we are preparing a packfile for a repository (and thus, taking
some time to do so), poll will return 0 immediately, causing us to
send keep-alives as quickly as possible until the packfile has been
created.  This can result in megabytes (or even gigabytes) of keep-
alive packets being sent.

GetTickCount is used as it is efficient, stable and monotonically
increasing.  (Neither GetSystemTime nor QueryPerformanceCounter are.)

I didn't immediately see a pattern where a test is optional.  I
would be loathe to have a test with a sleep in it running by default.
I would appreciate advice on how to mitigate this.

Thanks!

-ed

---
 lib/poll.c        |  7 +++++--
 tests/test-poll.c | 24 ++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/lib/poll.c b/lib/poll.c
index a3e0ab7..9bba316 100644
--- a/lib/poll.c
+++ b/lib/poll.c
@@ -445,7 +445,7 @@ poll (struct pollfd *pfd, nfds_t nfd, int timeout)
   static HANDLE hEvent;
   WSANETWORKEVENTS ev;
   HANDLE h, handle_array[FD_SETSIZE + 2];
-  DWORD ret, wait_timeout, nhandles;
+  DWORD ret, wait_timeout, nhandles, start;
   fd_set rfds, wfds, xfds;
   BOOL poll_again;
   MSG msg;
@@ -458,6 +458,9 @@ poll (struct pollfd *pfd, nfds_t nfd, int timeout)
       return -1;
     }
 
+  if (timeout)
+    start = GetTickCount();
+
   if (!hEvent)
     hEvent = CreateEvent (NULL, FALSE, FALSE, NULL);
 
@@ -602,7 +605,7 @@ restart:
         rc++;
     }
 
-  if (!rc && timeout == INFTIM)
+  if (!rc && timeout && GetTickCount() - start < timeout)
     {
       SleepEx (1, TRUE);
       goto restart;
diff --git a/tests/test-poll.c b/tests/test-poll.c
index 0cdb1f9..2003b3e 100644
--- a/tests/test-poll.c
+++ b/tests/test-poll.c
@@ -362,6 +362,29 @@ test_pipe (void)
   close (fd[1]);
 }
 
+/* Test that poll(2) will timeout.  */
+
+static void
+test_timeout (void)
+{
+  int fds[2];
+  time_t start, elapsed;
+
+  if (pipe(fds) < 0)
+    failed ("expected pipe to succeed");
+
+  start = time(NULL);
+
+  int ret = poll1(fds[1], POLLIN, 2000);
+  close(fds[0]);
+  close(fds[1]);
+
+  if (ret)
+    failed ("expected poll to not return a fd");
+
+  if (time(NULL) - start < 2)
+    failed ("expected poll to timeout");
+}
 
 /* Do them all.  */
 
@@ -379,6 +402,7 @@ main ()
   result += test (test_socket_pair, "Connected sockets test");
   result += test (test_accept_first, "General socket test with fork");
   result += test (test_pipe, "Pipe test");
+  result += test (test_timeout, "Poll should timeout");
 
   exit (result);
 }
-- 
1.9.2.msysgit.0