On Mon, Oct 07, 2013 at 10:09:17PM +0200, Giulio Camuffo wrote:
> Suppose we create a wl_egl_window and an EGLSurface. Then we call
> eglMakeCurrent(dpy,surf,surf,ctx) with that surface, render and swap.
> Later we destroy the surface and the window, and we make current
> another surface. That resulted in two invalid writes because
> the surface, which is refcounted, gets actually deleted at the
> eglMakeCurrent() call, and in dri2_destroy_surface() its window
> was used, window which was deleted earlier.
> So make also wl_egl_window refcounted, so that it isn't destroyed
> while its surface is still alive.
Just for reference, we discussed this in IRC and it came down to a
misunderstanding of how EGL works. An EGLSurface is current for a
context from eglMakeCurrent up until eglMakeCurrent is called for the
conetxte with another surface or the context is destroyed. The
wl_egl_surface (the native window object) has to be available (can not
be destroyed) for the entire time the EGLSurface exists.
Kristian
> Cc: mesa-sta...@lists.freedesktop.org
> ---
> src/egl/drivers/dri2/platform_wayland.c | 3 +++
> src/egl/wayland/wayland-egl/wayland-egl-priv.h | 2 ++
> src/egl/wayland/wayland-egl/wayland-egl.c | 5 ++++-
> 3 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/src/egl/drivers/dri2/platform_wayland.c
> b/src/egl/drivers/dri2/platform_wayland.c
> index 1d417bb..831cd23 100644
> --- a/src/egl/drivers/dri2/platform_wayland.c
> +++ b/src/egl/drivers/dri2/platform_wayland.c
> @@ -143,6 +143,7 @@ dri2_create_surface(_EGLDriver *drv, _EGLDisplay *disp,
> EGLint type,
>
> dri2_surf->wl_win->private = dri2_surf;
> dri2_surf->wl_win->resize_callback = resize_callback;
> + ++dri2_surf->wl_win->ref_count;
>
> dri2_surf->base.Width = -1;
> dri2_surf->base.Height = -1;
> @@ -222,6 +223,8 @@ dri2_destroy_surface(_EGLDriver *drv, _EGLDisplay *disp,
> _EGLSurface *surf)
> dri2_surf->wl_win->resize_callback = NULL;
> }
>
> + (dri2_surf->wl_win->destroy)(dri2_surf->wl_win);
> +
> free(surf);
>
> return EGL_TRUE;
> diff --git a/src/egl/wayland/wayland-egl/wayland-egl-priv.h
> b/src/egl/wayland/wayland-egl/wayland-egl-priv.h
> index da25be9..302e2cf 100644
> --- a/src/egl/wayland/wayland-egl/wayland-egl-priv.h
> +++ b/src/egl/wayland/wayland-egl/wayland-egl-priv.h
> @@ -25,8 +25,10 @@ struct wl_egl_window {
> int attached_width;
> int attached_height;
>
> + int ref_count;
> void *private;
> void (*resize_callback)(struct wl_egl_window *, void *);
> + void (*destroy)(struct wl_egl_window *);
> };
>
> #ifdef __cplusplus
> diff --git a/src/egl/wayland/wayland-egl/wayland-egl.c
> b/src/egl/wayland/wayland-egl/wayland-egl.c
> index 8bd49cf..7a93987 100644
> --- a/src/egl/wayland/wayland-egl/wayland-egl.c
> +++ b/src/egl/wayland/wayland-egl/wayland-egl.c
> @@ -34,6 +34,8 @@ wl_egl_window_create(struct wl_surface *surface,
> wl_egl_window_resize(egl_window, width, height, 0, 0);
> egl_window->attached_width = 0;
> egl_window->attached_height = 0;
> + egl_window->ref_count = 1;
> + egl_window->destroy = wl_egl_window_destroy;
>
> return egl_window;
> }
> @@ -41,7 +43,8 @@ wl_egl_window_create(struct wl_surface *surface,
> WL_EGL_EXPORT void
> wl_egl_window_destroy(struct wl_egl_window *egl_window)
> {
> - free(egl_window);
> + if (--egl_window->ref_count < 1)
> + free(egl_window);
> }
>
> WL_EGL_EXPORT void
> --
> 1.8.4
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
