From 57359da9b43d543cee05688a6b2d3ae4d3980fdb Mon Sep 17 00:00:00 2001 From: "Jasper St. Pierre" Date: Thu, 20 Mar 2014 13:20:47 -0400 Subject: [PATCH] wayland: Kill the buffer destroy error Previously, a sequence like this would crash a client: => surface.attach(buffer) => buffer.destroy() The correct behavior is to wait until we release the buffer before destroying it. => surface.attach(buffer) => surface.attach(buffer2) <= buffer.release() => buffer.destroy() The protocol upstream says that "the surface contents are undefined" in a case like this. Personally, I think that this is broken behavior and no client should ever do it, so I explicitly killed any client that tried to do this. But unfortunately, as we're all well aware, XWayland does this. Rather than wait for XWayland to be fixed, let's just allow this. Technically, since we always copy SHM buffers into GL textures, we could release the buffer as soon as the Cogl texture is made. Since we do this copy, the semantics we apply are that the texture is "frozen" in time until another newer buffer is attached. For simple clients that simply abort on exit and don't wait for the buffer event anyhow, this has the added bonus that we'll get nice destroy animations. --- src/compositor/meta-surface-actor-wayland.c | 45 ++++++++++++++++----- src/wayland/meta-wayland-surface.c | 18 ++++----- src/wayland/meta-wayland.c | 1 - 3 files changed, 43 insertions(+), 21 deletions(-) diff --git a/src/compositor/meta-surface-actor-wayland.c b/src/compositor/meta-surface-actor-wayland.c index bdd8df05c..875dd1c6b 100644 --- a/src/compositor/meta-surface-actor-wayland.c +++ b/src/compositor/meta-surface-actor-wayland.c @@ -35,11 +35,24 @@ struct _MetaSurfaceActorWaylandPrivate { MetaWaylandSurface *surface; MetaWaylandBuffer *buffer; + struct wl_listener buffer_destroy_listener; }; typedef struct _MetaSurfaceActorWaylandPrivate MetaSurfaceActorWaylandPrivate; G_DEFINE_TYPE_WITH_PRIVATE (MetaSurfaceActorWayland, meta_surface_actor_wayland, META_TYPE_SURFACE_ACTOR) +static void +meta_surface_actor_handle_buffer_destroy (struct wl_listener *listener, void *data) +{ + MetaSurfaceActorWaylandPrivate *priv = wl_container_of (listener, priv, buffer_destroy_listener); + + /* If the buffer is destroyed while we're attached to it, + * we want to unset priv->buffer so we don't access freed + * memory. Keep the texture set however so the user doesn't + * see the window disappear. */ + priv->buffer = NULL; +} + static void meta_surface_actor_wayland_process_damage (MetaSurfaceActor *actor, int x, int y, int width, int height) @@ -47,16 +60,19 @@ meta_surface_actor_wayland_process_damage (MetaSurfaceActor *actor, MetaSurfaceActorWayland *self = META_SURFACE_ACTOR_WAYLAND (actor); MetaSurfaceActorWaylandPrivate *priv = meta_surface_actor_wayland_get_instance_private (self); - struct wl_resource *resource = priv->buffer->resource; - struct wl_shm_buffer *shm_buffer = wl_shm_buffer_get (resource); - - if (shm_buffer) + if (priv->buffer) { - CoglTexture2D *texture = COGL_TEXTURE_2D (priv->buffer->texture); - cogl_wayland_texture_set_region_from_shm_buffer (texture, x, y, width, height, shm_buffer, x, y, 0, NULL); - } + struct wl_resource *resource = priv->buffer->resource; + struct wl_shm_buffer *shm_buffer = wl_shm_buffer_get (resource); - meta_surface_actor_update_area (META_SURFACE_ACTOR (self), x, y, width, height); + if (shm_buffer) + { + CoglTexture2D *texture = COGL_TEXTURE_2D (priv->buffer->texture); + cogl_wayland_texture_set_region_from_shm_buffer (texture, x, y, width, height, shm_buffer, x, y, 0, NULL); + } + + meta_surface_actor_update_area (META_SURFACE_ACTOR (self), x, y, width, height); + } } static void @@ -127,6 +143,9 @@ meta_surface_actor_wayland_class_init (MetaSurfaceActorWaylandClass *klass) static void meta_surface_actor_wayland_init (MetaSurfaceActorWayland *self) { + MetaSurfaceActorWaylandPrivate *priv = meta_surface_actor_wayland_get_instance_private (self); + + priv->buffer_destroy_listener.notify = meta_surface_actor_handle_buffer_destroy; } MetaSurfaceActor * @@ -149,10 +168,16 @@ meta_surface_actor_wayland_set_buffer (MetaSurfaceActorWayland *self, MetaSurfaceActorWaylandPrivate *priv = meta_surface_actor_wayland_get_instance_private (self); MetaShapedTexture *stex = meta_surface_actor_get_texture (META_SURFACE_ACTOR (self)); + if (priv->buffer) + wl_list_remove (&priv->buffer_destroy_listener.link); + priv->buffer = buffer; - if (buffer) - meta_shaped_texture_set_texture (stex, buffer->texture); + if (priv->buffer) + { + wl_signal_add (&priv->buffer->destroy_signal, &priv->buffer_destroy_listener); + meta_shaped_texture_set_texture (stex, priv->buffer->texture); + } else meta_shaped_texture_set_texture (stex, NULL); } diff --git a/src/wayland/meta-wayland-surface.c b/src/wayland/meta-wayland-surface.c index 940c32f0f..739c17133 100644 --- a/src/wayland/meta-wayland-surface.c +++ b/src/wayland/meta-wayland-surface.c @@ -74,16 +74,6 @@ typedef struct struct wl_listener sibling_destroy_listener; } MetaWaylandSubsurfacePlacementOp; -static void -surface_handle_buffer_destroy (struct wl_listener *listener, void *data) -{ - MetaWaylandSurface *surface = wl_container_of (listener, surface, buffer_destroy_listener); - - wl_resource_post_error (surface->resource, WL_DISPLAY_ERROR_INVALID_OBJECT, - "Destroyed buffer while it was attached to the surface"); - surface->buffer = NULL; -} - static void surface_set_buffer (MetaWaylandSurface *surface, MetaWaylandBuffer *buffer) @@ -106,6 +96,14 @@ surface_set_buffer (MetaWaylandSurface *surface, } } +static void +surface_handle_buffer_destroy (struct wl_listener *listener, void *data) +{ + MetaWaylandSurface *surface = wl_container_of (listener, surface, buffer_destroy_listener); + + surface_set_buffer (surface, NULL); +} + static void surface_process_damage (MetaWaylandSurface *surface, cairo_region_t *region) diff --git a/src/wayland/meta-wayland.c b/src/wayland/meta-wayland.c index a01d6f971..79ebd302d 100644 --- a/src/wayland/meta-wayland.c +++ b/src/wayland/meta-wayland.c @@ -151,7 +151,6 @@ meta_wayland_buffer_unref (MetaWaylandBuffer *buffer) if (buffer->ref_count == 0) { g_clear_pointer (&buffer->texture, cogl_object_unref); - g_assert (wl_resource_get_client (buffer->resource)); wl_resource_queue_event (buffer->resource, WL_BUFFER_RELEASE); } }