[PATCH wayland v4] client: Introduce proxy wrappers

[Pekka Paalanen]

On Fri, 29 Apr 2016 19:30:22 +0800
Jonas Ådahl <jadahl at gmail.com> wrote:

> Using the libwayland-client client API with multiple threads with
> thread local queues are prone to race conditions.
> 
> The problem is that one thread can read and queue events after another
> thread creates a proxy but before it sets the queue.
> 
> This may result in the event to the proxy being silently dropped, or
> potentially dispatched on the wrong thread had the creating thread set
> the implementation before setting the queue.
> 
> This patch introduces API to solve this case by introducing "proxy
> wrappers". In short, a proxy wrapper is a wl_proxy struct that will
> never itself proxy any events, but may be used by the client to set a
> queue, and use it instead of the original proxy when sending requests
> that creates new proxies. When sending requests, the wrapper will
> work in the same way as the normal proxy object, but the proxy created
> by sending a request (for example wl_display.sync) will inherit to the
> same proxy queue as the wrapper.
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=91273
> 
> Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
> Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> Reviewed-by: Derek Foreman <derekf at osg.samsung.com>
> ---
> 
> Changes since v3:
> 
>  - Migrated a couple of more wl_log(..); to wl_abort() that were missed in
>    the previous iteration.
>  - Removed any parent-proxy checks in the wrapper creator.
> 
> 
> 
>  src/wayland-client-core.h |   6 +++
>  src/wayland-client.c      | 110 +++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 115 insertions(+), 1 deletion(-)

Hi Jonas,

yup, this looks good. This and the tests v4 patch pushed:
   9a652c4..69ec70f  master -> master



> +WL_EXPORT void *
> +wl_proxy_create_wrapper(void *proxy)
> +{
> +	struct wl_proxy *wrapped_proxy = proxy;
> +	struct wl_proxy *wrapper;
> +
> +	wrapper = zalloc(sizeof *wrapper);
> +	if (!wrapper)
> +		return NULL;
> +
> +	pthread_mutex_lock(&wrapped_proxy->display->mutex);
> +
> +	wrapper->object.interface = wrapped_proxy->object.interface;
> +	wrapper->object.id = wrapped_proxy->object.id;
> +	wrapper->version = wrapped_proxy->version;
> +	wrapper->display = wrapped_proxy->display;
> +	wrapper->queue = wrapped_proxy->queue;
> +	wrapper->flags = WL_PROXY_FLAG_WRAPPER;
> +	wrapper->refcount = 1;
> +
> +	pthread_mutex_unlock(&wrapped_proxy->display->mutex);
> +
> +	return wrapper;
> +}

I didn't want to postpone the landing of this any longer, but I still
wonder if we really need that locking at all.


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: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160429/6cad43da/attachment-0001.sig>