Destroying a wl_buffer that is still attached to a wl_surface is undefined behavior according to the wayland protocol. We should delay the destruction until we get the release event.
To achieve this we need to track ownership of wl_buffers, both our own and the compositor's which occurs from either the first commit request or the first commit request after a release event until the next release event. --- On 12 February 2014 08:54, Pekka Paalanen <ppaala...@gmail.com> wrote: > I assume the code never added a wl_buffer listener before, because if > it did, this patch would be a no-op. "wl_buffer_add_listener" is a > misnomer, there can only ever be one listener, and trying to "add" > another will not actually do anything. Thanks for the headsup. But, indeed, there was nothing adding a wl_buffer listener before. > Also, you rely on wl_buffer.release not having arrived before you add > the listener. With weston's gl-renderer, the release comes very soon > after each wl_surface.commit for wl_shm buffers. Maybe it works, maybe > it doesn't, but it seems very fragile. Did you check you don't leak > wl_buffers now? Right, I was leaking in some cases. So, I came up with the solution below which, if I'm reading the logs correctly, doesn't leak and works correctly. This solution abuses the user_data field to essentially keep two toggle references, our own and the compositor's, and only destroys the wl_buffer when both are dropped. Thanks, Rui --- hw/xfree86/xwayland/xwayland-cursor.c | 4 ++- hw/xfree86/xwayland/xwayland-private.h | 4 +++ hw/xfree86/xwayland/xwayland-window.c | 8 +++--- hw/xfree86/xwayland/xwayland.c | 50 ++++++++++++++++++++++++++++++++++ 4 files changed, 61 insertions(+), 5 deletions(-) diff --git a/hw/xfree86/xwayland/xwayland-cursor.c b/hw/xfree86/xwayland/xwayland-cursor.c index 2b3cb5e..232b038 100644 --- a/hw/xfree86/xwayland/xwayland-cursor.c +++ b/hw/xfree86/xwayland/xwayland-cursor.c @@ -125,6 +125,7 @@ xwl_realize_cursor(DeviceIntPtr device, ScreenPtr screen, CursorPtr cursor) cursor->bits->width, cursor->bits->height, cursor->bits->width * 4, WL_SHM_FORMAT_ARGB8888); + _buffer_init(buffer); wl_shm_pool_destroy(pool); dixSetPrivate(&cursor->devPrivates, @@ -143,7 +144,7 @@ xwl_unrealize_cursor(DeviceIntPtr device, xwl_screen = xwl_screen_get(screen); buffer = dixGetPrivate(&cursor->devPrivates, &xwl_screen->cursor_private_key); - wl_buffer_destroy(buffer); + _buffer_disown(buffer); return TRUE; } @@ -176,6 +177,7 @@ xwl_seat_set_cursor(struct xwl_seat *xwl_seat) xwl_seat->x_cursor->bits->width, xwl_seat->x_cursor->bits->height); wl_surface_commit(xwl_seat->cursor); + _buffer_commited(buffer); } static void diff --git a/hw/xfree86/xwayland/xwayland-private.h b/hw/xfree86/xwayland/xwayland-private.h index bdecf8a..41e7e13 100644 --- a/hw/xfree86/xwayland/xwayland-private.h +++ b/hw/xfree86/xwayland/xwayland-private.h @@ -137,4 +137,8 @@ void xwl_output_remove(struct xwl_output *output); extern const struct xserver_listener xwl_server_listener; +void _buffer_commited(struct wl_buffer *buffer); +void _buffer_disown(struct wl_buffer *buffer); +void _buffer_init(struct wl_buffer *buffer); + #endif /* _XWAYLAND_PRIVATE_H_ */ diff --git a/hw/xfree86/xwayland/xwayland-window.c b/hw/xfree86/xwayland/xwayland-window.c index a2a8206..d18c7f6 100644 --- a/hw/xfree86/xwayland/xwayland-window.c +++ b/hw/xfree86/xwayland/xwayland-window.c @@ -62,10 +62,8 @@ xwl_window_attach(struct xwl_window *xwl_window, PixmapPtr pixmap) struct xwl_screen *xwl_screen = xwl_window->xwl_screen; struct wl_callback *callback; - /* We can safely destroy the buffer because we only use one buffer - * per surface in xwayland model */ if (xwl_window->buffer) - wl_buffer_destroy(xwl_window->buffer); + _buffer_disown(xwl_window->buffer); xwl_screen->driver->create_window_buffer(xwl_window, pixmap); @@ -74,6 +72,8 @@ xwl_window_attach(struct xwl_window *xwl_window, PixmapPtr pixmap) return; } + _buffer_init(xwl_window->buffer); + wl_surface_attach(xwl_window->surface, xwl_window->buffer, 0, 0); wl_surface_damage(xwl_window->surface, 0, 0, pixmap->drawable.width, @@ -185,7 +185,7 @@ xwl_unrealize_window(WindowPtr window) return ret; if (xwl_window->buffer) - wl_buffer_destroy(xwl_window->buffer); + _buffer_disown(xwl_window->buffer); wl_surface_destroy(xwl_window->surface); xorg_list_del(&xwl_window->link); if (RegionNotEmpty(DamageRegion(xwl_window->damage))) diff --git a/hw/xfree86/xwayland/xwayland.c b/hw/xfree86/xwayland/xwayland.c index c70a52d..c373fcc 100644 --- a/hw/xfree86/xwayland/xwayland.c +++ b/hw/xfree86/xwayland/xwayland.c @@ -72,6 +72,55 @@ const struct xserver_listener xwl_server_listener = { }; static void +_buffer_add_bits(struct wl_buffer *buffer, unsigned long bits) +{ + unsigned long ref = (unsigned long) wl_buffer_get_user_data(buffer); + ref |= bits; + wl_buffer_set_user_data(buffer, (void *) ref); +} + +static void +_buffer_clear_bits(struct wl_buffer *buffer, unsigned long bits) +{ + unsigned long ref = (unsigned long) wl_buffer_get_user_data(buffer); + ref &= ~bits; + if (!ref) + wl_buffer_destroy(buffer); + else + wl_buffer_set_user_data(buffer, (void *) ref); +} + +static void +_buffer_release_handler(void *data, struct wl_buffer *buffer) +{ + _buffer_clear_bits(buffer, 1 << 1); +} + +static const struct wl_buffer_listener _buffer_listener = { + _buffer_release_handler, +}; + +void +_buffer_commited(struct wl_buffer *buffer) +{ + _buffer_add_bits(buffer, 1 << 1); +} + +void +_buffer_disown(struct wl_buffer *buffer) +{ + _buffer_clear_bits(buffer, 1 << 0); +} + +void +_buffer_init(struct wl_buffer *buffer) +{ + /* setup the release listener and add the "owned" reference */ + wl_buffer_add_listener(buffer, &_buffer_listener, (void *) (1 << 0)); +} + + +static void xwl_input_delayed_init(void *data, struct wl_callback *callback, uint32_t time) { struct xwl_screen *xwl_screen = data; @@ -350,6 +399,7 @@ void xwl_screen_post_damage(struct xwl_screen *xwl_screen) xwl_window->buffer, 0, 0); wl_surface_commit(xwl_window->surface); + _buffer_commited(xwl_window->buffer); DamageEmpty(xwl_window->damage); } -- 1.8.3.1 _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel