From e37f18ee97aa4856108ef701628e013d6bf4903e Mon Sep 17 00:00:00 2001 From: Olivier Fourdan Date: Fri, 23 Sep 2022 09:23:02 +0200 Subject: [PATCH] xwayland: Delay wl_surface destruction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X11 and Wayland requests are unordered, causing a race in the X11 window and wl_surface association. To mitigate that race, delay the wl_surface destruction by 1 second, so that the compositor has time to establish the association before the wl_surface is destroyed: to see both the wl_surface created and the WL_SURFACE_ID X11 property set. This is only a mitigation though, a more robust solution requires a future dedicated Wayland protocol. v2: Clean up pending wl_surface destroy on exit as well. Closes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1157 Signed-off-by: Olivier Fourdan Suggested-by: Pekka Paalanen Reviewed-by: Michel Dänzer Tested-by: Joshua Ashton Tested-by: Sterophonick See-also: https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/163 --- hw/xwayland/xwayland-screen.c | 6 ++++ hw/xwayland/xwayland-screen.h | 1 + hw/xwayland/xwayland-window.c | 56 ++++++++++++++++++++++++++++++++++- hw/xwayland/xwayland-window.h | 8 +++++ 4 files changed, 70 insertions(+), 1 deletion(-) diff --git a/hw/xwayland/xwayland-screen.c b/hw/xwayland/xwayland-screen.c index 1e2a345c2..72ea97ac2 100644 --- a/hw/xwayland/xwayland-screen.c +++ b/hw/xwayland/xwayland-screen.c @@ -182,6 +182,7 @@ xwl_close_screen(ScreenPtr screen) struct xwl_screen *xwl_screen = xwl_screen_get(screen); struct xwl_output *xwl_output, *next_xwl_output; struct xwl_seat *xwl_seat, *next_xwl_seat; + struct xwl_wl_surface *xwl_wl_surface, *xwl_wl_surface_next; DeleteCallback(&PropertyStateCallback, xwl_property_callback, screen); @@ -204,6 +205,10 @@ xwl_close_screen(ScreenPtr screen) xwl_screen_destroy_drm_lease_device(xwl_screen, device_data->drm_lease_device); + xorg_list_for_each_entry_safe(xwl_wl_surface, xwl_wl_surface_next, + &xwl_screen->pending_wl_surface_destroy, link) + xwl_window_surface_do_destroy(xwl_wl_surface); + RemoveNotifyFd(xwl_screen->wayland_fd); wl_display_disconnect(xwl_screen->display); @@ -818,6 +823,7 @@ xwl_screen_init(ScreenPtr pScreen, int argc, char **argv) xorg_list_init(&xwl_screen->drm_lease_devices); xorg_list_init(&xwl_screen->queued_drm_lease_devices); xorg_list_init(&xwl_screen->drm_leases); + xorg_list_init(&xwl_screen->pending_wl_surface_destroy); xwl_screen->depth = 24; if (!monitorResolution) diff --git a/hw/xwayland/xwayland-screen.h b/hw/xwayland/xwayland-screen.h index 43414f77d..3c8eb8270 100644 --- a/hw/xwayland/xwayland-screen.h +++ b/hw/xwayland/xwayland-screen.h @@ -107,6 +107,7 @@ struct xwl_screen { struct xorg_list queued_drm_lease_devices; struct xorg_list drm_leases; struct xwl_output *fixed_output; + struct xorg_list pending_wl_surface_destroy; uint32_t serial; #define XWL_FORMAT_ARGB8888 (1 << 0) diff --git a/hw/xwayland/xwayland-window.c b/hw/xwayland/xwayland-window.c index 0f6cacd3f..549de18eb 100644 --- a/hw/xwayland/xwayland-window.c +++ b/hw/xwayland/xwayland-window.c @@ -46,6 +46,8 @@ #include "viewporter-client-protocol.h" #include "xdg-shell-client-protocol.h" +#define DELAYED_WL_SURFACE_DESTROY 1000 /* ms */ + static DevPrivateKeyRec xwl_window_private_key; static DevPrivateKeyRec xwl_damage_private_key; static const char *xwl_surface_tag = "xwl-surface"; @@ -852,6 +854,58 @@ xwl_realize_window(WindowPtr window) return ensure_surface_for_window(window); } +static void +xwl_surface_destroy_free_timer(struct xwl_wl_surface *xwl_wl_surface) +{ + if (xwl_wl_surface->wl_surface_destroy_timer) { + TimerFree(xwl_wl_surface->wl_surface_destroy_timer); + xwl_wl_surface->wl_surface_destroy_timer = NULL; + } +} + +void +xwl_window_surface_do_destroy(struct xwl_wl_surface *xwl_wl_surface) +{ + wl_surface_destroy(xwl_wl_surface->wl_surface); + xorg_list_del(&xwl_wl_surface->link); + xwl_surface_destroy_free_timer(xwl_wl_surface); + free(xwl_wl_surface); +} + +static CARD32 +xwl_surface_destroy_callback(OsTimerPtr timer, CARD32 now, void *arg) +{ + struct xwl_wl_surface *xwl_wl_surface = arg; + + xwl_window_surface_do_destroy(xwl_wl_surface); + + return 0; +} + +static void +release_wl_surface_for_window(struct xwl_window *xwl_window) +{ + struct xwl_wl_surface *xwl_wl_surface; + + /* If the Xserver is terminating, destroy the surface immediately */ + if ((dispatchException & DE_TERMINATE) == DE_TERMINATE) { + wl_surface_destroy(xwl_window->surface); + return; + } + + /* Otherwise, schedule the destruction later, to mitigate the race + * between X11 and Wayland processing so that the compositor has the + * time to establish the association before the wl_surface is destroyed. + */ + xwl_wl_surface = xnfcalloc(1, sizeof *xwl_wl_surface); + xwl_wl_surface->wl_surface = xwl_window->surface; + xorg_list_add(&xwl_wl_surface->link, + &xwl_window->xwl_screen->pending_wl_surface_destroy); + xwl_wl_surface->wl_surface_destroy_timer = + TimerSet(NULL, 0, DELAYED_WL_SURFACE_DESTROY, + xwl_surface_destroy_callback, xwl_wl_surface); +} + Bool xwl_unrealize_window(WindowPtr window) { @@ -906,7 +960,7 @@ xwl_unrealize_window(WindowPtr window) } #endif - wl_surface_destroy(xwl_window->surface); + release_wl_surface_for_window(xwl_window); xorg_list_del(&xwl_window->link_damage); xorg_list_del(&xwl_window->link_window); unregister_damage(window); diff --git a/hw/xwayland/xwayland-window.h b/hw/xwayland/xwayland-window.h index 3dbfd2dc2..49ef3c1f9 100644 --- a/hw/xwayland/xwayland-window.h +++ b/hw/xwayland/xwayland-window.h @@ -37,6 +37,12 @@ #include "xwayland-types.h" +struct xwl_wl_surface { + OsTimerPtr wl_surface_destroy_timer; + struct wl_surface *wl_surface; + struct xorg_list link; +}; + struct xwl_window { struct xwl_screen *xwl_screen; struct wl_surface *surface; @@ -91,6 +97,8 @@ void xwl_move_window(WindowPtr window, Bool xwl_destroy_window(WindowPtr window); void xwl_window_post_damage(struct xwl_window *xwl_window); void xwl_window_create_frame_callback(struct xwl_window *xwl_window); +void xwl_window_surface_do_destroy(struct xwl_wl_surface *xwl_wl_surface); + Bool xwl_window_init(void); #endif /* XWAYLAND_WINDOW_H */