[Cc-ing Daniel/Pekka]
Hi all,
On 24 June 2016 at 03:46, Jonas Ådahl <jad...@gmail.com> wrote:
> When EGL is used on some other thread than the thread that drives the
> main wl_display queue, the Wayland EGL dri2 implementation is
> vulnerable to a race condition related to display round trips and global
> object advertisements.
>
> The race that may happen is that after after a proxy is created, but
> before the queue is set, events meant to be emitted via the yet to be
> set queue may already have been queued on the wrong queue.
>
> In order to make it possible to avoid this race, wayland 1.11
> introduced new API that allows creating a proxy wrapper that may be used
> as the factory proxy when creating new proxies via Wayland requests. The
> queue of a proxy wrapper can be changed without effecting what queue
> events emitted by the actual proxy will be queued on, while still
> effecting what default queue proxies created from it will have.
>
> By introducing a wl_display proxy wrapper and using this when performing
> round trips (via wl_display_sync()) and retrieving the global objects (via
> wl_display_get_registry()), the mentioned race condition is avoided.
>
> Signed-off-by: Jonas Ådahl <jad...@gmail.com>
> ---
>
> Changes since v1:
>
> - Added proxy clean up on tear down
> - Changed required version to the stable wayland release (1.11)
>
>
> configure.ac | 2 +-
> src/egl/drivers/dri2/egl_dri2.c | 1 +
> src/egl/drivers/dri2/egl_dri2.h | 1 +
> src/egl/drivers/dri2/platform_wayland.c | 30 ++++++++++++++++--------------
> 4 files changed, 19 insertions(+), 15 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 023110e..d73af83 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -83,7 +83,7 @@ GLPROTO_REQUIRED=1.4.14
> LIBOMXIL_BELLAGIO_REQUIRED=0.0
> LIBVA_REQUIRED=0.38.0
> VDPAU_REQUIRED=1.1
> -WAYLAND_REQUIRED=1.2.0
> +WAYLAND_REQUIRED=1.11
> XCB_REQUIRED=1.9.3
> XCBDRI2_REQUIRED=1.8
> XCBGLX_REQUIRED=1.8.1
> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
> index d8448f4..88191b4 100644
> --- a/src/egl/drivers/dri2/egl_dri2.c
> +++ b/src/egl/drivers/dri2/egl_dri2.c
> @@ -849,6 +849,7 @@ dri2_terminate(_EGLDriver *drv, _EGLDisplay *disp)
> wl_shm_destroy(dri2_dpy->wl_shm);
> wl_registry_destroy(dri2_dpy->wl_registry);
> wl_event_queue_destroy(dri2_dpy->wl_queue);
> + wl_proxy_wrapper_destroy(dri2_dpy->wl_dpy_wrapper);
> if (dri2_dpy->own_device) {
> wl_display_disconnect(dri2_dpy->wl_dpy);
> }
> diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
> index ddb5f39..075c2a4 100644
> --- a/src/egl/drivers/dri2/egl_dri2.h
> +++ b/src/egl/drivers/dri2/egl_dri2.h
> @@ -205,6 +205,7 @@ struct dri2_egl_display
>
> #ifdef HAVE_WAYLAND_PLATFORM
> struct wl_display *wl_dpy;
> + struct wl_display *wl_dpy_wrapper;
> struct wl_registry *wl_registry;
> struct wl_drm *wl_server_drm;
> struct wl_drm *wl_drm;
> diff --git a/src/egl/drivers/dri2/platform_wayland.c
> b/src/egl/drivers/dri2/platform_wayland.c
> index ff0d5c8..519469e 100644
> --- a/src/egl/drivers/dri2/platform_wayland.c
> +++ b/src/egl/drivers/dri2/platform_wayland.c
> @@ -74,9 +74,8 @@ roundtrip(struct dri2_egl_display *dri2_dpy)
> struct wl_callback *callback;
> int done = 0, ret = 0;
>
> - callback = wl_display_sync(dri2_dpy->wl_dpy);
> + callback = wl_display_sync(dri2_dpy->wl_dpy_wrapper);
> wl_callback_add_listener(callback, &sync_listener, &done);
> - wl_proxy_set_queue((struct wl_proxy *) callback, dri2_dpy->wl_queue);
> while (ret != -1 && !done)
> ret = wl_display_dispatch_queue(dri2_dpy->wl_dpy, dri2_dpy->wl_queue);
>
> @@ -765,11 +764,9 @@ dri2_wl_swap_buffers_with_damage(_EGLDriver *drv,
> * handle the commit and send a release event before checking for a free
> * buffer */
> if (dri2_surf->throttle_callback == NULL) {
> - dri2_surf->throttle_callback = wl_display_sync(dri2_dpy->wl_dpy);
> + dri2_surf->throttle_callback =
> wl_display_sync(dri2_dpy->wl_dpy_wrapper);
> wl_callback_add_listener(dri2_surf->throttle_callback,
> &throttle_listener, dri2_surf);
> - wl_proxy_set_queue((struct wl_proxy *) dri2_surf->throttle_callback,
> - dri2_dpy->wl_queue);
> }
>
> wl_display_flush(dri2_dpy->wl_dpy);
> @@ -1093,12 +1090,17 @@ dri2_initialize_wayland_drm(_EGLDriver *drv,
> _EGLDisplay *disp)
>
> dri2_dpy->wl_queue = wl_display_create_queue(dri2_dpy->wl_dpy);
>
> + dri2_dpy->wl_dpy_wrapper = wl_proxy_create_wrapper(dri2_dpy->wl_dpy);
> + if (dri2_dpy->wl_dpy_wrapper == NULL)
> + goto cleanup_dpy_wrapper;
> +
> + wl_proxy_set_queue((struct wl_proxy *) dri2_dpy->wl_dpy_wrapper,
> + dri2_dpy->wl_queue);
> +
> if (dri2_dpy->own_device)
> wl_display_dispatch_pending(dri2_dpy->wl_dpy);
>
> - dri2_dpy->wl_registry = wl_display_get_registry(dri2_dpy->wl_dpy);
> - wl_proxy_set_queue((struct wl_proxy *) dri2_dpy->wl_registry,
> - dri2_dpy->wl_queue);
> + dri2_dpy->wl_registry = wl_display_get_registry(dri2_dpy->wl_dpy_wrapper);
> wl_registry_add_listener(dri2_dpy->wl_registry,
> ®istry_listener_drm, dri2_dpy);
> if (roundtrip(dri2_dpy) < 0 || dri2_dpy->wl_drm == NULL)
> @@ -1235,6 +1237,10 @@ dri2_initialize_wayland_drm(_EGLDriver *drv,
> _EGLDisplay *disp)
> wl_drm_destroy(dri2_dpy->wl_drm);
> cleanup_registry:
> wl_registry_destroy(dri2_dpy->wl_registry);
> + wl_proxy_wrapper_destroy(dri2_dpy->wl_dpy_wrapper);
> + cleanup_dpy_wrapper:
> + if (dri2_dpy->wl_dpy != disp->PlatformDisplay)
> + wl_display_disconnect(dri2_dpy->wl_dpy);
> wl_event_queue_destroy(dri2_dpy->wl_queue);
> cleanup_dpy:
> free(dri2_dpy);
> @@ -1546,11 +1552,9 @@ dri2_wl_swrast_commit_backbuffer(struct
> dri2_egl_surface *dri2_surf)
> * handle the commit and send a release event before checking for a free
> * buffer */
> if (dri2_surf->throttle_callback == NULL) {
> - dri2_surf->throttle_callback = wl_display_sync(dri2_dpy->wl_dpy);
> + dri2_surf->throttle_callback =
> wl_display_sync(dri2_dpy->wl_dpy_wrapper);
> wl_callback_add_listener(dri2_surf->throttle_callback,
> &throttle_listener, dri2_surf);
> - wl_proxy_set_queue((struct wl_proxy *) dri2_surf->throttle_callback,
> - dri2_dpy->wl_queue);
> }
>
> wl_display_flush(dri2_dpy->wl_dpy);
> @@ -1824,9 +1828,7 @@ dri2_initialize_wayland_swrast(_EGLDriver *drv,
> _EGLDisplay *disp)
> if (dri2_dpy->own_device)
> wl_display_dispatch_pending(dri2_dpy->wl_dpy);
>
> - dri2_dpy->wl_registry = wl_display_get_registry(dri2_dpy->wl_dpy);
> - wl_proxy_set_queue((struct wl_proxy *) dri2_dpy->wl_registry,
> - dri2_dpy->wl_queue);
> + dri2_dpy->wl_registry = wl_display_get_registry(dri2_dpy->wl_dpy_wrapper);
> wl_registry_add_listener(dri2_dpy->wl_registry,
> ®istry_listener_swrast, dri2_dpy);
>
Can we have someone with more wayland experience than me look into the
above patch ?
Thanks
Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
