On Fri, 29 Apr 2016 19:30:22 +0800
Jonas Ådahl <jad...@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 <jad...@gmail.com>
> Reviewed-by: Pekka Paalanen <pekka.paala...@collabora.co.uk>
> Reviewed-by: Derek Foreman <der...@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

Attachment: pgpw94TjW3GEf.pgp
Description: OpenPGP digital signature

_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to