apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Paul Querna <c...@force-elite.com>
Subject Re: [PATCH] bug in kqueue implementation of apr_pollset_poll (wrong conversion from usec)
Date Sat, 02 Sep 2006 21:59:45 GMT
Thank You, I have committed your patch in r439667 of trunk:

http://svn.apache.org/viewvc?view=rev&revision=439667


Marco Molteni wrote:
> Hi,
> 
> first of all, thanks for providing APR :-)
> 
> I think I've found a bug in the kqueue implementation of apr_pollset_poll().
> I am using FreeBSD -current with apr-1.2.7, but the SVN trunk
> version of kqueue.c has the same bug:
> 
> 
>     232  APR_DECLARE(apr_status_t) apr_pollset_poll(apr_pollset_t *pollset,
>     233                                             apr_interval_time_t timeout,
>     234                                             apr_int32_t *num,
>     235                                             const apr_pollfd_t **descriptors)
>     236  {
>     237      int ret, i;
>     238      struct timespec tv, *tvptr;
>     239      apr_status_t rv = APR_SUCCESS;
>     240  
>     241      if (timeout < 0) {
>     242          tvptr = NULL;
>     243      }
>     244      else {
>     245          tv.tv_sec = (long) apr_time_sec(timeout);
>     246          tv.tv_nsec = (long) apr_time_msec(timeout);
>     247          tvptr = &tv;
>     248      }
> 
> 
> lines 245,246 convert from timeout (usec) to struct timespec, but line 246
> assigns milliseconds to nanoseconds, with a wrong magnitude of 10^6.
> 
> Net effect is that a timeout of say 999 msec becomes a timeout of 999 nsec.
> 
> Please find attached a proposed patch and a test program to expose the bug.
> 
> thanks
> marco
> 
> 
> 
> 
> ------------------------------------------------------------------------
> 
> --- kqueue.c.orig	Fri Aug 18 16:44:49 2006
> +++ kqueue.c	Fri Aug 18 16:51:06 2006
> @@ -240,13 +240,13 @@ APR_DECLARE(apr_status_t) apr_pollset_po
>  
>      if (timeout < 0) {
>          tvptr = NULL;
>      }
>      else {
>          tv.tv_sec = (long) apr_time_sec(timeout);
> -        tv.tv_nsec = (long) apr_time_msec(timeout);
> +        tv.tv_nsec = (long) apr_time_usec(timeout) * 1000;
>          tvptr = &tv;
>      }
>  
>      ret = kevent(pollset->kqueue_fd, NULL, 0, pollset->ke_set, pollset->nalloc,
>                  tvptr);
>      (*num) = ret;
> 
> 
> ------------------------------------------------------------------------
> 
> /* apr_kqueue_bug.c
>    Expose a bug in the kqueue version of apr_pollset_poll().
> 
>    compile with:
>    cc -W -Wall -Werror -o apr_kqueue_bug apr_kqueue_bug.c \
>        -I/usr/local/include/apr-1 -L /usr/local/lib -lapr-1
>  */
> 
> #include <stdio.h>
> #include <stdlib.h>
> #include <apr_poll.h>
> 
> #define CHECK(x) do { \
>     rv = x; \
>     if (rv != APR_SUCCESS) { printf("%s %s", #x, errmsg(rv)); exit(1); } \
>     } while (0)
> 
> const char *
> errmsg(apr_status_t rv)
> {
>     static char buf[80];
> 
>     if (rv == APR_SUCCESS) {
>         return "success";
>     } else {
>         apr_strerror(rv, buf, sizeof buf);
>         return buf;
>     }
> }
> 
> void mywait(apr_pollset_t *pollset, apr_time_t timeout_us)
> {
>     apr_status_t rv;
>     apr_time_t   start_us, stop_us;
>     apr_pollfd_t const *ret_pfd;
>     int          num_pfd = 0;
> 
>     start_us = apr_time_now();
>     rv = apr_pollset_poll(pollset, timeout_us, &num_pfd, &ret_pfd);
>     stop_us = apr_time_now();
>     if (! APR_STATUS_IS_TIMEUP(rv)) {
>         printf("unexpected apr_pollset_poll %s", errmsg(rv));
>     }
>     printf("wait: asked %lld ms, waited %lld ms\n",
>         apr_time_as_msec(timeout_us), apr_time_as_msec(stop_us - start_us));
> }
> 
> int main()
> {
>     apr_status_t   rv;
>     apr_pollset_t *pollset;
>     apr_pool_t    *pool;
>     apr_pollfd_t   pfd;
>     apr_socket_t  *sock;
> 
>     CHECK(apr_initialize());
>     CHECK(apr_pool_create(&pool, NULL));
>     CHECK(apr_pollset_create(&pollset, 512, pool, 0));
>     CHECK(apr_socket_create(&sock, APR_INET, SOCK_STREAM, APR_PROTO_TCP, pool));
> 
>     pfd.desc_type = APR_POLL_SOCKET;
>     pfd.reqevents = APR_POLLIN;
>     pfd.desc.s = sock;
>     CHECK(apr_pollset_add(pollset, &pfd));
> 
>     mywait(pollset,  999 * 1000); // 999 ms  => will wait 0 ms
>     mywait(pollset, 1999 * 1000); // 1999 ms => will wait 1000 ms
> 
>     return 0;
> }


Mime
View raw message