[PATCH wayland] client: ensure thread safety for wl_display_roundtrip_queue()

[Jonas Ådahl]

On Mon, Feb 15, 2016 at 02:19:43PM +0000, Ucan, Emre (ADITG/SW1) wrote:
> We have to block other threads from reading events. Otherwise other threads
> may assign done event to the default queue, if they call
> wl_display_read_events before setting the queue for the callback proxy.
> This would cause a deadlock in wl_display_dispatch_queue,
> because the default queue is not dispatched.
> 
> We can call wl_display_prepare_read_queue API which blocks on success
> other threads from reading events and call wl_display_cancel read after
> the queue is set to the proxy. This new implementation ensures thread
> safety for the API.

This is the work-around available to avoid the issue, but it is not the
correct solution for libwayland-client. Right now we have two potential
solutions: one is to add a "proxy wrapper" where one configures the
proxy wrapper object (setting the queue) before using it to issue
requests, the other is to add thread safe functions and patch the scanner
to add thread safe helpers which would be used instead of for example
"wl_display_sync()".

The proxy wrapper approach can be tested out by appling this patch:
https://lists.freedesktop.org/archives/wayland-devel/2015-June/023054.html

For more information, check the corresponding bug in bugzilla:
https://bugs.freedesktop.org/show_bug.cgi?id=91273


Jonas

> 
> Signed-off-by: Emre Ucan <eucan at de.adit-jv.com>
> ---
>  src/wayland-client.c |   16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/src/wayland-client.c b/src/wayland-client.c
> index ef12bfc..00d8cda 100644
> --- a/src/wayland-client.c
> +++ b/src/wayland-client.c
> @@ -1075,12 +1075,26 @@ wl_display_roundtrip_queue(struct wl_display *display, struct wl_event_queue *qu
>  	struct wl_callback *callback;
>  	int done, ret = 0;
>  
> +	/*We have to block other threads from reading events. Otherwise other threads
> +	 * may assign done event to the default queue, if they call wl_display_read_events
> +	 * before setting the queue for the callback proxy. This would cause a deadlock
> +	 * in wl_display_dispatch_queue, because the default queue is not dispatched.*/
> +	while (ret >= 0 && wl_display_prepare_read_queue(display, queue))
> +		ret = wl_display_dispatch_queue_pending(display, queue);
> +
> +	if (ret < 0)
> +		return ret;
> +
>  	done = 0;
>  	callback = wl_display_sync(display);
> -	if (callback == NULL)
> +	if (callback == NULL) {
> +		wl_display_cancel_read(display);
>  		return -1;
> +	}
> +
>  	wl_proxy_set_queue((struct wl_proxy *) callback, queue);
>  	wl_callback_add_listener(callback, &sync_listener, &done);
> +	wl_display_cancel_read(display);
>  	while (!done && ret >= 0)
>  		ret = wl_display_dispatch_queue(display, queue);
>  
> -- 
> 1.7.9.5
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel