Good time of day.
We've noticed that there is a bug when wakeup pipe in polling becomes broken 
that leads to an instant taint of polling process
It is possible because in Windows pipe is done via real network sockets that 
could be closed by network change or in some other cases
In general, no matter the OS if wakeup pipe gets broken 
apr_poll_drain_wakeup_pipe's status is disregarded and no action is taken
Patch attached.

Regards,
Oleg Pereverzev
Author: Oleg Pereverzev
Company: NXLog
Date: 15 Oct 2021

Index: include/arch/unix/apr_arch_poll_private.h
===================================================================
--- a/include/arch/unix/apr_arch_poll_private.h	2017-09-19 00:31:30.000000000 -0700
+++ b/include/arch/unix/apr_arch_poll_private.h	2021-10-15 08:09:13.709390800 -0700
@@ -184,6 +184,8 @@
 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);
+apr_status_t apr_poll_drain_wakeup_pipe(apr_file_t **wakeup_pipe);
+apr_status_t apr_pollset_wakeup_pipe_regenerate(apr_pollset_t *pollset);
+apr_status_t apr_pollcb_wakeup_pipe_regenerate(apr_pollcb_t *pollcb);

 #endif /* APR_ARCH_POLL_PRIVATE_H */

Index: poll/unix/epoll.c
===================================================================
--- a/poll/unix/epoll.c	2018-01-03 01:49:38.000000000 -0800
+++ b/poll/unix/epoll.c	2021-10-15 08:09:13.709390800 -0700
@@ -257,6 +257,7 @@
 {
     int ret;
     apr_status_t rv = APR_SUCCESS;
+    apr_status_t wakeup_rv = APR_SUCCESS;

     *num = 0;

@@ -289,8 +290,13 @@
             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);
                 rv = APR_EINTR;
+                if (apr_poll_drain_wakeup_pipe(pollset->wakeup_pipe) != APR_SUCCESS) {
+                    wakeup_rv = apr_pollset_wakeup_pipe_regenerate(pollset);
+                    if (wakeup_rv != APR_SUCCESS) {
+                        rv = wakeup_rv;
+                    }
+                }
             }
             else {
                 pollset->p->result_set[j] = *fdptr;
@@ -460,7 +468,12 @@
             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);
+                if (apr_poll_drain_wakeup_pipe(pollcb->wakeup_pipe) != APR_SUCCESS) {
+                    rv = apr_pollcb_wakeup_pipe_regenerate(pollcb);
+                    if (rv != APR_SUCCESS) {
+                        return rv;
+                    }
+                }
                 return APR_EINTR;
             }

Index: poll/unix/kqueue.c
===================================================================
--- a/poll/unix/kqueue.c	2018-01-03 01:49:38.000000000 -0800
+++ b/poll/unix/kqueue.c	2021-10-15 08:09:13.709390800 -0700
@@ -257,6 +257,7 @@
     int ret;
     struct timespec tv, *tvptr;
     apr_status_t rv = APR_SUCCESS;
+    apr_status_t wakeup_rv = APR_SUCCESS;

     *num = 0;

@@ -286,8 +287,13 @@
             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);
                 rv = APR_EINTR;
+                if (apr_poll_drain_wakeup_pipe(pollset->wakeup_pipe) != APR_SUCCESS) {
+                    wakeup_rv = apr_pollset_wakeup_pipe_regenerate(pollset);
+                    if (wakeup_rv != APR_SUCCESS) {
+                        rv = wakeup_rv;
+                    }
+                }
             }
             else {
                 pollset->p->result_set[j] = *fd;
@@ -473,7 +481,12 @@
             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);
+                if (apr_poll_drain_wakeup_pipe(pollcb->wakeup_pipe) != APR_SUCCESS) {
+                    rv = apr_pollcb_wakeup_pipe_regenerate(pollcb);
+                    if (rv != APR_SUCCESS) {
+                        return rv;
+                    }
+                }
                 return APR_EINTR;
             }

Index: poll/unix/poll.c
===================================================================
--- a/poll/unix/poll.c	2017-09-19 00:31:30.000000000 -0700
+++ b/poll/unix/poll.c	2021-10-15 08:09:13.709390800 -0700
@@ -240,6 +240,7 @@
 {
     int ret;
     apr_status_t rv = APR_SUCCESS;
+    apr_status_t wakeup_rv = APR_SUCCESS;

     *num = 0;

@@ -279,8 +280,14 @@
                 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);
                     rv = APR_EINTR;
+
+                    if (apr_poll_drain_wakeup_pipe(pollset->wakeup_pipe) != APR_SUCCESS) {
+                        wakeup_rv = apr_pollset_wakeup_pipe_regenerate(pollset);
+                        if (wakeup_rv != APR_SUCCESS) {
+                            rv = wakeup_rv;
+                        }
+                    }
                 }
                 else {
                     pollset->p->result_set[j] = pollset->p->query_set[i];
@@ -431,7 +440,13 @@
                 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);
+
+                    if (apr_poll_drain_wakeup_pipe(pollcb->wakeup_pipe) != APR_SUCCESS) {
+                        rv = apr_pollcb_wakeup_pipe_regenerate(pollcb);
+                        if (rv != APR_SUCCESS) {
+                            return rv;
+                        }
+                    }
                     return APR_EINTR;
                 }

Index: poll/unix/port.c
===================================================================
--- a/poll/unix/port.c	2018-01-03 01:49:38.000000000 -0800
+++ b/poll/unix/port.c	2021-10-15 08:09:13.709390800 -0700
@@ -359,6 +359,7 @@
     apr_int32_t j;
     pfd_elem_t *ep;
     apr_status_t rv = APR_SUCCESS;
+    apr_status_t wakeup_rv = APR_SUCCESS;

     *num = 0;
     nget = 1;
@@ -411,8 +412,13 @@
         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;
+            if (apr_poll_drain_wakeup_pipe(pollset->wakeup_pipe) != APR_SUCCESS) {
+                wakeup_rv = apr_pollset_wakeup_pipe_regenerate(pollset);
+                if (wakeup_rv != APR_SUCCESS) {
+                    rv = wakeup_rv;
+                }
+            }
         }
         else {
             pollset->p->result_set[j] = ep->pfd;
@@ -563,7 +571,12 @@
             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);
+                if (apr_poll_drain_wakeup_pipe(pollcb->wakeup_pipe) != APR_SUCCESS) {
+                    rv = apr_pollcb_wakeup_pipe_regenerate(pollcb);
+                    if (rv != APR_SUCCESS) {
+                        return rv;
+                    }
+                }
                 return APR_EINTR;
             }

Index: poll/unix/select.c
===================================================================
--- a/poll/unix/select.c	2017-09-19 00:31:30.000000000 -0700
+++ b/poll/unix/select.c	2021-10-15 08:09:13.709390800 -0700
@@ -346,6 +346,7 @@
     struct timeval tv, *tvptr;
     fd_set readset, writeset, exceptset;
     apr_status_t rv = APR_SUCCESS;
+    apr_status_t wakeup_rv = APR_SUCCESS;

     *num = 0;

@@ -401,8 +402,17 @@
         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);
                 rv = APR_EINTR;
+
+                if ( apr_poll_drain_wakeup_pipe(pollset->wakeup_pipe) != APR_SUCCESS )
+                {
+                    wakeup_rv = apr_pollset_wakeup_pipe_regenerate(pollset);
+                    if ( wakeup_rv != APR_SUCCESS )
+                    {
+                        rv = wakeup_rv;
+                    }
+                }
+
                 continue;
             }
             else {

Index: poll/unix/wakeup.c
===================================================================
--- a/poll/unix/wakeup.c	2016-12-03 12:49:29.000000000 -0800
+++ b/poll/unix/wakeup.c	2021-10-15 08:09:13.725015800 -0700
@@ -134,12 +134,13 @@

 /* Read and discard whatever is in the wakeup pipe.
  */
-void apr_poll_drain_wakeup_pipe(apr_file_t **wakeup_pipe)
+apr_status_t apr_poll_drain_wakeup_pipe(apr_file_t **wakeup_pipe)
 {
     char rb[512];
     apr_size_t nr = sizeof(rb);
+    apr_status_t rv;

-    while (apr_file_read(wakeup_pipe[0], rb, &nr) == APR_SUCCESS) {
+    while ((rv = apr_file_read(wakeup_pipe[0], rb, &nr)) == APR_SUCCESS) {
         /* Although 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
@@ -148,4 +149,38 @@
         if (nr != sizeof(rb))
             break;
     }
+
+    return rv;
+}
+
+apr_status_t apr_pollset_wakeup_pipe_regenerate(apr_pollset_t *pollset)
+{
+    apr_status_t rv;
+
+    apr_pollset_remove(pollset, &pollset->wakeup_pfd);
+    apr_poll_close_wakeup_pipe(pollset->wakeup_pipe);
+    if (!((rv = apr_poll_create_wakeup_pipe(pollset->pool,
+            &pollset->wakeup_pfd, pollset->wakeup_pipe)) == APR_SUCCESS &&
+          (rv = apr_pollset_add(pollset,
+            &pollset->wakeup_pfd)) == APR_SUCCESS)) {
+        return rv;
+    }
+
+    return APR_SUCCESS;
+}
+
+apr_status_t apr_pollcb_wakeup_pipe_regenerate(apr_pollcb_t *pollcb)
+{
+    apr_status_t rv;
+
+    apr_pollcb_remove(pollcb, &pollcb->wakeup_pfd);
+    apr_poll_close_wakeup_pipe(pollcb->wakeup_pipe);
+    if (!((rv = apr_poll_create_wakeup_pipe(pollcb->pool,
+            &pollcb->wakeup_pfd, pollcb->wakeup_pipe)) == APR_SUCCESS &&
+          (rv = apr_pollcb_add(pollcb,
+            &pollcb->wakeup_pfd)) == APR_SUCCESS)) {
+        return rv;
+    }
+
+    return APR_SUCCESS;
 }

Reply via email to