[PATCH wayland v2] event-loop: Dispatch idle callbacks twice

[Bryce Harrington]

On Wed, Jan 28, 2015 at 06:16:16PM +0200, Giulio Camuffo wrote:
> Forgot the
> 
> Reviewed-by: Giulio Camuffo <giuliocamuffo at gmail.com>

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 <giuliocamuffo at gmail.com>:
> > 2015-01-28 17:49 GMT+02:00 Derek Foreman <derekf at osg.samsung.com>:
> >> On 28/01/15 09:39 AM, Giulio Camuffo wrote:
> >>> 2015-01-28 17:25 GMT+02:00 Derek Foreman <derekf at osg.samsung.com>:
> >>>> 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 <derekf at osg.samsung.com>
> >>>> ---
> >>>>
> >>>> 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
> >>>> wayland-devel at lists.freedesktop.org
> >>>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> >>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel