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
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 @@
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 @@
*major = VERSION_MAJOR;
if (minor != NULL)
*minor = VERSION_MINOR;
+
+ mHibernation.setDisplayValid(true);
+
return EGL_TRUE;
}
@@ -282,6 +284,8 @@
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 @@
if (result == EGL_TRUE) {
c->onMakeCurrent(draw, read);
if (!cur_c) {
- mWakeCount++;
- mAttemptHibernation = false;
+ mHibernation.incWakeCount(HibernationMachine::STRONG);
}
}
} else {
@@ -360,8 +363,7 @@
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 @@
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 @@
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::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;
}
// ----------------------------------------------------------------------------