Re: Patch for apr_poll_drain_wakeup_pipe() getting stuck

[Yann Ylavic]

Hi,

On Wed, Nov 10, 2021 at 2:40 PM Yann Ylavic  wrote:
>
> On Tue, Nov 9, 2021 at 11:52 PM Mihaly Szjatinya
>  wrote:
> >
> > We've encountered  a bug in pollset wakeup pipe. With
> > how wakeup pipe is created now, it's possible that reading loop in
> > `apr_poll_drain_wakeup_pipe()` gets stuck. That could happen, if there is 
> > only a
> > multiple of 512 bytes in the pipe.
> > The problem is that reading loop in `apr_poll_drain_wakeup_pipe()` is 
> > written
> > with the assumption that `apr_file_read()` would never block. On Windows, 
> > where
> > pipes are created differently, this assumption holds. On *nix this can be 
> > fixed
> > with making the read end of the pipe non-blocking.
> > Patch attached.
>
> Thanks, good catch and fix!
> Applied in http://svn.apache.org/r1894914.

Finally this commit was reverted because we think that the same issue
exists on Windows and it can't be addressed by simply making the
read-end of the pipe nonblocking.

So we went with another approach (https://svn.apache.org/r1895106)
that works for all the platforms and hopefully you would validate it
in your environment.


Regards;
Yann.

Re: Patch for apr_poll_drain_wakeup_pipe() getting stuck

[Yann Ylavic]

Hi,

On Tue, Nov 9, 2021 at 11:52 PM Mihaly Szjatinya
 wrote:
>
> We've encountered  a bug in pollset wakeup pipe. With
> how wakeup pipe is created now, it's possible that reading loop in
> `apr_poll_drain_wakeup_pipe()` gets stuck. That could happen, if there is 
> only a
> multiple of 512 bytes in the pipe.
> The problem is that reading loop in `apr_poll_drain_wakeup_pipe()` is written
> with the assumption that `apr_file_read()` would never block. On Windows, 
> where
> pipes are created differently, this assumption holds. On *nix this can be 
> fixed
> with making the read end of the pipe non-blocking.
> Patch attached.

Thanks, good catch and fix!
Applied in http://svn.apache.org/r1894914.

Regards;
Yann.

Patch for apr_poll_drain_wakeup_pipe() getting stuck

[Mihaly Szjatinya]

Hello,
We've encountered  a bug in pollset wakeup pipe. With
how wakeup pipe is created now, it's possible that reading loop in
`apr_poll_drain_wakeup_pipe()` gets stuck. That could happen, if there is only a
multiple of 512 bytes in the pipe.
​
The problem is that reading loop in `apr_poll_drain_wakeup_pipe()` is written
with the assumption that `apr_file_read()` would never block. On Windows, where
pipes are created differently, this assumption holds. On *nix this can be fixed
with making the read end of the pipe non-blocking.
​
Patch attached.
​
Regards,
Mihaly Szjatinya
Index: poll/unix/wakeup.c
===
--- a/poll/unix/wakeup.c	(revision 1894572)
+++ b/poll/unix/wakeup.c	(working copy)
@@ -80,8 +80,9 @@
 {
 apr_status_t rv;
 
-if ((rv = apr_file_pipe_create(_pipe[0], _pipe[1],
-   pool)) != APR_SUCCESS)
+if ((rv = apr_file_pipe_create_ex(_pipe[0], _pipe[1],
+  APR_WRITE_BLOCK,
+  pool)) != APR_SUCCESS)
 return rv;
 
 pfd->p = pool;