You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by mt...@apache.org on 2021/11/17 13:45:31 UTC

svn commit: r1895106 - in /apr/apr/trunk: include/arch/unix/apr_arch_poll_private.h poll/unix/epoll.c poll/unix/kqueue.c poll/unix/poll.c poll/unix/pollcb.c poll/unix/pollset.c poll/unix/port.c poll/unix/select.c poll/unix/wakeup.c

Author: mturk
Date: Wed Nov 17 13:45:31 2021
New Revision: 1895106

URL: http://svn.apache.org/viewvc?rev=1895106&view=rev
Log:
Fix drain wakeup pipe issue when multiple threads call apr_pollset_wakeup/apr_pollcb_wakeup for the same pollset filling up drain pipe. Use atomics so that wakeup call is noop if some other thread allready done this

Modified:
    apr/apr/trunk/include/arch/unix/apr_arch_poll_private.h
    apr/apr/trunk/poll/unix/epoll.c
    apr/apr/trunk/poll/unix/kqueue.c
    apr/apr/trunk/poll/unix/poll.c
    apr/apr/trunk/poll/unix/pollcb.c
    apr/apr/trunk/poll/unix/pollset.c
    apr/apr/trunk/poll/unix/port.c
    apr/apr/trunk/poll/unix/select.c
    apr/apr/trunk/poll/unix/wakeup.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=1895106&r1=1895105&r2=1895106&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 Wed Nov 17 13:45:31 2021
@@ -123,6 +123,7 @@ struct apr_pollset_t
     /* Pipe descriptors used for wakeup */
     apr_file_t *wakeup_pipe[2];
     apr_pollfd_t wakeup_pfd;
+    volatile apr_uint32_t wakeup_set;
     apr_pollset_private_t *p;
     const apr_pollset_provider_t *provider;
 };
@@ -151,6 +152,7 @@ struct apr_pollcb_t {
     /* Pipe descriptors used for wakeup */
     apr_file_t *wakeup_pipe[2];
     apr_pollfd_t wakeup_pfd;
+    volatile apr_uint32_t wakeup_set;
     int fd;
     apr_pollcb_pset pollset;
     apr_pollfd_t **copyset;
@@ -182,6 +184,6 @@ struct apr_pollcb_provider_t {
 apr_status_t apr_poll_create_wakeup_pipe(apr_pool_t *pool, apr_pollfd_t *pfd, 
                                          apr_file_t **wakeup_pipe);
 apr_status_t apr_poll_close_wakeup_pipe(apr_file_t **wakeup_pipe);
-void apr_poll_drain_wakeup_pipe(apr_file_t **wakeup_pipe);
+void apr_poll_drain_wakeup_pipe(volatile apr_uint32_t *wakeup_set, apr_file_t **wakeup_pipe);
 
 #endif /* APR_ARCH_POLL_PRIVATE_H */

Modified: apr/apr/trunk/poll/unix/epoll.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/poll/unix/epoll.c?rev=1895106&r1=1895105&r2=1895106&view=diff
==============================================================================
--- apr/apr/trunk/poll/unix/epoll.c (original)
+++ apr/apr/trunk/poll/unix/epoll.c Wed Nov 17 13:45:31 2021
@@ -289,7 +289,7 @@ static apr_status_t impl_pollset_poll(ap
             if ((pollset->flags & APR_POLLSET_WAKEABLE) &&
                 fdptr->desc_type == APR_POLL_FILE &&
                 fdptr->desc.f == pollset->wakeup_pipe[0]) {
-                apr_poll_drain_wakeup_pipe(pollset->wakeup_pipe);
+                apr_poll_drain_wakeup_pipe(&pollset->wakeup_set, pollset->wakeup_pipe);
                 rv = APR_EINTR;
             }
             else {
@@ -460,7 +460,7 @@ static apr_status_t impl_pollcb_poll(apr
             if ((pollcb->flags & APR_POLLSET_WAKEABLE) &&
                 pollfd->desc_type == APR_POLL_FILE &&
                 pollfd->desc.f == pollcb->wakeup_pipe[0]) {
-                apr_poll_drain_wakeup_pipe(pollcb->wakeup_pipe);
+                apr_poll_drain_wakeup_pipe(&pollcb->wakeup_set, pollcb->wakeup_pipe);
                 return APR_EINTR;
             }
 

Modified: apr/apr/trunk/poll/unix/kqueue.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/poll/unix/kqueue.c?rev=1895106&r1=1895105&r2=1895106&view=diff
==============================================================================
--- apr/apr/trunk/poll/unix/kqueue.c (original)
+++ apr/apr/trunk/poll/unix/kqueue.c Wed Nov 17 13:45:31 2021
@@ -286,7 +286,7 @@ static apr_status_t impl_pollset_poll(ap
             if ((pollset->flags & APR_POLLSET_WAKEABLE) &&
                 fd->desc_type == APR_POLL_FILE &&
                 fd->desc.f == pollset->wakeup_pipe[0]) {
-                apr_poll_drain_wakeup_pipe(pollset->wakeup_pipe);
+                apr_poll_drain_wakeup_pipe(&pollset->wakeup_set, pollset->wakeup_pipe);
                 rv = APR_EINTR;
             }
             else {
@@ -473,7 +473,7 @@ static apr_status_t impl_pollcb_poll(apr
             if ((pollcb->flags & APR_POLLSET_WAKEABLE) &&
                 pollfd->desc_type == APR_POLL_FILE &&
                 pollfd->desc.f == pollcb->wakeup_pipe[0]) {
-                apr_poll_drain_wakeup_pipe(pollcb->wakeup_pipe);
+                apr_poll_drain_wakeup_pipe(&pollcb->wakeup_set, pollcb->wakeup_pipe);
                 return APR_EINTR;
             }
 

Modified: apr/apr/trunk/poll/unix/poll.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/poll/unix/poll.c?rev=1895106&r1=1895105&r2=1895106&view=diff
==============================================================================
--- apr/apr/trunk/poll/unix/poll.c (original)
+++ apr/apr/trunk/poll/unix/poll.c Wed Nov 17 13:45:31 2021
@@ -275,7 +275,7 @@ static apr_status_t impl_pollset_poll(ap
                 if ((pollset->flags & APR_POLLSET_WAKEABLE) &&
                     pollset->p->query_set[i].desc_type == APR_POLL_FILE &&
                     pollset->p->query_set[i].desc.f == pollset->wakeup_pipe[0]) {
-                    apr_poll_drain_wakeup_pipe(pollset->wakeup_pipe);
+                    apr_poll_drain_wakeup_pipe(&pollset->wakeup_set, pollset->wakeup_pipe);
                     rv = APR_EINTR;
                 }
                 else {
@@ -422,7 +422,7 @@ static apr_status_t impl_pollcb_poll(apr
                 if ((pollcb->flags & APR_POLLSET_WAKEABLE) &&
                     pollfd->desc_type == APR_POLL_FILE &&
                     pollfd->desc.f == pollcb->wakeup_pipe[0]) {
-                    apr_poll_drain_wakeup_pipe(pollcb->wakeup_pipe);
+                    apr_poll_drain_wakeup_pipe(&pollcb->wakeup_set, pollcb->wakeup_pipe);
                     return APR_EINTR;
                 }
 

Modified: apr/apr/trunk/poll/unix/pollcb.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/poll/unix/pollcb.c?rev=1895106&r1=1895105&r2=1895106&view=diff
==============================================================================
--- apr/apr/trunk/poll/unix/pollcb.c (original)
+++ apr/apr/trunk/poll/unix/pollcb.c Wed Nov 17 13:45:31 2021
@@ -23,6 +23,7 @@
 #include "apr_poll.h"
 #include "apr_time.h"
 #include "apr_portable.h"
+#include "apr_atomic.h"
 #include "apr_arch_file_io.h"
 #include "apr_arch_networkio.h"
 #include "apr_arch_poll_private.h"
@@ -134,6 +135,7 @@ APR_DECLARE(apr_status_t) apr_pollcb_cre
     pollcb->flags = flags;
     pollcb->pool = p;
     pollcb->provider = provider;
+    pollcb->wakeup_set = 0;
 
     rv = (*provider->create)(pollcb, size, p, flags);
     if (rv == APR_ENOTIMPL) {
@@ -212,8 +214,12 @@ APR_DECLARE(apr_status_t) apr_pollcb_pol
 
 APR_DECLARE(apr_status_t) apr_pollcb_wakeup(apr_pollcb_t *pollcb)
 {
-    if (pollcb->flags & APR_POLLSET_WAKEABLE)
-        return apr_file_putc(1, pollcb->wakeup_pipe[1]);
+    if (pollcb->flags & APR_POLLSET_WAKEABLE) {
+        if (apr_atomic_cas32(&pollcb->wakeup_set, 1, 0) == 0)
+            return apr_file_putc(1, pollcb->wakeup_pipe[1]);
+        else
+           return APR_SUCCESS;
+    }
     else
         return APR_EINIT;
 }

Modified: apr/apr/trunk/poll/unix/pollset.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/poll/unix/pollset.c?rev=1895106&r1=1895105&r2=1895106&view=diff
==============================================================================
--- apr/apr/trunk/poll/unix/pollset.c (original)
+++ apr/apr/trunk/poll/unix/pollset.c Wed Nov 17 13:45:31 2021
@@ -23,6 +23,7 @@
 #include "apr_poll.h"
 #include "apr_time.h"
 #include "apr_portable.h"
+#include "apr_atomic.h"
 #include "apr_arch_file_io.h"
 #include "apr_arch_networkio.h"
 #include "apr_arch_poll_private.h"
@@ -140,6 +141,7 @@ APR_DECLARE(apr_status_t) apr_pollset_cr
     pollset->pool = p;
     pollset->flags = flags;
     pollset->provider = provider;
+    pollset->wakeup_set = 0;
 
     rv = (*provider->create)(pollset, size, p, flags);
     if (rv == APR_ENOTIMPL) {
@@ -216,8 +218,12 @@ APR_DECLARE(apr_status_t) apr_pollset_de
 
 APR_DECLARE(apr_status_t) apr_pollset_wakeup(apr_pollset_t *pollset)
 {
-    if (pollset->flags & APR_POLLSET_WAKEABLE)
-        return apr_file_putc(1, pollset->wakeup_pipe[1]);
+    if (pollset->flags & APR_POLLSET_WAKEABLE) {
+        if (apr_atomic_cas32(&pollset->wakeup_set, 1, 0) == 0)
+            return apr_file_putc(1, pollset->wakeup_pipe[1]);
+        else
+           return APR_SUCCESS;
+    }
     else
         return APR_EINIT;
 }

Modified: apr/apr/trunk/poll/unix/port.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/poll/unix/port.c?rev=1895106&r1=1895105&r2=1895106&view=diff
==============================================================================
--- apr/apr/trunk/poll/unix/port.c (original)
+++ apr/apr/trunk/poll/unix/port.c Wed Nov 17 13:45:31 2021
@@ -411,7 +411,7 @@ static apr_status_t impl_pollset_poll(ap
         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);
+            apr_poll_drain_wakeup_pipe(&pollset->wakeup_set, pollset->wakeup_pipe);
             rv = APR_EINTR;
         }
         else {
@@ -563,7 +563,7 @@ static apr_status_t impl_pollcb_poll(apr
             if ((pollcb->flags & APR_POLLSET_WAKEABLE) &&
                 pollfd->desc_type == APR_POLL_FILE &&
                 pollfd->desc.f == pollcb->wakeup_pipe[0]) {
-                apr_poll_drain_wakeup_pipe(pollcb->wakeup_pipe);
+                apr_poll_drain_wakeup_pipe(&pollcb->wakeup_set, pollcb->wakeup_pipe);
                 return APR_EINTR;
             }
 

Modified: apr/apr/trunk/poll/unix/select.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/poll/unix/select.c?rev=1895106&r1=1895105&r2=1895106&view=diff
==============================================================================
--- apr/apr/trunk/poll/unix/select.c (original)
+++ apr/apr/trunk/poll/unix/select.c Wed Nov 17 13:45:31 2021
@@ -401,7 +401,7 @@ static apr_status_t impl_pollset_poll(ap
         else {
             if ((pollset->flags & APR_POLLSET_WAKEABLE) &&
                 pollset->p->query_set[i].desc.f == pollset->wakeup_pipe[0]) {
-                apr_poll_drain_wakeup_pipe(pollset->wakeup_pipe);
+                apr_poll_drain_wakeup_pipe(&pollset->wakeup_set, pollset->wakeup_pipe);
                 rv = APR_EINTR;
                 continue;
             }

Modified: apr/apr/trunk/poll/unix/wakeup.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/poll/unix/wakeup.c?rev=1895106&r1=1895105&r2=1895106&view=diff
==============================================================================
--- apr/apr/trunk/poll/unix/wakeup.c (original)
+++ apr/apr/trunk/poll/unix/wakeup.c Wed Nov 17 13:45:31 2021
@@ -18,6 +18,7 @@
 #include "apr_poll.h"
 #include "apr_time.h"
 #include "apr_portable.h"
+#include "apr_atomic.h"
 #include "apr_arch_file_io.h"
 #include "apr_arch_networkio.h"
 #include "apr_arch_poll_private.h"
@@ -36,9 +37,6 @@ apr_status_t apr_poll_create_wakeup_pipe
                                       pool)) != APR_SUCCESS)
         return rv;
 
-    /* Read end of the pipe is non-blocking */
-    apr_file_pipe_timeout_set(wakeup_pipe[0], 0);
-
     pfd->reqevents = APR_POLLIN;
     pfd->desc_type = APR_POLL_FILE;
     pfd->desc.f = wakeup_pipe[0];
@@ -139,18 +137,18 @@ apr_status_t apr_poll_close_wakeup_pipe(
 
 /* Read and discard whatever is in the wakeup pipe.
  */
-void apr_poll_drain_wakeup_pipe(apr_file_t **wakeup_pipe)
+void apr_poll_drain_wakeup_pipe(volatile apr_uint32_t *wakeup_set, apr_file_t **wakeup_pipe)
 {
-    char rb[512];
-    apr_size_t nr = sizeof(rb);
 
-    while (apr_file_read(wakeup_pipe[0], rb, &nr) == APR_SUCCESS) {
-        /* Although we write just one byte to the other end of the pipe
+    while (apr_atomic_cas32(wakeup_set, 0, 1) > 0) {
+        char ch;
+        /* though we write just one byte to the other end of the pipe
          * during wakeup, multiple threads could call the wakeup.
          * So simply drain out from the input side of the pipe all
          * the data.
          */
-        if (nr != sizeof(rb))
+        if (apr_file_getc(&ch, wakeup_pipe[0]) != APR_SUCCESS)
             break;
     }
 }
+