fix a dead-lock in eglMakeCurrent

this was introduced in a recent change. eglMakeCurrent can
end up calling eglDestroyImageKHR via ANativewWindow::disconnect
when the consumer is in the same process.

we make sure we don't hold the lock while this is happening.

Change-Id: Id17fe4fd76eecf5f962cefb9aa32be41fc1b042d

diff --git a/opengl/libs/EGL/egl_display.cpp b/opengl/libs/EGL/egl_display.cpp
index 5cf5236..6b2ae51 100644 (file)
--- a/opengl/libs/EGL/egl_display.cpp
+++ b/opengl/libs/EGL/egl_display.cpp
@@ -345,42 +345,73 @@ EGLBoolean egl_display_t::terminate() {
 void egl_display_t::loseCurrent(egl_context_t * cur_c)
 {
     if (cur_c) {
-        egl_surface_t * cur_r = get_surface(cur_c->read);
-        egl_surface_t * cur_d = get_surface(cur_c->draw);
-
-        // by construction, these are either 0 or valid (possibly terminated)
-        // it should be impossible for these to be invalid
-        ContextRef _cur_c(cur_c);
-        SurfaceRef _cur_r(cur_r);
-        SurfaceRef _cur_d(cur_d);
+        egl_display_t* display = cur_c->getDisplay();
+        if (display) {
+            display->loseCurrentImpl(cur_c);
+        }
+    }
+}
 
+void egl_display_t::loseCurrentImpl(egl_context_t * cur_c)
+{
+    // by construction, these are either 0 or valid (possibly terminated)
+    // it should be impossible for these to be invalid
+    ContextRef _cur_c(cur_c);
+    SurfaceRef _cur_r(cur_c ? get_surface(cur_c->read) : NULL);
+    SurfaceRef _cur_d(cur_c ? get_surface(cur_c->draw) : NULL);
+
+    { // scope for the lock
+        Mutex::Autolock _l(lock);
         cur_c->onLooseCurrent();
 
-        _cur_c.release();
-        _cur_r.release();
-        _cur_d.release();
     }
+
+    // This cannot be called with the lock held because it might end-up
+    // calling back into EGL (in particular when a surface is destroyed
+    // it calls ANativeWindow::disconnect
+    _cur_c.release();
+    _cur_r.release();
+    _cur_d.release();
 }
 
 EGLBoolean egl_display_t::makeCurrent(egl_context_t* c, egl_context_t* cur_c,
         EGLSurface draw, EGLSurface read, EGLContext ctx,
         EGLSurface impl_draw, EGLSurface impl_read, EGLContext impl_ctx)
 {
-    Mutex::Autolock _l(lock);
     EGLBoolean result;
-    if (c) {
-        result = c->cnx->egl.eglMakeCurrent(
-                disp[c->impl].dpy, impl_draw, impl_read, impl_ctx);
-    } else {
-        result = cur_c->cnx->egl.eglMakeCurrent(
-                disp[cur_c->impl].dpy, impl_draw, impl_read, impl_ctx);
-    }
-    if (result == EGL_TRUE) {
-        loseCurrent(cur_c);
+
+    // by construction, these are either 0 or valid (possibly terminated)
+    // it should be impossible for these to be invalid
+    ContextRef _cur_c(cur_c);
+    SurfaceRef _cur_r(cur_c ? get_surface(cur_c->read) : NULL);
+    SurfaceRef _cur_d(cur_c ? get_surface(cur_c->draw) : NULL);
+
+    { // scope for the lock
+        Mutex::Autolock _l(lock);
         if (c) {
-            c->onMakeCurrent(draw, read);
+            result = c->cnx->egl.eglMakeCurrent(
+                    disp[c->impl].dpy, impl_draw, impl_read, impl_ctx);
+            if (result == EGL_TRUE) {
+                c->onMakeCurrent(draw, read);
+            }
+        } else {
+            result = cur_c->cnx->egl.eglMakeCurrent(
+                    disp[cur_c->impl].dpy, impl_draw, impl_read, impl_ctx);
+            if (result == EGL_TRUE) {
+                cur_c->onLooseCurrent();
+            }
         }
     }
+
+    if (result == EGL_TRUE) {
+        // This cannot be called with the lock held because it might end-up
+        // calling back into EGL (in particular when a surface is destroyed
+        // it calls ANativeWindow::disconnect
+        _cur_c.release();
+        _cur_r.release();
+        _cur_d.release();
+    }
+
     return result;
 }
 

diff --git a/opengl/libs/EGL/egl_display.h b/opengl/libs/EGL/egl_display.h
index 4479e00..f3c4ddf 100644 (file)
--- a/opengl/libs/EGL/egl_display.h
+++ b/opengl/libs/EGL/egl_display.h
@@ -64,6 +64,7 @@ struct egl_config_t {
 class EGLAPI egl_display_t { // marked as EGLAPI for testing purposes
     static egl_display_t sDisplay[NUM_DISPLAYS];
     EGLDisplay getDisplay(EGLNativeDisplayType display);
+    void loseCurrentImpl(egl_context_t * cur_c);
 
 public:
     enum {