Hi, On 15 September 2016 at 11:09, Olivier Fourdan <ofour...@redhat.com> wrote: >> > What this patch does is to avoid dispatching to the Wayland file >> > descriptor until it becomes available for writing again, while at the >> > same time continue processing X11 requests to release the deadlock. >> >> Could you expand on why the Wayland fd should not be dispatched while >> you wait for it to flush? >> >> Is it just because dispatching it is likely to cause more Wayland >> requests to be sent? I wonder how that holds up. Maybe it doesn't >> matter as the premise is that the Wayland compositor is stuck. > > Actually, the idea is to minimize the risk of breaking further down in > libwayland itself, either in copy_fds_to_connection() > ("request could not be marshaled: can't send file descriptor") or in > wl_proxy_marshal_array_constructor_versioned() ("Error sending request: > Resource temporarily unavailable")
So, yes: you want to avoid sending more traffic down a socket which is already full. >> If all replies are immediate, it sounds like this would be a fairly >> reliable fix after all. > > To be honest, I wouldn't consider this a fix, merely a workaround, not to say > a ugly hack, but afaics from my testing here, it avoids the lock (even > though, with vlc fullscreen, the deadlock reoccur continuously when the > pointer hovers the timeline as the tooltip, a shaped window, gets > mapped/unmapped continuously which causes mutter to issue a lot of those > blocking rountrips) - Yet it offers a chance to get out of the deadlock > (eventually...) and not lose Xwayland, which for gnome-shell/mutter means > losing the entire user session. Indeed, it is a workaround: X11 requests can still provoke more activity. Consider where you ignore all Wayland events, but gtkperf continues rendering as an X11 client. Flushing that rendering to the upstream compositor (wl_surface_attach/damage/commit/etc) will cause more traffic on the Wayland connection. You might be able to do even better by calling OnlyListenToOneClient(clientInfo.clients[wm_client_index]) when transitioning wait_flush from 0 to 1, and ListenToAllClients() when it transitions back from 1 to 0. That being said, it's a good start, and I definitely believe you when you say it improves things for now. I think in the long run it does need to be extended so we avoid things like flushing more rendering, but with EINTR checked in xwl_display_pollout: Reviewed-by: Daniel Stone <dani...@collabora.com> Cheers, Daniel _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel