poll: Round up milliseconds timeouts.

When converting appr_interval_time_t (usecs) to system call's msecs, round up.
apr_*poll() calls should wait *at least* the given timeout, not less.


poll: Follow up to r1902236: Fix poll() sleeps cases.

Don't convert timeout to milliseconds before potentially callig apr_sleep().

Tests for "poll() didn't sleep" now use the real timeout as lower limit.


tests: check whether epoll_wait() timeout is reliable and adjust justsleep().

* configure.in:
  Small epoll_wait() loop to check timeout reliability and set
  HAVE_EPOLL_WAIT_RELIABLE_TIMEOUT.

* test/testpoll.c(justsleep):
  Allow some jiffy is !HAVE_EPOLL_WAIT_RELIABLE_TIMEOUT.


tests: Follow up to r1908616: Simplify epoll_wait() check.


Merges r1902236, r1902258, r1908616, r1908618 from trunk.
Merges r1909334 from 1.8.x.
Submitted by: ylavic


git-svn-id: https://svn.apache.org/repos/asf/apr/apr/branches/1.7.x@1909335 13f79535-47bb-0310-9956-ffa450edef68

8 files changed, 89 insertions, 24 deletions

diff --git a/configure.in b/configure.in
index ff92e55c0..95dd0dfb5 100644
--- a/configure.in
+++ b/configure.in
@@ -1055,7 +1055,7 @@ AC_CACHE_CHECK([for epoll support], [apr_cv_epoll],
 #include <sys/epoll.h>
 #include <unistd.h>
 
-int main()
+int main(int argc, const char *argv[])
 {
     return epoll_create(5) == -1;
 }], [apr_cv_epoll=yes], [apr_cv_epoll=no], [apr_cv_epoll=no])])
@@ -1071,7 +1071,7 @@ AC_CACHE_CHECK([for epoll_create1 support], [apr_cv_epoll_create1],
 #include <sys/epoll.h>
 #include <unistd.h>
 
-int main()
+int main(int argc, const char *argv[])
 {
     return epoll_create1(0) == -1;
 }], [apr_cv_epoll_create1=yes], [apr_cv_epoll_create1=no], [apr_cv_epoll_create1=no])])
@@ -1080,6 +1080,47 @@ if test "$apr_cv_epoll_create1" = "yes"; then
    AC_DEFINE([HAVE_EPOLL_CREATE1], 1, [Define if epoll_create1 function is supported])
 fi
 
+AC_CACHE_CHECK([whether epoll_wait has a reliable timeout (min)],
+               [apr_cv_epoll_wait_has_reliable_timeout],
+[AC_TRY_RUN([
+#include <unistd.h>
+#include <sys/epoll.h>
+#include <sys/time.h>   /* for gettimeofday */
+
+#define TV2US(tv) ((tv).tv_sec * 1000000LL + (tv).tv_usec)
+
+int main(int argc, const char *argv[])
+{
+    struct epoll_event events[1];
+    int fd, i;
+#ifdef HAVE_EPOLL_CREATE1
+    fd = epoll_create1(0);
+#else
+    fd = epoll_create(1);
+#endif
+    if (fd < 0) {
+        return 1;
+    }
+    for (i = 0; i < 10; ++i) {
+        struct timeval t1 = {0,},
+                       t2 = {0,};
+        (void)gettimeofday(&t1, NULL);
+        (void)epoll_wait(fd, events, 1, 100);  /* ms */
+        (void)gettimeofday(&t2, NULL);
+        if (TV2US(t2) - TV2US(t1) < 100000) {  /* us */
+            return 1;
+        }
+    }
+    return 0;
+}], [apr_cv_epoll_wait_has_reliable_timeout=yes],
+    [apr_cv_epoll_wait_has_reliable_timeout=no],
+    [apr_cv_epoll_wait_has_reliable_timeout=no])])
+
+if test "$apr_cv_epoll_wait_has_reliable_timeout" = "yes"; then
+   AC_DEFINE([HAVE_EPOLL_WAIT_RELIABLE_TIMEOUT], 1,
+             [Define if epoll_wait has a reliable timeout (min)])
+fi
+
 # test for dup3
 AC_CACHE_CHECK([for dup3 support], [apr_cv_dup3],
 [AC_TRY_RUN([
diff --git a/poll/os2/poll.c b/poll/os2/poll.c
index 3c36e5e68..a9d5153bf 100644
--- a/poll/os2/poll.c
+++ b/poll/os2/poll.c
@@ -61,7 +61,8 @@ APR_DECLARE(apr_status_t) apr_poll(apr_pollfd_t *aprset, apr_int32_t num,
     }
 
     if (timeout > 0) {
-        timeout /= 1000; /* convert microseconds to milliseconds */
+        /* convert microseconds to milliseconds (round up) */
+        timeout = (timeout + 999) / 1000;
     }
 
     i = select(pollset, num_read, num_write, num_except, timeout);
diff --git a/poll/os2/pollset.c b/poll/os2/pollset.c
index 87b3c1841..c03219957 100644
--- a/poll/os2/pollset.c
+++ b/poll/os2/pollset.c
@@ -223,7 +223,7 @@ APR_DECLARE(apr_status_t) apr_pollset_poll(apr_pollset_t *pollset,
     (*num) = 0;
 
     if (timeout > 0) {
-        timeout /= 1000;
+        timeout = (timeout + 999) / 1000;
     }
 
     rv = select(pollresult, pollset->num_read, pollset->num_write, pollset->num_except, timeout);
diff --git a/poll/unix/epoll.c b/poll/unix/epoll.c
index 4ab03f67c..ad3cc0b9d 100644
--- a/poll/unix/epoll.c
+++ b/poll/unix/epoll.c
@@ -261,7 +261,7 @@ static apr_status_t impl_pollset_poll(apr_pollset_t *pollset,
     *num = 0;
 
     if (timeout > 0) {
-        timeout /= 1000;
+        timeout = (timeout + 999) / 1000;
     }
 
     ret = epoll_wait(pollset->p->epoll_fd, pollset->p->pollset, pollset->nalloc,
@@ -442,7 +442,7 @@ static apr_status_t impl_pollcb_poll(apr_pollcb_t *pollcb,
     apr_status_t rv = APR_SUCCESS;
     
     if (timeout > 0) {
-        timeout /= 1000;
+        timeout = (timeout + 999) / 1000;
     }
     
     ret = epoll_wait(pollcb->fd, pollcb->pollset.epoll, pollcb->nalloc,
diff --git a/poll/unix/poll.c b/poll/unix/poll.c
index 5b878f15b..28090c4fa 100644
--- a/poll/unix/poll.c
+++ b/poll/unix/poll.c
@@ -114,7 +114,8 @@ APR_DECLARE(apr_status_t) apr_poll(apr_pollfd_t *aprset, apr_int32_t num,
     num_to_poll = i;
 
     if (timeout > 0) {
-        timeout /= 1000; /* convert microseconds to milliseconds */
+        /* convert microseconds to milliseconds (round up) */
+        timeout = (timeout + 999) / 1000;
     }
 
     i = poll(pollset, num_to_poll, timeout);
@@ -252,14 +253,15 @@ static apr_status_t impl_pollset_poll(apr_pollset_t *pollset,
         }
         return APR_SUCCESS;
     }
+#endif
+
     if (timeout > 0) {
-        timeout /= 1000;
+        timeout = (timeout + 999) / 1000;
     }
+
+#ifdef WIN32
     ret = WSAPoll(pollset->p->pollset, pollset->nelts, (int)timeout);
 #else
-    if (timeout > 0) {
-        timeout /= 1000;
-    }
     ret = poll(pollset->p->pollset, pollset->nelts, timeout);
 #endif
     if (ret < 0) {
@@ -407,14 +409,15 @@ static apr_status_t impl_pollcb_poll(apr_pollcb_t *pollcb,
         }
         return APR_SUCCESS;
     }
+#endif
+
     if (timeout > 0) {
-        timeout /= 1000;
+        timeout = (timeout + 999) / 1000;
     }
+
+#ifdef WIN32
     ret = WSAPoll(pollcb->pollset.ps, pollcb->nelts, (int)timeout);
 #else
-    if (timeout > 0) {
-        timeout /= 1000;
-    }
     ret = poll(pollcb->pollset.ps, pollcb->nelts, timeout);
 #endif
     if (ret < 0) {
diff --git a/poll/unix/z_asio.c b/poll/unix/z_asio.c
index 48b531cc8..e7d9d9da3 100644
--- a/poll/unix/z_asio.c
+++ b/poll/unix/z_asio.c
@@ -554,7 +554,7 @@ static posix_poll(apr_pollset_t *pollset,
     DBG(4, "entered\n");
 
     if (timeout > 0) {
-        timeout /= 1000;
+        timeout = (timeout + 999) / 1000;
     }
     rv = poll(priv->pollset, pollset->nelts, timeout);
     (*num) = rv;
@@ -698,6 +698,7 @@ static apr_status_t asio_pollset_poll(apr_pollset_t *pollset,
             tv.tv_nsec = apr_time_usec(timeout) * 1000;
         } else {
             tv.tv_sec = INT_MAX;  /* block until something is ready */
+            tv.tv_nsec = 0;
         }
 
         DBG2(6, "nothing on the ready ring "
diff --git a/support/unix/waitio.c b/support/unix/waitio.c
index fa5edb65e..a3c6a8349 100644
--- a/support/unix/waitio.c
+++ b/support/unix/waitio.c
@@ -42,10 +42,13 @@ apr_status_t apr_wait_for_io_or_timeout(apr_file_t *f, apr_socket_t *s,
     struct pollfd pfd;
     int rc, timeout;
 
-    timeout    = f        ? f->timeout / 1000 : s->timeout / 1000;
+    timeout    = f        ? f->timeout        : s->timeout;
     pfd.fd     = f        ? f->filedes        : s->socketdes;
     pfd.events = for_read ? POLLIN            : POLLOUT;
 
+    if (timeout > 0) {
+        timeout = (timeout + 999) / 1000;
+    }
     do {
         rc = poll(&pfd, 1, timeout);
     } while (rc == -1 && errno == EINTR);
diff --git a/test/testpoll.c b/test/testpoll.c
index 9f90af2dd..960b6a7ad 100644
--- a/test/testpoll.c
+++ b/test/testpoll.c
@@ -22,6 +22,13 @@
 #include "apr_network_io.h"
 #include "apr_poll.h"
 
+#if defined(__linux__)
+#include "arch/unix/apr_private.h"
+#endif
+#ifndef HAVE_EPOLL_WAIT_RELIABLE_TIMEOUT
+#define HAVE_EPOLL_WAIT_RELIABLE_TIMEOUT 0
+#endif
+
 #define SMALL_NUM_SOCKETS 3
 /* We can't use 64 here, because some platforms *ahem* Solaris *ahem* have
  * a default limit of 64 open file descriptors per process.  If we use
@@ -854,6 +861,16 @@ static void pollcb_wakeup(abts_case *tc, void *data)
     ABTS_INT_EQUAL(tc, APR_EINTR, rv);
 }
 
+#define JUSTSLEEP_DELAY apr_time_from_msec(200)
+#if HAVE_EPOLL_WAIT_RELIABLE_TIMEOUT
+#define JUSTSLEEP_ENOUGH(ts, te) \
+    ((te) - (ts) >= JUSTSLEEP_DELAY)
+#else
+#define JUSTSLEEP_JIFFY apr_time_from_msec(10)
+#define JUSTSLEEP_ENOUGH(ts, te) \
+    ((te) - (ts) >= JUSTSLEEP_DELAY - JUSTSLEEP_JIFFY)
+#endif
+
 static void justsleep(abts_case *tc, void *data)
 {
     apr_int32_t nsds;
@@ -872,13 +889,13 @@ static void justsleep(abts_case *tc, void *data)
 
     nsds = 1;
     t1 = apr_time_now();
-    rv = apr_poll(NULL, 0, &nsds, apr_time_from_msec(200));
+    rv = apr_poll(NULL, 0, &nsds, JUSTSLEEP_DELAY);
     t2 = apr_time_now();
     ABTS_INT_EQUAL(tc, 1, APR_STATUS_IS_TIMEUP(rv));
     ABTS_INT_EQUAL(tc, 0, nsds);
     ABTS_ASSERT(tc,
                 "apr_poll() didn't sleep",
-                (t2 - t1) > apr_time_from_msec(100));
+                JUSTSLEEP_ENOUGH(t1, t2));
 
     for (i = 0; i < sizeof methods / sizeof methods[0]; i++) {
         rv = apr_pollset_create_ex(&pollset, 5, p, 0, methods[i]);
@@ -887,14 +904,13 @@ static void justsleep(abts_case *tc, void *data)
 
             nsds = 1;
             t1 = apr_time_now();
-            rv = apr_pollset_poll(pollset, apr_time_from_msec(200), &nsds,
-                                  &hot_files);
+            rv = apr_pollset_poll(pollset, JUSTSLEEP_DELAY, &nsds, &hot_files);
             t2 = apr_time_now();
             ABTS_INT_EQUAL(tc, 1, APR_STATUS_IS_TIMEUP(rv));
             ABTS_INT_EQUAL(tc, 0, nsds);
             ABTS_ASSERT(tc,
                         "apr_pollset_poll() didn't sleep",
-                        (t2 - t1) > apr_time_from_msec(100));
+                        JUSTSLEEP_ENOUGH(t1, t2));
 
             rv = apr_pollset_destroy(pollset);
             ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
@@ -905,12 +921,12 @@ static void justsleep(abts_case *tc, void *data)
             ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
 
             t1 = apr_time_now();
-            rv = apr_pollcb_poll(pollcb, apr_time_from_msec(200), NULL, NULL);
+            rv = apr_pollcb_poll(pollcb, JUSTSLEEP_DELAY, NULL, NULL);
             t2 = apr_time_now();
             ABTS_INT_EQUAL(tc, 1, APR_STATUS_IS_TIMEUP(rv));
             ABTS_ASSERT(tc,
                         "apr_pollcb_poll() didn't sleep",
-                        (t2 - t1) > apr_time_from_msec(100));
+                        JUSTSLEEP_ENOUGH(t1, t2));
 
             /* no apr_pollcb_destroy() */
         }