From: Alexandros Frantzis alexandros.frantzis@collabora.com
Don't access the parent wayland_surface through their HWND while holding a lock to the child surface, since this requires acquiring the win_data_mutex and may lead to a deadlock.
Instead introduce a weak reference mechanism and use that to directly access the parent wayland_surface. --- dlls/winewayland.drv/wayland_surface.c | 58 +++++++++++++++++++++----- dlls/winewayland.drv/waylanddrv.h | 7 +++- dlls/winewayland.drv/window.c | 6 ++- 3 files changed, 58 insertions(+), 13 deletions(-)
diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index 425c270099e..57fd217e0d3 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -168,6 +168,7 @@ struct wayland_surface *wayland_surface_create(HWND hwnd) }
surface->window.scale = 1.0; + surface->weak_ref = 1;
return surface;
@@ -176,6 +177,43 @@ err: return NULL; }
+/********************************************************************** + * wayland_surface_get_weak_ref + * + * Gets a weak reference to a wayland_surface. + */ +struct wayland_surface *wayland_surface_get_weak_ref(struct wayland_surface *surface) +{ + InterlockedIncrement(&surface->weak_ref); + return surface; +} + +/********************************************************************** + * wayland_surface_get_weak_ref + * + * Releases a weak reference to a wayland_surface. + */ +void wayland_surface_release_weak_ref(struct wayland_surface *surface) +{ + if (InterlockedDecrement(&surface->weak_ref) > 0) return; + pthread_mutex_destroy(&surface->mutex); + free(surface); +} + +/********************************************************************** + * wayland_surface_lock_weak_ref + * + * Returns a locked wayland_surface from a weak reference, or NULL if the + * surface has been destroyed. + */ +struct wayland_surface *wayland_surface_lock_weak_ref(struct wayland_surface *surface) +{ + pthread_mutex_lock(&surface->mutex); + if (!surface->destroyed) return surface; + pthread_mutex_unlock(&surface->mutex); + return NULL; +} + /********************************************************************** * wayland_surface_destroy * @@ -237,9 +275,7 @@ void wayland_surface_destroy(struct wayland_surface *surface)
wl_display_flush(process_wayland.wl_display);
- pthread_mutex_destroy(&surface->mutex); - - free(surface); + wayland_surface_release_weak_ref(surface); }
/********************************************************************** @@ -297,7 +333,7 @@ void wayland_surface_make_subsurface(struct wayland_surface *surface, }
surface->role = WAYLAND_SURFACE_ROLE_SUBSURFACE; - surface->parent_hwnd = parent->hwnd; + surface->parent_weak_ref = wayland_surface_get_weak_ref(parent);
/* Let parent handle all pointer events. */ empty_region = wl_compositor_create_region(process_wayland.wl_compositor); @@ -350,11 +386,16 @@ void wayland_surface_clear_role(struct wayland_surface *surface) surface->wl_subsurface = NULL; }
+ if (surface->parent_weak_ref) + { + wayland_surface_release_weak_ref(surface->parent_weak_ref); + surface->parent_weak_ref = NULL; + } + memset(&surface->pending, 0, sizeof(surface->pending)); memset(&surface->requested, 0, sizeof(surface->requested)); memset(&surface->processing, 0, sizeof(surface->processing)); memset(&surface->current, 0, sizeof(surface->current)); - surface->parent_hwnd = 0;
/* Ensure no buffer is attached, otherwise future role assignments may fail. */ wl_surface_attach(surface->wl_surface, NULL, 0, 0); @@ -646,12 +687,9 @@ static void wayland_surface_reconfigure_subsurface(struct wayland_surface *surfa struct wayland_surface *parent; int local_x, local_y, x, y;
- /* TODO: Locking the parent surface using the HWND may lead to a deadlock, - * since we will try to acquire the win_data_mutex while holding a surface - * mutex (from the argument of this function), whereas all other paths - * acquire the win_data_mutex before any surface mutex. */ if (surface->processing.serial && surface->processing.processed && - (parent = wayland_surface_lock_hwnd(surface->parent_hwnd))) + surface->parent_weak_ref && + (parent = wayland_surface_lock_weak_ref(surface->parent_weak_ref))) { /* For now we use a subsurface only for child windows, whose window * coordinates are relative to the client area of their parent. */ diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 22e8b27df82..d553f06a420 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -213,7 +213,9 @@ struct wayland_surface int buffer_width, buffer_height; HCURSOR hcursor; enum wayland_surface_role role; - HWND parent_hwnd; + LONG weak_ref; + BOOL destroyed; + struct wayland_surface *parent_weak_ref; };
struct wayland_shm_buffer @@ -247,6 +249,9 @@ void wayland_output_use_xdg_extension(struct wayland_output *output); */
struct wayland_surface *wayland_surface_create(HWND hwnd); +struct wayland_surface *wayland_surface_get_weak_ref(struct wayland_surface *surface); +void wayland_surface_release_weak_ref(struct wayland_surface *surface); +struct wayland_surface *wayland_surface_lock_weak_ref(struct wayland_surface *surface); void wayland_surface_destroy(struct wayland_surface *surface); void wayland_surface_make_toplevel(struct wayland_surface *surface); void wayland_surface_make_subsurface(struct wayland_surface *surface, diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index de395389264..8453d73488c 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -240,7 +240,8 @@ static void wayland_win_data_update_wayland_surface(struct wayland_win_data *dat
if ((role == WAYLAND_SURFACE_ROLE_TOPLEVEL) != !!(surface->xdg_toplevel) || (role == WAYLAND_SURFACE_ROLE_SUBSURFACE) != !!(surface->wl_subsurface) || - (role == WAYLAND_SURFACE_ROLE_SUBSURFACE && surface->parent_hwnd != parent_data->hwnd)) + (role == WAYLAND_SURFACE_ROLE_SUBSURFACE && + surface->parent_weak_ref && surface->parent_weak_ref->hwnd != parent_data->hwnd)) { /* If we have a pre-existing surface ensure it has no role. */ if (data->wayland_surface) wayland_surface_clear_role(surface); @@ -290,7 +291,8 @@ static void wayland_win_data_update_wayland_state(struct wayland_win_data *data)
if (surface->wl_subsurface) { - TRACE("hwnd=%p subsurface parent=%p\n", surface->hwnd, surface->parent_hwnd); + TRACE("hwnd=%p subsurface parent=%p\n", surface->hwnd, + surface->parent_weak_ref ? surface->parent_weak_ref->hwnd : 0); /* Although subsurfaces don't have a dedicated surface config mechanism, * we use the config fields to mark them as updated. */ surface->processing.serial = 1;