[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