egl/dri2: try to bind old context if bindContext failed
authorLuigi Santivetti <luigi.santivetti@imgtec.com>
Tue, 30 Jun 2020 13:59:45 +0000 (14:59 +0100)
committerMarge Bot <eric+marge@anholt.net>
Tue, 21 Jul 2020 18:42:03 +0000 (18:42 +0000)
This change mostly touches error handling code paths, where a
bug was found when the DRI driver failed to bind a new DRI
context. Specifically, the reason for it to fail was the window
system unable (for whatever reason) to provide the DRI drawable
with a buffer. In this instance, Mesa un-does the EGL bindings,
but doesn't restore the old DRI context, hence remaining in a
funny state. It's worth mentioning that despite trying, there
is no guarantee that the old DRI context can be restored,
depending on the runtime.

Before this change, if bindContext() failed then
dri2_make_current() would rebind the old EGL context and
surfaces and return EGL_BAD_MATCH. However, it wouldn't rebind
the DRI context and surfaces, thus leaving it in an
inconsistent and unrecoverable state.

After this change, dri2_make_current() tries to bind the old
DRI context and surfaces when bindContext() failed. If unable
to do so, it leaves EGL and the DRI driver in a consistent
state, it reports an error and returns EGL_BAD_MATCH.

Fixes: 4e8f95f64d004aa1 ("egl_dri2: Always unbind old contexts")
Signed-off-by: Luigi Santivetti <luigi.santivetti@imgtec.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Acked-by: Eric Engestrom <eric@engestrom.ch>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/5707>

src/egl/drivers/dri2/egl_dri2.c

index f42ba6687f59c24150326fd623d3f87793e6c47c..299508b679cb274768d59dd4f69b71ea4c365bee 100644 (file)
@@ -1768,6 +1768,7 @@ dri2_make_current(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *dsurf,
    _EGLSurface *tmp_dsurf, *tmp_rsurf;
    __DRIdrawable *ddraw, *rdraw;
    __DRIcontext *cctx;
+   EGLint egl_error = EGL_SUCCESS;
 
    if (!dri2_dpy)
       return _eglError(EGL_NOT_INITIALIZED, "eglMakeCurrent");
@@ -1802,17 +1803,20 @@ dri2_make_current(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *dsurf,
 
    if (cctx || ddraw || rdraw) {
       if (!dri2_dpy->core->bindContext(cctx, ddraw, rdraw)) {
+         _EGLContext *tmp_ctx;
+
+         /* dri2_dpy->core->bindContext failed. We cannot tell for sure why, but
+          * setting the error to EGL_BAD_MATCH is surely better than leaving it
+          * as EGL_SUCCESS.
+          */
+         egl_error = EGL_BAD_MATCH;
+
          /* undo the previous _eglBindContext */
          _eglBindContext(old_ctx, old_dsurf, old_rsurf, &ctx, &tmp_dsurf, &tmp_rsurf);
          assert(&dri2_ctx->base == ctx &&
                 tmp_dsurf == dsurf &&
                 tmp_rsurf == rsurf);
 
-         if (old_dsurf && _eglSurfaceInSharedBufferMode(old_dsurf) &&
-             old_dri2_dpy->vtbl->set_shared_buffer_mode) {
-            old_dri2_dpy->vtbl->set_shared_buffer_mode(old_disp, old_dsurf, true);
-         }
-
          _eglPutSurface(dsurf);
          _eglPutSurface(rsurf);
          _eglPutContext(ctx);
@@ -1821,14 +1825,34 @@ dri2_make_current(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *dsurf,
          _eglPutSurface(old_rsurf);
          _eglPutContext(old_ctx);
 
-         /* dri2_dpy->core->bindContext failed. We cannot tell for sure why, but
-          * setting the error to EGL_BAD_MATCH is surely better than leaving it
-          * as EGL_SUCCESS.
+         ddraw = (old_dsurf) ? dri2_dpy->vtbl->get_dri_drawable(old_dsurf) : NULL;
+         rdraw = (old_rsurf) ? dri2_dpy->vtbl->get_dri_drawable(old_rsurf) : NULL;
+         cctx = (old_ctx) ? dri2_egl_context(old_ctx)->dri_context : NULL;
+
+         /* undo the previous dri2_dpy->core->unbindContext */
+         if (dri2_dpy->core->bindContext(cctx, ddraw, rdraw)) {
+            if (old_dsurf && _eglSurfaceInSharedBufferMode(old_dsurf) &&
+                old_dri2_dpy->vtbl->set_shared_buffer_mode) {
+               old_dri2_dpy->vtbl->set_shared_buffer_mode(old_disp, old_dsurf, true);
+            }
+
+            return _eglError(egl_error, "eglMakeCurrent");
+         }
+
+         /* We cannot restore the same state as it was before calling
+          * eglMakeCurrent() and the spec isn't clear about what to do. We
+          * can prevent EGL from calling into the DRI driver with no DRI
+          * context bound.
           */
-         return _eglError(EGL_BAD_MATCH, "eglMakeCurrent");
-      }
+         dsurf = rsurf = NULL;
+         ctx = NULL;
 
-      dri2_dpy->ref_count++;
+         _eglBindContext(ctx, dsurf, rsurf, &tmp_ctx, &tmp_dsurf, &tmp_rsurf);
+         assert(tmp_ctx == old_ctx && tmp_dsurf == old_dsurf &&
+                tmp_rsurf == old_rsurf);
+
+         _eglLog(_EGL_WARNING, "DRI2: failed to rebind the previous context");
+      }
    }
 
    dri2_destroy_surface(drv, disp, old_dsurf);
@@ -1839,6 +1863,11 @@ dri2_make_current(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *dsurf,
       dri2_display_release(old_disp);
    }
 
+   if (egl_error != EGL_SUCCESS)
+      return _eglError(egl_error, "eglMakeCurrent");
+
+   dri2_dpy->ref_count++;
+
    if (dsurf && _eglSurfaceHasMutableRenderBuffer(dsurf) &&
        dri2_dpy->vtbl->set_shared_buffer_mode) {
       /* Always update the shared buffer mode. This is obviously needed when