svn commit: r750744 - in /apr/apr/trunk: include/arch/unix/apr_arch_poll_private.h poll/unix/port.c

[tr...@apache.org]

Author: trawick
Date: Fri Mar  6 01:38:58 2009
New Revision: 750744

URL: http://svn.apache.org/viewvc?rev=750744&view=rev
Log:
fix a race condition between pollset_poll and pollset_remove

T1: port_getn called from pollset_poll
T2: pollset_remove called for fd X
T2: obtains ring lock
T1: port_getn returns event for fd X, kernel disassociates from port
T1: blocks on ring lock
T2: moves ring element for fd X from query ring to dead ring
T2: releases ring lock
T1: obtains ring lock
T1: moves ring element for fd X to add ring, assuming still on query ring


Modified:
    apr/apr/trunk/include/arch/unix/apr_arch_poll_private.h
    apr/apr/trunk/poll/unix/port.c

Modified: apr/apr/trunk/include/arch/unix/apr_arch_poll_private.h
URL: http://svn.apache.org/viewvc/apr/apr/trunk/include/arch/unix/apr_arch_poll_private.h?rev=750744&r1=750743&r2=750744&view=diff
==============================================================================
--- apr/apr/trunk/include/arch/unix/apr_arch_poll_private.h (original)
+++ apr/apr/trunk/include/arch/unix/apr_arch_poll_private.h Fri Mar  6 01:38:58 2009
@@ -97,6 +97,9 @@
 struct pfd_elem_t {
     APR_RING_ENTRY(pfd_elem_t) link;
     apr_pollfd_t pfd;
+#ifdef HAVE_PORT_CREATE
+   int on_query_ring;
+#endif
 };
 
 #endif

Modified: apr/apr/trunk/poll/unix/port.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/poll/unix/port.c?rev=750744&r1=750743&r2=750744&view=diff
==============================================================================
--- apr/apr/trunk/poll/unix/port.c (original)
+++ apr/apr/trunk/poll/unix/port.c Fri Mar  6 01:38:58 2009
@@ -162,6 +162,7 @@
     else {
         elem = (pfd_elem_t *) apr_palloc(pollset->pool, sizeof(pfd_elem_t));
         APR_RING_ELEM_INIT(elem, link);
+        elem->on_query_ring = 0;
     }
     elem->pfd = *descriptor;
 
@@ -185,6 +186,7 @@
         }
         else {
             pollset->nelts++;
+            elem->on_query_ring = 1;
             APR_RING_INSERT_TAIL(&(pollset->p->query_ring), elem, pfd_elem_t, link);
         }
     } 
@@ -246,6 +248,12 @@
         res = port_dissociate(pollset->p->port_fd, PORT_SOURCE_FD, fd);
 
         if (res < 0) {
+            /* The expected case for this failure is that another
+             * thread's call to port_getn() returned this fd and
+             * disassociated the fd from the event port, and 
+             * impl_pollset_poll() is blocked on the ring lock,
+             * which this thread holds.
+             */
             err = errno;
             rv = APR_NOTFOUND;
         }
@@ -257,6 +265,7 @@
 
             if (descriptor->desc.s == ep->pfd.desc.s) {
                 APR_RING_REMOVE(ep, link);
+                ep->on_query_ring = 0;
                 APR_RING_INSERT_TAIL(&(pollset->p->dead_ring),
                                      ep, pfd_elem_t, link);
                 if (ENOENT == err) {
@@ -319,6 +328,7 @@
             break;
         }
 
+        ep->on_query_ring = 1;
         APR_RING_INSERT_TAIL(&(pollset->p->query_ring), ep, pfd_elem_t, link);
     }
 
@@ -366,11 +376,18 @@
                 pollset->p->result_set[j].rtnevents =
                     get_revent(pollset->p->port_set[i].portev_events);
 
-                APR_RING_REMOVE((pfd_elem_t*)pollset->p->port_set[i].portev_user,
-                                link);
-                APR_RING_INSERT_TAIL(&(pollset->p->add_ring), 
-                                (pfd_elem_t*)pollset->p->port_set[i].portev_user,
-                                pfd_elem_t, link);
+                /* 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++;
             }
         }