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: http://svn.apache.org/repos/asf/apr/apr/branches/1.6.x@1819938 13f79535-47bb-0310-9956-ffa450edef68

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;