[PATCH] wayland: Don't race when releasing named buffers
Jonas Ådahl
jadahl at gmail.com
Wed Oct 23 01:06:10 PDT 2013
Still needs review.
On Wed, Oct 2, 2013 at 5:06 PM, Jonas Ådahl <jadahl at gmail.com> wrote:
> This patch fixes a race when a client releases a named buffer before the
> server had the time to handle 'wl_drm_create_buffer'. When triggered,
> the server would fail to create the buffer since the client already
> having destroyed it, throwing out the client in the process with a
> protocol error.
>
> To solve this, use a lazy non-blocking roundtrip when creating the
> buffer that will only potentially block when releasing if the roundtrip
> callback was not already invoked.
>
> Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
> ---
> src/egl/drivers/dri2/egl_dri2.h | 1 +
> src/egl/drivers/dri2/platform_wayland.c | 74 ++++++++++++++++++++++++++++++++-
> 2 files changed, 73 insertions(+), 2 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
> index fba5f81..6508612 100644
> --- a/src/egl/drivers/dri2/egl_dri2.h
> +++ b/src/egl/drivers/dri2/egl_dri2.h
> @@ -189,6 +189,7 @@ struct dri2_egl_surface
> struct wl_buffer *wl_buffer;
> __DRIimage *dri_image;
> int pitch, name;
> + struct wl_callback *created_callback;
> #endif
> #ifdef HAVE_DRM_PLATFORM
> struct gbm_bo *bo;
> diff --git a/src/egl/drivers/dri2/platform_wayland.c b/src/egl/drivers/dri2/platform_wayland.c
> index 1d417bb..3a873f7 100644
> --- a/src/egl/drivers/dri2/platform_wayland.c
> +++ b/src/egl/drivers/dri2/platform_wayland.c
> @@ -184,6 +184,55 @@ dri2_create_window_surface(_EGLDriver *drv, _EGLDisplay *disp,
> window, attrib_list);
> }
>
> +static void
> +buffer_create_sync_callback(void *data,
> + struct wl_callback *callback,
> + uint32_t serial)
> +{
> + struct wl_callback **created_callback = data;
> +
> + *created_callback = NULL;
> + wl_callback_destroy(callback);
> +}
> +
> +static const struct wl_callback_listener buffer_create_sync_listener = {
> + buffer_create_sync_callback
> +};
> +
> +static void
> +lazy_create_buffer_roundtrip(struct dri2_egl_display *dri2_dpy,
> + struct dri2_egl_surface *dri2_surf)
> +{
> + struct wl_callback *callback;
> +
> + callback = wl_display_sync(dri2_dpy->wl_dpy);
> + dri2_surf->current->created_callback = callback;
> +
> + wl_callback_add_listener(callback,
> + &buffer_create_sync_listener,
> + &dri2_surf->current->created_callback);
> + wl_proxy_set_queue((struct wl_proxy *) callback, dri2_dpy->wl_queue);
> +}
> +
> +static int
> +synchronize_create_buffer(struct dri2_egl_display *dri2_dpy,
> + struct dri2_egl_surface *dri2_surf,
> + int i)
> +{
> + int ret = 0;
> +
> + while (ret != -1 && dri2_surf->color_buffers[i].created_callback)
> + ret = wl_display_dispatch_queue(dri2_dpy->wl_dpy, dri2_dpy->wl_queue);
> +
> + if (dri2_surf->color_buffers[i].created_callback) {
> + wl_callback_destroy(dri2_surf->color_buffers[i].created_callback);
> + dri2_surf->color_buffers[i].created_callback = NULL;
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> /**
> * Called via eglDestroySurface(), drv->API.DestroySurface().
> */
> @@ -206,6 +255,8 @@ dri2_destroy_surface(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *surf)
> wl_buffer_destroy(dri2_surf->color_buffers[i].wl_buffer);
> if (dri2_surf->color_buffers[i].dri_image)
> dri2_dpy->image->destroyImage(dri2_surf->color_buffers[i].dri_image);
> + if (dri2_surf->color_buffers[i].created_callback)
> + wl_callback_destroy(dri2_surf->color_buffers[i].created_callback);
> }
>
> for (i = 0; i < __DRI_BUFFER_COUNT; i++)
> @@ -227,7 +278,7 @@ dri2_destroy_surface(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *surf)
> return EGL_TRUE;
> }
>
> -static void
> +static int
> dri2_release_buffers(struct dri2_egl_surface *dri2_surf)
> {
> struct dri2_egl_display *dri2_dpy =
> @@ -235,6 +286,11 @@ dri2_release_buffers(struct dri2_egl_surface *dri2_surf)
> int i;
>
> for (i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) {
> + if (dri2_surf->color_buffers[i].created_callback) {
> + if (synchronize_create_buffer(dri2_dpy, dri2_surf, i) != 0)
> + return -1;
> + }
> +
> if (dri2_surf->color_buffers[i].wl_buffer &&
> !dri2_surf->color_buffers[i].locked)
> wl_buffer_destroy(dri2_surf->color_buffers[i].wl_buffer);
> @@ -251,6 +307,8 @@ dri2_release_buffers(struct dri2_egl_surface *dri2_surf)
> dri2_surf->dri_buffers[i]->attachment != __DRI_BUFFER_BACK_LEFT)
> dri2_dpy->dri2->releaseBuffer(dri2_dpy->dri_screen,
> dri2_surf->dri_buffers[i]);
> +
> + return 0;
> }
>
> static int
> @@ -349,7 +407,10 @@ dri2_get_buffers_with_format(__DRIdrawable * driDrawable,
> (dri2_surf->base.Width != dri2_surf->wl_win->width ||
> dri2_surf->base.Height != dri2_surf->wl_win->height)) {
>
> - dri2_release_buffers(dri2_surf);
> + if (dri2_release_buffers(dri2_surf) != 0) {
> + _eglError(EGL_BAD_DISPLAY, "failed to release color buffers");
> + return NULL;
> + }
>
> dri2_surf->base.Width = dri2_surf->wl_win->width;
> dri2_surf->base.Height = dri2_surf->wl_win->height;
> @@ -381,6 +442,13 @@ dri2_get_buffers_with_format(__DRIdrawable * driDrawable,
> for (i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) {
> if (!dri2_surf->color_buffers[i].locked &&
> dri2_surf->color_buffers[i].wl_buffer) {
> + if (dri2_surf->color_buffers[i].created_callback) {
> + if (synchronize_create_buffer(dri2_dpy, dri2_surf, i) != 0) {
> + _eglError(EGL_BAD_DISPLAY, "failed to release color buffer");
> + return NULL;
> + }
> + }
> +
> wl_buffer_destroy(dri2_surf->color_buffers[i].wl_buffer);
> dri2_dpy->image->destroyImage(dri2_surf->color_buffers[i].dri_image);
> dri2_surf->color_buffers[i].wl_buffer = NULL;
> @@ -483,6 +551,8 @@ create_wl_buffer(struct dri2_egl_surface *dri2_surf)
> dri2_surf->base.Height,
> dri2_surf->current->pitch,
> dri2_surf->format);
> +
> + lazy_create_buffer_roundtrip(dri2_dpy, dri2_surf);
> }
>
> wl_proxy_set_queue((struct wl_proxy *) dri2_surf->current->wl_buffer,
> --
> 1.8.1.2
>
More information about the wayland-devel
mailing list