[PATCH v2 wayland] shm: Add shm_buffer ref and shm_pool unref functions

[Pekka Paalanen]

On Mon,  5 Oct 2015 13:12:12 -0500
Derek Foreman <derekf at osg.samsung.com> wrote:

> Sometimes the compositor wants to make sure a shm pool doesn't disappear
> out from under it.
> 
> For example, in Enlightenment, rendering happens in a separate thread
> while the main thread can still dispatch events.  If a client is destroyed
> during rendering, all its resources are cleaned up and its shm pools are
> unmapped.  This causes the rendering thread to segfault.
> 
> This patch adds a way for the compositor to increment the refcount of the
> shm pool so it can't disappear, and decrement it when it's finished.
> 
> The ref/unref are asymmetrical (ref returns the pool) because it's
> possible the buffer itself will be gone when you need to unref the pool.
> 
> Signed-off-by: Derek Foreman <derekf at osg.samsung.com>
> ---
> 
> Changes from v1:
>  renamed functions to wl_shm_buffer_ref_pool and wl_shm_pool_unref
>  added doxy
> 
>  src/wayland-server-core.h |  7 +++++++
>  src/wayland-shm.c         | 36 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 43 insertions(+)
> 
> diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h
> index e605432..4c2bdfe 100644
> --- a/src/wayland-server-core.h
> +++ b/src/wayland-server-core.h
> @@ -362,6 +362,7 @@ wl_resource_get_destroy_listener(struct wl_resource *resource,
>  	     resource = tmp,							\
>  	     tmp = wl_resource_from_link(wl_resource_get_link(resource)->next))
>  
> +struct wl_shm_pool;
>  struct wl_shm_buffer;
>  
>  void
> @@ -388,6 +389,12 @@ wl_shm_buffer_get_width(struct wl_shm_buffer *buffer);
>  int32_t
>  wl_shm_buffer_get_height(struct wl_shm_buffer *buffer);
>  
> +struct wl_shm_pool *
> +wl_shm_buffer_ref_pool(struct wl_shm_buffer *buffer);
> +
> +void
> +wl_shm_pool_unref(struct wl_shm_pool *pool);
> +
>  int
>  wl_display_init_shm(struct wl_display *display);
>  
> diff --git a/src/wayland-shm.c b/src/wayland-shm.c
> index 5c419fa..4f9d9d0 100644
> --- a/src/wayland-shm.c
> +++ b/src/wayland-shm.c
> @@ -412,6 +412,42 @@ wl_shm_buffer_get_height(struct wl_shm_buffer *buffer)
>  	return buffer->height;
>  }
>  
> +/** Get a reference to a shm_buffer's shm_pool
> + *
> + * \param buffer The buffer object
> + *
> + * Returns a pointer to a buffer's shm_pool and increases the
> + * shm_pool refcount.
> + *
> + * This refcount keeps the shm pool from being destroyed on client
> + * disconnect, so the compositor must call wl_shm_pool_unref on it
> + * when finished.

It's not the disconnect directly, but cleaning up all wl_resources
after a disconnect causes the side-effect of the pool refcount dropping
to zero. Not sure that's an important detail to mention here, though.
The client can also do this without a disconnect, by just destroying
all relevant wl_proxies.

> + *
> + * \memberof wl_shm_buffer

See also: wl_shm_pool_unref.

> + */
> +WL_EXPORT struct wl_shm_pool *
> +wl_shm_buffer_ref_pool(struct wl_shm_buffer *buffer)
> +{
> +	assert(buffer->pool->refcount);
> +
> +	buffer->pool->refcount++;
> +	return buffer->pool;
> +}
> +
> +/** Unreference a shm_pool
> + *
> + * \param buffer The pool object
> + *
> + * Drops a reference to a wl_shm_pool object.
> + *
> + * \memberof wl_shm_buffer

wl_shm_buffer?

I think we already have an empty wl_shm_pool section in the docs,
wouldn't this belong there, with a see also wl_shm_buffer_ref_pool()?

If Giulio or anyone still needs to get the fd of a shm buffer or
anything, that would be naturally done by taking a ref for the
wl_shm_pool, and then asking the pool for its fd and size.

> + */
> +WL_EXPORT void
> +wl_shm_pool_unref(struct wl_shm_pool *pool)
> +{
> +	shm_pool_unref(pool);
> +}
> +
>  static void
>  reraise_sigbus(void)
>  {

Anyway, documentation bikeshedding apart, this patch looks good to me,
so:

Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>

I did not test this in any way.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20151016/dc047507/attachment.sig>