From c34b87f6b7c9cb3d9fdfbf8991250356832ff374 Mon Sep 17 00:00:00 2001 From: Oleg Pereverzev Date: Tue, 2 Nov 2021 17:34:54 +0200 Subject: [PATCH] Polling wakeup: recover from possible faults or return error --- include/arch/unix/apr_arch_poll_private.h | 4 ++- poll/unix/epoll.c | 19 +++++++++-- poll/unix/kqueue.c | 19 +++++++++-- poll/unix/poll.c | 21 ++++++++++-- poll/unix/port.c | 19 +++++++++-- poll/unix/select.c | 12 ++++++- poll/unix/wakeup.c | 39 +++++++++++++++++++++-- 7 files changed, 121 insertions(+), 12 deletions(-) diff --git a/include/arch/unix/apr_arch_poll_private.h b/include/arch/unix/apr_arch_poll_private.h index 4c2aaa7041f..a5a0db12020 100644 --- a/include/arch/unix/apr_arch_poll_private.h +++ b/include/arch/unix/apr_arch_poll_private.h @@ -182,6 +182,8 @@ 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); +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 */ diff --git a/poll/unix/epoll.c b/poll/unix/epoll.c index 4ab03f67ccc..466dd9ee7e1 100644 --- a/poll/unix/epoll.c +++ b/poll/unix/epoll.c @@ -257,6 +257,7 @@ static apr_status_t impl_pollset_poll(apr_pollset_t *pollset, { int ret; apr_status_t rv = APR_SUCCESS; + apr_status_t wakeup_rv = APR_SUCCESS; *num = 0; @@ -289,8 +290,15 @@ static apr_status_t impl_pollset_poll(apr_pollset_t *pollset, 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,14 @@ static apr_status_t impl_pollcb_poll(apr_pollcb_t *pollcb, 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; } diff --git a/poll/unix/kqueue.c b/poll/unix/kqueue.c index 548464db148..7995b729ca3 100644 --- a/poll/unix/kqueue.c +++ b/poll/unix/kqueue.c @@ -257,6 +257,7 @@ static apr_status_t impl_pollset_poll(apr_pollset_t *pollset, int ret; struct timespec tv, *tvptr; apr_status_t rv = APR_SUCCESS; + apr_status_t wakeup_rv = APR_SUCCESS; *num = 0; @@ -286,8 +287,15 @@ static apr_status_t impl_pollset_poll(apr_pollset_t *pollset, 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,14 @@ static apr_status_t impl_pollcb_poll(apr_pollcb_t *pollcb, 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; } diff --git a/poll/unix/poll.c b/poll/unix/poll.c index e96a53cfffb..be57899a29b 100644 --- a/poll/unix/poll.c +++ b/poll/unix/poll.c @@ -236,6 +236,7 @@ static apr_status_t impl_pollset_poll(apr_pollset_t *pollset, { int ret; apr_status_t rv = APR_SUCCESS; + apr_status_t wakeup_rv = APR_SUCCESS; *num = 0; @@ -275,8 +276,16 @@ static apr_status_t impl_pollset_poll(apr_pollset_t *pollset, 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]; @@ -422,7 +431,15 @@ static apr_status_t impl_pollcb_poll(apr_pollcb_t *pollcb, 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; } diff --git a/poll/unix/port.c b/poll/unix/port.c index c1e599412ec..aef77e70aee 100644 --- a/poll/unix/port.c +++ b/poll/unix/port.c @@ -359,6 +359,7 @@ static apr_status_t impl_pollset_poll(apr_pollset_t *pollset, 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,15 @@ static apr_status_t impl_pollset_poll(apr_pollset_t *pollset, 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,14 @@ static apr_status_t impl_pollcb_poll(apr_pollcb_t *pollcb, 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; } diff --git a/poll/unix/select.c b/poll/unix/select.c index 51be3c1cd5f..bc339d9f259 100644 --- a/poll/unix/select.c +++ b/poll/unix/select.c @@ -346,6 +346,7 @@ static apr_status_t impl_pollset_poll(apr_pollset_t *pollset, 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 @@ static apr_status_t impl_pollset_poll(apr_pollset_t *pollset, 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 { diff --git a/poll/unix/wakeup.c b/poll/unix/wakeup.c index f8ea0604842..d5ab0b34794 100644 --- a/poll/unix/wakeup.c +++ b/poll/unix/wakeup.c @@ -134,12 +134,13 @@ apr_status_t apr_poll_close_wakeup_pipe(apr_file_t **wakeup_pipe) /* 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 @@ void apr_poll_drain_wakeup_pipe(apr_file_t **wakeup_pipe) 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; }