Hi,
It's holiday season and things may be moving slower than it usually
is. The patch have been available for a while, and it fixes the port
code for Solaris which I really like to get it in.
It could be somewhat controversial for the atomic code used to defer
the port_associate in apr_pollset_add in order to pass the testpoll
code, which is the needed fix if we consider the semantic of
apr_pollset_poll should be giving accumulated events; Otherwise, we
should revise the test/testpoll.c.
With that said, I break the patch into two, the first port_1203-1.diff
fix bugs of:
1. Return APR_SUCCESS if the poll removed from pollset is in the
add_ring, which means the poll had been added but is current
disassociated because an event.
2. In apr_pollcb_poll, call apr_pollcb_add to ensure port_associate.
And the port_1203-2.diff is the fix to pass testpoll given current
situation, which is less a concern as I consider in most apps,
apr_pollset_poll is called in a loop.
Any comments are welcomed.
Cheers,
Henry
Index: poll/unix/port.c
===================================================================
--- poll/unix/port.c (revision 600793)
+++ poll/unix/port.c (working copy)
@@ -192,6 +192,7 @@
pfd_elem_t *ep;
apr_status_t rv = APR_SUCCESS;
int res;
+ int err;
pollset_lock_rings();
@@ -205,6 +206,7 @@
res = port_dissociate(pollset->port_fd, PORT_SOURCE_FD, fd);
if (res < 0) {
+ err = errno;
rv = APR_NOTFOUND;
}
@@ -233,6 +235,9 @@
APR_RING_REMOVE(ep, link);
APR_RING_INSERT_TAIL(&(pollset->dead_ring),
ep, pfd_elem_t, link);
+ if (ENOENT == err) {
+ rv = APR_SUCCESS;
+ }
break;
}
}
@@ -464,6 +469,7 @@
if (rv) {
return rv;
}
+ rv = apr_pollcb_add(pollcb, pollfd);
}
}
--- poll/unix/port_1.c Mon Dec 3 21:37:14 2007
+++ poll/unix/port.c Mon Dec 3 21:37:41 2007
@@ -15,6 +15,7 @@
*/
#include "apr_arch_poll_private.h"
+#include "apr_atomic.h"
#ifdef POLLSET_USES_PORT
@@ -80,6 +81,8 @@
/* A ring of pollfd_t where rings that have been _remove'd but
might still be inside a _poll */
APR_RING_HEAD(pfd_dead_ring_t, pfd_elem_t) dead_ring;
+ /* number of threads in poll */
+ volatile apr_uint32_t waiting;
};
static apr_status_t backend_cleanup(void *p_)
@@ -110,6 +113,7 @@
return APR_ENOTIMPL;
}
#endif
+ (*pollset)->waiting = 0;
(*pollset)->nelts = 0;
(*pollset)->nalloc = size;
(*pollset)->flags = flags;
@@ -168,16 +172,22 @@
fd = descriptor->desc.f->filedes;
}
- res = port_associate(pollset->port_fd, PORT_SOURCE_FD, fd,
- get_event(descriptor->reqevents), (void *)elem);
+ if (apr_atomic_read32(&pollset->waiting)) {
+ res = port_associate(pollset->port_fd, PORT_SOURCE_FD, fd,
+ get_event(descriptor->reqevents), (void *)elem);
- if (res < 0) {
- rv = APR_ENOMEM;
- APR_RING_INSERT_TAIL(&(pollset->free_ring), elem, pfd_elem_t, link);
- }
+ if (res < 0) {
+ rv = APR_ENOMEM;
+ APR_RING_INSERT_TAIL(&(pollset->free_ring), elem, pfd_elem_t, link);
+ }
+ else {
+ pollset->nelts++;
+ APR_RING_INSERT_TAIL(&(pollset->query_ring), elem, pfd_elem_t, link);
+ }
+ }
else {
pollset->nelts++;
- APR_RING_INSERT_TAIL(&(pollset->query_ring), elem, pfd_elem_t, link);
+ APR_RING_INSERT_TAIL(&(pollset->add_ring), elem, pfd_elem_t, link);
}
pollset_unlock_rings();
@@ -273,6 +283,8 @@
pollset_lock_rings();
+ apr_atomic_inc32(&pollset->waiting);
+
while (!APR_RING_EMPTY(&(pollset->add_ring), pfd_elem_t, link)) {
ep = APR_RING_FIRST(&(pollset->add_ring));
APR_RING_REMOVE(ep, link);
@@ -296,6 +308,9 @@
ret = port_getn(pollset->port_fd, pollset->port_set, pollset->nalloc,
&nget, tvptr);
+ /* decrease the waiting ASAP to reduce the window for calling
+ port_associate within apr_pollset_add() */
+ apr_atomic_dec32(&pollset->waiting);
(*num) = nget;
if (ret == -1) {
