[Mesa-dev] [PATCH] egl/dri2: dri2_make_current: Make sure display is initialized before using it

[Rhys Kidd]

Whilst I don't know that code as well as I'd like. It does get my

Tested-by: Rhys Kidd <rhyskidd at gmail.com>

for fixing that piglit on i965 ILK. Thanks for the two patches!

On 15 July 2016 at 04:28, Nicolas Boichat <drinkcat at chromium.org> wrote:

> android.opengl.cts.WrapperTest#testGetIntegerv1 CTS test calls
> eglTerminate, followed by eglReleaseThread. In that case, the
> display will not be initialized when eglReleaseThread calls
> MakeCurrent with NULL parameters, to unbind the context, which
> causes a a segfault in drv->API.MakeCurrent (dri2_make_current),
> either in glFlush or in a latter call.
>
> A similar case is observed in this bug:
> https://bugs.freedesktop.org/show_bug.cgi?id=69622, where the test
> call eglTerminate, then eglMakeCurrent(dpy, NULL, NULL, NULL).
>
> This patch:
>  1. Validates that the display is initialized, if ctx/dsurf/rsurf
>     are not all NULL.
>  2. Does not call glFlush/unBindContext is there is no display.
>  3. However, it still goes through the normal path as
>     drv->API.DestroyContext decrements the reference count on the
>     context, and frees the structure.
>
> Cc: "11.2 12.0" <mesa-stable at lists.freedesktop.org>
> Signed-off-by: Nicolas Boichat <drinkcat at chromium.org>
> ---
>
> I did not actually test the case reported on the bug, only the CTS
> test, would be nice if someone can try it out (Rhys? Chad?).
>
> Applies after "egl/dri2: dri2_make_current: Set EGL error if bindContext
> fails", but wanted to keep the 2 patches separate as they are different
> problems and discussions.
>
>  src/egl/drivers/dri2/egl_dri2.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/egl_dri2.c
> b/src/egl/drivers/dri2/egl_dri2.c
> index 2e97d85..3269e52 100644
> --- a/src/egl/drivers/dri2/egl_dri2.c
> +++ b/src/egl/drivers/dri2/egl_dri2.c
> @@ -1166,7 +1166,8 @@ dri2_destroy_context(_EGLDriver *drv, _EGLDisplay
> *disp, _EGLContext *ctx)
>     struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
>
>     if (_eglPutContext(ctx)) {
> -      dri2_dpy->core->destroyContext(dri2_ctx->dri_context);
> +      if (dri2_dpy)
> +         dri2_dpy->core->destroyContext(dri2_ctx->dri_context);
>        free(dri2_ctx);
>     }
>
> @@ -1189,6 +1190,10 @@ dri2_make_current(_EGLDriver *drv, _EGLDisplay
> *disp, _EGLSurface *dsurf,
>     __DRIdrawable *ddraw, *rdraw;
>     __DRIcontext *cctx;
>
> +   /* Check that the display is initialized */
> +   if ((ctx != NULL || dsurf != NULL || rsurf != NULL) && !dri2_dpy)
> +      return _eglError(EGL_NOT_INITIALIZED, "eglMakeCurrent error");
> +
>     /* make new bindings */
>     if (!_eglBindContext(ctx, dsurf, rsurf, &old_ctx, &old_dsurf,
> &old_rsurf)) {
>        /* _eglBindContext already sets the EGL error (in
> _eglCheckMakeCurrent) */
> @@ -1196,14 +1201,14 @@ dri2_make_current(_EGLDriver *drv, _EGLDisplay
> *disp, _EGLSurface *dsurf,
>     }
>
>     /* flush before context switch */
> -   if (old_ctx && dri2_drv->glFlush)
> +   if (old_ctx && dri2_dpy && dri2_drv->glFlush)
>        dri2_drv->glFlush();
>
>     ddraw = (dsurf) ? dri2_dpy->vtbl->get_dri_drawable(dsurf) : NULL;
>     rdraw = (rsurf) ? dri2_dpy->vtbl->get_dri_drawable(rsurf) : NULL;
>     cctx = (dri2_ctx) ? dri2_ctx->dri_context : NULL;
>
> -   if (old_ctx) {
> +   if (old_ctx && dri2_dpy) {
>        __DRIcontext *old_cctx = dri2_egl_context(old_ctx)->dri_context;
>        dri2_dpy->core->unbindContext(old_cctx);
>     }
> @@ -1292,6 +1297,8 @@ static EGLBoolean
>  dri2_destroy_surface(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surf)
>  {
>     struct dri2_egl_display *dri2_dpy = dri2_egl_display(dpy);
> +   if (!dri2_dpy)
> +      return EGL_TRUE;
>     return dri2_dpy->vtbl->destroy_surface(drv, dpy, surf);
>  }
>
> --
> 2.8.0.rc3.226.g39d4020
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160715/0f7f8807/attachment.html>