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
