From f54067bf7f6d79dd2dcaddbdf43d720e4ddf36ee Mon Sep 17 00:00:00 2001 From: Yann Ylavic Date: Wed, 3 Jan 2018 09:49:38 +0000 Subject: Merge r1819857, r1819858, r1819860, r1819861, r1819934, r1819935 from trunk: testpoll: check that the wakeup pipe is still in the pollset after returning from poll(), e.g. APR_POLLSET_PORT should re-arm it automatically. poll, port: re-add the wakeup pipe to the pollset after it triggered. Just like user fds (file, socket), otherwise it's one shot only (PR-61786). Corresponding test committed in r1819857. poll, port: no need to release and re-acquire the lock in between walking the pollset and handling the dead ring, all is simple/fast/nonblocking ops. Also, set types of "i" and "j" respectively to the ones of nget and *num. poll, port: follow up to r1819860. This test is redundant now, axe it (no functional change). poll, kqueue: save a pollfd (mem)copy per returned event. poll, epoll: pollset's pfd is not modified on poll(), mark it const. Reviewed/backported by: ylavic git-svn-id: https://svn.apache.org/repos/asf/apr/apr/branches/1.7.x@1819937 13f79535-47bb-0310-9956-ffa450edef68 --- poll/unix/epoll.c | 2 +- poll/unix/kqueue.c | 10 ++++---- poll/unix/port.c | 74 ++++++++++++++++++++++++------------------------------ test/testpoll.c | 17 +++++++++---- 4 files changed, 51 insertions(+), 52 deletions(-) diff --git a/poll/unix/epoll.c b/poll/unix/epoll.c index 02db48a40..4ab03f67c 100644 --- a/poll/unix/epoll.c +++ b/poll/unix/epoll.c @@ -274,7 +274,7 @@ static apr_status_t impl_pollset_poll(apr_pollset_t *pollset, } else { int i, j; - apr_pollfd_t *fdptr; + const apr_pollfd_t *fdptr; for (i = 0, j = 0; i < ret; i++) { if (pollset->flags & APR_POLLSET_NOCOPY) { diff --git a/poll/unix/kqueue.c b/poll/unix/kqueue.c index 6a0ae614a..548464db1 100644 --- a/poll/unix/kqueue.c +++ b/poll/unix/kqueue.c @@ -257,7 +257,6 @@ static apr_status_t impl_pollset_poll(apr_pollset_t *pollset, int ret; struct timespec tv, *tvptr; apr_status_t rv = APR_SUCCESS; - apr_pollfd_t fd; *num = 0; @@ -280,17 +279,18 @@ static apr_status_t impl_pollset_poll(apr_pollset_t *pollset, } else { int i, j; + const apr_pollfd_t *fd; for (i = 0, j = 0; i < ret; i++) { - fd = (((pfd_elem_t *)(pollset->p->ke_set[i].udata))->pfd); + fd = &((pfd_elem_t *)pollset->p->ke_set[i].udata)->pfd; if ((pollset->flags & APR_POLLSET_WAKEABLE) && - fd.desc_type == APR_POLL_FILE && - fd.desc.f == pollset->wakeup_pipe[0]) { + fd->desc_type == APR_POLL_FILE && + fd->desc.f == pollset->wakeup_pipe[0]) { apr_poll_drain_wakeup_pipe(pollset->wakeup_pipe); rv = APR_EINTR; } else { - pollset->p->result_set[j] = fd; + pollset->p->result_set[j] = *fd; pollset->p->result_set[j].rtnevents = get_kqueue_revent(pollset->p->ke_set[i].filter, pollset->p->ke_set[i].flags); diff --git a/poll/unix/port.c b/poll/unix/port.c index a436f62d8..c1e599412 100644 --- a/poll/unix/port.c +++ b/poll/unix/port.c @@ -354,11 +354,11 @@ static apr_status_t impl_pollset_poll(apr_pollset_t *pollset, const apr_pollfd_t **descriptors) { apr_os_sock_t fd; - int ret, i, j; - unsigned int nget; + int ret; + unsigned int nget, i; + apr_int32_t j; pfd_elem_t *ep; apr_status_t rv = APR_SUCCESS; - apr_pollfd_t fp; *num = 0; nget = 1; @@ -404,48 +404,40 @@ static apr_status_t impl_pollset_poll(apr_pollset_t *pollset, port_associate within apr_pollset_add() */ apr_atomic_dec32(&pollset->p->waiting); - if (nget) { - - pollset_lock_rings(); + pollset_lock_rings(); - for (i = 0, j = 0; i < nget; i++) { - fp = (((pfd_elem_t*)(pollset->p->port_set[i].portev_user))->pfd); - if ((pollset->flags & APR_POLLSET_WAKEABLE) && - fp.desc_type == APR_POLL_FILE && - fp.desc.f == pollset->wakeup_pipe[0]) { - apr_poll_drain_wakeup_pipe(pollset->wakeup_pipe); - rv = APR_EINTR; - } - else { - pollset->p->result_set[j] = fp; - pollset->p->result_set[j].rtnevents = - get_revent(pollset->p->port_set[i].portev_events); - - /* If the ring element is still on the query ring, move it - * to the add ring for re-association with the event port - * later. (It may have already been moved to the dead ring - * by a call to pollset_remove on another thread.) - */ - ep = (pfd_elem_t *)pollset->p->port_set[i].portev_user; - if (ep->on_query_ring) { - APR_RING_REMOVE(ep, link); - ep->on_query_ring = 0; - APR_RING_INSERT_TAIL(&(pollset->p->add_ring), ep, - pfd_elem_t, link); - } - j++; - } + for (i = 0, j = 0; i < nget; i++) { + ep = (pfd_elem_t *)pollset->p->port_set[i].portev_user; + if ((pollset->flags & APR_POLLSET_WAKEABLE) && + ep->pfd.desc_type == APR_POLL_FILE && + ep->pfd.desc.f == pollset->wakeup_pipe[0]) { + apr_poll_drain_wakeup_pipe(pollset->wakeup_pipe); + rv = APR_EINTR; } - pollset_unlock_rings(); - if ((*num = j)) { /* any event besides wakeup pipe? */ - rv = APR_SUCCESS; - if (descriptors) { - *descriptors = pollset->p->result_set; - } + else { + pollset->p->result_set[j] = ep->pfd; + pollset->p->result_set[j].rtnevents = + get_revent(pollset->p->port_set[i].portev_events); + j++; + } + /* If the ring element is still on the query ring, move it + * to the add ring for re-association with the event port + * later. (It may have already been moved to the dead ring + * by a call to pollset_remove on another thread.) + */ + if (ep->on_query_ring) { + APR_RING_REMOVE(ep, link); + ep->on_query_ring = 0; + APR_RING_INSERT_TAIL(&(pollset->p->add_ring), ep, + pfd_elem_t, link); + } + } + if ((*num = j)) { /* any event besides wakeup pipe? */ + rv = APR_SUCCESS; + if (descriptors) { + *descriptors = pollset->p->result_set; } } - - pollset_lock_rings(); /* Shift all PFDs in the Dead Ring to the Free Ring */ APR_RING_CONCAT(&(pollset->p->free_ring), &(pollset->p->dead_ring), pfd_elem_t, link); diff --git a/test/testpoll.c b/test/testpoll.c index 644983df3..9f90af2dd 100644 --- a/test/testpoll.c +++ b/test/testpoll.c @@ -783,6 +783,7 @@ static void pollset_wakeup(abts_case *tc, void *data) apr_pollset_t *pollset; apr_int32_t num; const apr_pollfd_t *descriptors; + int i; rv = apr_pollset_create_ex(&pollset, 1, p, APR_POLLSET_WAKEABLE, default_pollset_impl); @@ -792,12 +793,18 @@ static void pollset_wakeup(abts_case *tc, void *data) } ABTS_INT_EQUAL(tc, APR_SUCCESS, rv); - /* send wakeup but no data; apr_pollset_poll() should return APR_EINTR */ - rv = apr_pollset_wakeup(pollset); - ABTS_INT_EQUAL(tc, APR_SUCCESS, rv); + /* Send wakeup but no data; apr_pollset_poll() should return APR_EINTR. + * Do it twice to test implementations that need to re-arm the events after + * poll()ing (e.g. APR_POLLSET_PORT), hence verify that the wakeup pipe is + * still in the place afterward. + */ + for (i = 0; i < 2; ++i) { + rv = apr_pollset_wakeup(pollset); + ABTS_INT_EQUAL(tc, APR_SUCCESS, rv); - rv = apr_pollset_poll(pollset, -1, &num, &descriptors); - ABTS_INT_EQUAL(tc, APR_EINTR, rv); + rv = apr_pollset_poll(pollset, -1, &num, &descriptors); + ABTS_INT_EQUAL(tc, APR_EINTR, rv); + } /* send wakeup and data; apr_pollset_poll() should return APR_SUCCESS */ socket_pollfd.desc_type = APR_POLL_SOCKET; -- cgit v1.2.1