On Wed, Jan 28, 2015 at 06:16:16PM +0200, Giulio Camuffo wrote: > Forgot the > > Reviewed-by: Giulio Camuffo <[email protected]>
Thanks, applied. 7575e2e..5ec8062 master -> master I think this is one of those things that'll be easier to prove right in practice rather than on paper, so let's put this through but keep a close eye on behaviors. > 2015-01-28 17:58 GMT+02:00 Giulio Camuffo <[email protected]>: > > 2015-01-28 17:49 GMT+02:00 Derek Foreman <[email protected]>: > >> On 28/01/15 09:39 AM, Giulio Camuffo wrote: > >>> 2015-01-28 17:25 GMT+02:00 Derek Foreman <[email protected]>: > >>>> To fix a shutdown crash in weston's x11 compositor I want to move the > >>>> weston X window close to an idle handler. > >>>> > >>>> Since idle handlers are processed at the start of an event loop, the > >>>> handler that deals with window close will run at the start of the > >>>> next input_loop dispatch, after which the dispatcher blocks on epoll > >>>> forever (since all input events that will ever occur have been consumed). > >>>> > >>>> Dispatching idle callbacks both at the start and end of event-loop > >>>> processing will prevent this permanent blocking. > >>>> > >>>> Note that just moving the callback dispatch could theoretically > >>>> result in an idle callback being delayed indefinitely while waiting > >>>> for epoll_wait() to complete. > >>>> > >>>> Callbacks are removed from the list when they're run, so the second > >>>> dispatch won't result in any extra calls. > >>>> > >>>> Signed-off-by: Derek Foreman <[email protected]> > >>>> --- > >>>> > >>>> Amusingly, the problem Bill points out is exactly the problem this patch > >>>> is > >>>> trying to correct. > >>>> > >>>> This version of the patch leaves the original dispatch and adds a second. > >>>> > >>>> > >>>> src/event-loop.c | 4 +++- > >>>> 1 file changed, 3 insertions(+), 1 deletion(-) > >>>> > >>>> diff --git a/src/event-loop.c b/src/event-loop.c > >>>> index 1f571ba..d257d78 100644 > >>>> --- a/src/event-loop.c > >>>> +++ b/src/event-loop.c > >>>> @@ -421,10 +421,12 @@ wl_event_loop_dispatch(struct wl_event_loop *loop, > >>>> int timeout) > >>>> > >>>> wl_event_loop_process_destroy_list(loop); > >>>> > >>>> + wl_event_loop_dispatch_idle(loop); > >>>> + > >>> > >>> How does this fix the problem? The idle callbacks are still called > >>> before the epoll_wait(), so shouldn't the problem you describe in the > >>> commit message still happen? > >>> Besides, the idle callbacks are anyway going to be called very soon at > >>> the next loop in wl_display_run() after wl_event_loop_dispatch() > >>> returns. Or is the post_dispatch_check() call below involved somehow? > >> > >> The callback in question is added by the handler > >> source->interface->dispatch() calls in the middle. :) > >> > >> So the second idle dispatch gets it, and the loop isn't entered again. > >> > >> Without the change it won't get called until the loop is entered, and > >> there's no events left to process, so epoll_wait() just sits... > > > > Ah, so the loop is broken and wl_display_run() returns after the > > current iteration. Looks good then. > > > >> > >>> -- > >>> Giulio > >>> > >>>> do { > >>>> n = post_dispatch_check(loop); > >>>> } while (n > 0); > >>>> - > >>>> + > >>>> return 0; > >>>> } > >>>> > >>>> -- > >>>> 2.1.4 > >>>> > >>>> _______________________________________________ > >>>> wayland-devel mailing list > >>>> [email protected] > >>>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel > >> > _______________________________________________ > wayland-devel mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/wayland-devel _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
