[PATCH 6/6] xwayland: don't race with weston

[Kristian Hoegsberg]

On Mon, Apr 16, 2012 at 05:31:51PM +0300, Tiago Vignatti wrote:
> Signed-off-by: Tiago Vignatti <tiago.vignatti at intel.com>

Ok, I now see the race you're talking about.  We can't do a roundtrip
here though, we can hit a mutual deadlock between weston and X, where
weston waits for a reply to get xcb_get_property(), while X is waiting
in wl_display_roundtrip().

The problem is that one one hand we need to receive set_window_id in
weston before we can create the shell_surface, on the other hand, we
can't set the surface type until we've read the X window properties,
which we do in MapNotify.  Both set_window_id and MapNotify are sent
when the X server calls RealizeWindow and they arrive in weston during
the same wakeup and get handled in undefined order.  The roundtrip in
your patch enforces a certain order, but causes deadlocks.

Now, set_window_id and MapNotify are essentially the same message
("the window is now mapped") but in different protocol streams.  We
can just do everything we need to do (read out the window properties)
in the set_window_id handler instead and the race is gone.  Further,
we need to call set_toplevel before the initial attach.  xwayland
sends the set_surface_id request right after creating the surface and
always before the initial attach, so this is really the right place do
all this.

Kristian

> ---
>  hw/xfree86/xwayland/xwayland-window.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/xfree86/xwayland/xwayland-window.c b/hw/xfree86/xwayland/xwayland-window.c
> index 6d1d7bf..f26e0a0 100644
> --- a/hw/xfree86/xwayland/xwayland-window.c
> +++ b/hw/xfree86/xwayland/xwayland-window.c
> @@ -191,6 +191,11 @@ xwl_realize_window(WindowPtr window)
>  	xserver_set_window_id(xwl_screen->xorg_server,
>  			      xwl_window->surface, window->drawable.id);
>  
> +    /* we need to wait in order to avoid a race with Weston WM, when it would
> +     * try to anticipate XCB_MAP_NOTIFY, requiring create_surface completed
> +     * (for shell_surface_set_toplevel) */
> +    wl_display_roundtrip(xwl_screen->display);
> +
>      wl_surface_set_user_data(xwl_window->surface, xwl_window);
>      xwl_window_attach(xwl_window, (*screen->GetWindowPixmap)(window));
>  
> -- 
> 1.7.5.4
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel