On Fri, Aug 5, 2016 at 3:19 PM, Yann Ylavic <ylavic....@gmail.com> wrote:
> Hi Luca,
>
> On Fri, Aug 5, 2016 at 2:58 PM, Luca Toscano <toscano.l...@gmail.com> wrote:
>>
>> 2016-08-04 17:56 GMT+02:00 Luca Toscano <toscano.l...@gmail.com>:
>>>
>>> Would it be possible to avoid them adding APR_POLLSET_WAKEABLE to the
>>> event_pollset flags and calling apr_pollset_wakeup right after
>>> apr_skiplist_insert?
>
> That might be possible, but there is more than apr_skiplist_insert()
> to handle I'm afraid, see below.
>
>>
>> Very high level idea: http://apaste.info/MKE
>
> The timeout_interval also handles the listener_may_exit (and
> connection_count == 0 in graceful mode), your patch seems not handle
> them...
>
> The listener_may_exit case may be easily handled in wakeup_listener
> (one more apr_pollset_wakeup() there), however the connection_count
> looks more complicated.
>
> IOW, not sure something like the below would be enough...
>
> Index: server/mpm/event/event.c
> ===================================================================
> --- server/mpm/event/event.c (revision 1755288)
> +++ server/mpm/event/event.c (working copy)
> @@ -484,6 +484,7 @@ static void close_worker_sockets(void)
> static void wakeup_listener(void)
> {
> listener_may_exit = 1;
> + apr_pollset_wakeup(event_pollset);
> if (!listener_os_thread) {
> /* XXX there is an obscure path that this doesn't handle perfectly:
> * right after listener thread is created but before
Or rather, in wakeup_listener():
@@ -493,6 +493,9 @@ static void wakeup_listener(void)
return;
}
+ /* unblock the listener if it's waiting for a timer */
+ apr_pollset_wakeup(event_pollset);
+
/* unblock the listener if it's waiting for a worker */
ap_queue_info_term(worker_queue_info);
_
> @@ -696,7 +697,9 @@ static apr_status_t decrement_connection_count(voi
> default:
> break;
> }
> - apr_atomic_dec32(&connection_count);
> + if (!apr_atomic_dec32(&connection_count) && listener_may_exit) {
> + apr_pollset_wakeup(event_pollset);
> + }
> return APR_SUCCESS;
> }
>
> _
>
> Regards,
> Yann.
