On Sun, Apr 5, 2015 at 3:06 PM, Jeff Trawick <[email protected]> wrote:
> On 04/05/2015 09:00 AM, [email protected] wrote:
>>
>> Author: trawick
>> Date: Sun Apr 5 13:00:33 2015
>> New Revision: 1671390
>>
>> URL: http://svn.apache.org/r1671390
>> Log:
>> Merge r1671389 from trunk:
>>
>> poll() implementation of apr_pollset_poll(): Return APR_EINTR as
>> appropriate.
>> (APR_SUCCESS was returned instead in that scenario.)
>
>
> Can anyone look over this and +/- 1 for the 1.5.x branch?
[...]
>> ==============================================================================
>> --- apr/apr/branches/1.6.x/poll/unix/poll.c (original)
>> +++ apr/apr/branches/1.6.x/poll/unix/poll.c Sun Apr 5 13:00:33 2015
>> @@ -261,7 +261,7 @@ static apr_status_t impl_pollset_poll(ap
>> }
>> ret = poll(pollset->p->pollset, pollset->nelts, timeout);
>> #endif
>> - (*num) = ret;
>> + *num = 0;
>> if (ret < 0) {
>> return apr_get_netos_error();
>> }
>> @@ -290,8 +290,7 @@ static apr_status_t impl_pollset_poll(ap
>> }
>> }
>> }
>> - if (((*num) = j) > 0)
>> - rv = APR_SUCCESS;
>> + *num = j;
The description of apr_pollset_poll() in apr_poll.h states that EINTR
is only returned when "there were no signalled descriptors at the time
of the wakeup call" (i.e. j == 0 above), whereas now both EINTR and
*num > 0 can be returned in this case.
Also, it seems that the other pollset mechanisms (epoll, kqueue, port)
still return SUCCESS here.
What is the issue with SUCCESS? Is it that we should return EINTR
-soon or later- whenever a pollset is woken up (for the application to
catch the case)?
If so, maybe instead we could delay the pipe draining until next
call(s), when no user descriptor is simultaneously ready...
Something like the below (based on the previous code):
Index: poll/unix/poll.c
===================================================================
--- poll/unix/poll.c (revision 1670476)
+++ poll/unix/poll.c (working copy)
@@ -279,7 +279,6 @@ static apr_status_t impl_pollset_poll(apr_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;
}
else {
@@ -292,6 +291,8 @@ static apr_status_t impl_pollset_poll(apr_pollset_
}
if (((*num) = j) > 0)
rv = APR_SUCCESS;
+ else if (rv == APR_EINTR)
+ apr_poll_drain_wakeup_pipe(pollset->wakeup_pipe);
}
if (descriptors && (*num))
*descriptors = pollset->p->result_set;
--
Regards,
Yann.
