Fix deadlock when cleaning objects in eglTerminate

When eglTerminate() is called with a window surface still exists, a
deadlock would occur since egl_display_t::terminate() holds a lock
while destroying the window surface, which calls
onWindowSurfaceDestroyed() which attempts to take the same lock.

This change refactors the hibernation code and data into a separate
object with its own lock, separate from the egl_display_t lock. This
avoids the deadlock and better encapsulates the hibernation logic.

The change also fixes a bug discovered incidentally while debugging:
hibernating after calling eglTerminate() succeeds, but will cause
awakens from subsequent eglInitialize() to fail. We will no longer
hibernate a terminated display.

Change-Id: If55e5bb603d4f8953babc439ffc8d8a60af103d9

2 files changed, 79 insertions, 29 deletions

diff --git a/opengl/libs/EGL/egl_display.cpp b/opengl/libs/EGL/egl_display.cpp
index 2e08f24..f2e3897 100644
--- a/opengl/libs/EGL/egl_display.cpp
+++ b/opengl/libs/EGL/egl_display.cpp
@@ -70,8 +70,7 @@ extern void setGLHooksThreadSpecific(gl_hooks_t const *value);
 egl_display_t egl_display_t::sDisplay[NUM_DISPLAYS];
 
 egl_display_t::egl_display_t() :
-    magic('_dpy'), finishOnSwap(false), traceGpuCompletion(false), refs(0),
-    mWakeCount(0), mHibernating(false), mAttemptHibernation(false) {
+    magic('_dpy'), finishOnSwap(false), traceGpuCompletion(false), refs(0) {
 }
 
 egl_display_t::~egl_display_t() {
@@ -253,6 +252,9 @@ EGLBoolean egl_display_t::initialize(EGLint *major, EGLint *minor) {
         *major = VERSION_MAJOR;
     if (minor != NULL)
         *minor = VERSION_MINOR;
+
+    mHibernation.setDisplayValid(true);
+
     return EGL_TRUE;
 }
 
@@ -282,6 +284,8 @@ EGLBoolean egl_display_t::terminate() {
         res = EGL_TRUE;
     }
 
+    mHibernation.setDisplayValid(false);
+
     // Mark all objects remaining in the list as terminated, unless
     // there are no reference to them, it which case, we're free to
     // delete them.
@@ -351,8 +355,7 @@ EGLBoolean egl_display_t::makeCurrent(egl_context_t* c, egl_context_t* cur_c,
             if (result == EGL_TRUE) {
                 c->onMakeCurrent(draw, read);
                 if (!cur_c) {
-                    mWakeCount++;
-                    mAttemptHibernation = false;
+                    mHibernation.incWakeCount(HibernationMachine::STRONG);
                 }
             }
         } else {
@@ -360,8 +363,7 @@ EGLBoolean egl_display_t::makeCurrent(egl_context_t* c, egl_context_t* cur_c,
                     disp.dpy, impl_draw, impl_read, impl_ctx);
             if (result == EGL_TRUE) {
                 cur_c->onLooseCurrent();
-                mWakeCount--;
-                mAttemptHibernation = true;
+                mHibernation.decWakeCount(HibernationMachine::STRONG);
             }
         }
     }
@@ -378,14 +380,26 @@ EGLBoolean egl_display_t::makeCurrent(egl_context_t* c, egl_context_t* cur_c,
     return result;
 }
 
-bool egl_display_t::enter() {
-    Mutex::Autolock _l(lock);
+// ----------------------------------------------------------------------------
+
+bool egl_display_t::HibernationMachine::incWakeCount(WakeRefStrength strength) {
+    Mutex::Autolock _l(mLock);
     ALOGE_IF(mWakeCount < 0 || mWakeCount == INT32_MAX,
              "Invalid WakeCount (%d) on enter\n", mWakeCount);
+
     mWakeCount++;
+    if (strength == STRONG)
+        mAttemptHibernation = false;
+
     if (CC_UNLIKELY(mHibernating)) {
         ALOGV("Awakening\n");
         egl_connection_t* const cnx = &gEGLImpl;
+
+        // These conditions should be guaranteed before entering hibernation;
+        // we don't want to get into a state where we can't wake up.
+        ALOGD_IF(!mDpyValid || !cnx->egl.eglAwakenProcessIMG,
+                 "Invalid hibernation state, unable to awaken\n");
+
         if (!cnx->egl.eglAwakenProcessIMG()) {
             ALOGE("Failed to awaken EGL implementation\n");
             return false;
@@ -395,13 +409,20 @@ bool egl_display_t::enter() {
     return true;
 }
 
-void egl_display_t::leave() {
-    Mutex::Autolock _l(lock);
+void egl_display_t::HibernationMachine::decWakeCount(WakeRefStrength strength) {
+    Mutex::Autolock _l(mLock);
     ALOGE_IF(mWakeCount <= 0, "Invalid WakeCount (%d) on leave\n", mWakeCount);
-    if (--mWakeCount == 0 && CC_UNLIKELY(mAttemptHibernation)) {
+
+    mWakeCount--;
+    if (strength == STRONG)
+        mAttemptHibernation = true;
+
+    if (mWakeCount == 0 && CC_UNLIKELY(mAttemptHibernation)) {
         egl_connection_t* const cnx = &gEGLImpl;
         mAttemptHibernation = false;
-        if (cnx->egl.eglHibernateProcessIMG && cnx->egl.eglAwakenProcessIMG) {
+        if (mDpyValid &&
+                cnx->egl.eglHibernateProcessIMG &&
+                cnx->egl.eglAwakenProcessIMG) {
             ALOGV("Hibernating\n");
             if (!cnx->egl.eglHibernateProcessIMG()) {
                 ALOGE("Failed to hibernate EGL implementation\n");
@@ -412,16 +433,9 @@ void egl_display_t::leave() {
     }
 }
 
-void egl_display_t::onWindowSurfaceCreated() {
-    Mutex::Autolock _l(lock);
-    mWakeCount++;
-    mAttemptHibernation = false;
-}
-
-void egl_display_t::onWindowSurfaceDestroyed() {
-    Mutex::Autolock _l(lock);
-    mWakeCount--;
-    mAttemptHibernation = true;
+void egl_display_t::HibernationMachine::setDisplayValid(bool valid) {
+    Mutex::Autolock _l(mLock);
+    mDpyValid = valid;
 }
 
 // ----------------------------------------------------------------------------
diff --git a/opengl/libs/EGL/egl_display.h b/opengl/libs/EGL/egl_display.h
index 28607da..412568b 100644
--- a/opengl/libs/EGL/egl_display.h
+++ b/opengl/libs/EGL/egl_display.h
@@ -77,8 +77,12 @@ public:
     // proper hibernate/wakeup sequencing. If a surface destruction triggers
     // hibernation, hibernation will be delayed at least until the calling
     // thread's egl_display_ptr is destroyed.
-    void onWindowSurfaceCreated();
-    void onWindowSurfaceDestroyed();
+    void onWindowSurfaceCreated() {
+        mHibernation.incWakeCount(HibernationMachine::STRONG);
+    }
+    void onWindowSurfaceDestroyed() {
+        mHibernation.decWakeCount(HibernationMachine::STRONG);
+    }
 
     static egl_display_t* get(EGLDisplay dpy);
     static EGLDisplay getFromNativeDisplay(EGLNativeDisplayType disp);
@@ -123,8 +127,8 @@ public:
 
 private:
     friend class egl_display_ptr;
-    bool enter();
-    void leave();
+    bool enter() { return mHibernation.incWakeCount(HibernationMachine::WEAK); }
+    void leave() { return mHibernation.decWakeCount(HibernationMachine::WEAK); }
 
             uint32_t                    refs;
     mutable Mutex                       lock;
@@ -133,9 +137,41 @@ private:
             String8 mVersionString;
             String8 mClientApiString;
             String8 mExtensionString;
-            int32_t mWakeCount;
-            bool    mHibernating;
-            bool    mAttemptHibernation;
+
+    // HibernationMachine uses its own internal mutex to protect its own data.
+    // The owning egl_display_t's lock may be but is not required to be held
+    // when calling HibernationMachine methods. As a result, nothing in this
+    // class may call back up to egl_display_t directly or indirectly.
+    class HibernationMachine {
+    public:
+        // STRONG refs cancel (inc) or initiate (dec) a hibernation attempt
+        // the next time the wakecount reaches zero. WEAK refs don't affect
+        // whether a hibernation attempt will be made. Use STRONG refs only
+        // for infrequent/heavy changes that are likely to indicate the
+        // EGLDisplay is entering or leaving a long-term idle state.
+        enum WakeRefStrength {
+            WEAK   = 0,
+            STRONG = 1,
+        };
+
+        HibernationMachine(): mWakeCount(0), mHibernating(false),
+                mAttemptHibernation(false), mDpyValid(false)
+        {}
+        ~HibernationMachine() {}
+
+        bool incWakeCount(WakeRefStrength strenth);
+        void decWakeCount(WakeRefStrength strenth);
+
+        void setDisplayValid(bool valid);
+
+    private:
+        Mutex   mLock;
+        int32_t mWakeCount;
+        bool    mHibernating;
+        bool    mAttemptHibernation;
+        bool    mDpyValid;
+    };
+    HibernationMachine mHibernation;
 };
 
 // ----------------------------------------------------------------------------