SF could get stuck waiting for vsync when turning the screen off

When turning the screen off we could have 2 waiters on the
vsync condition: The main vsync waiter as well as one in
onScreenReleased(). We were only signaling the condition though,
so it it would be possible to wake onScreenReleased() without waking
the main vsync thread which would then be stuck in .wait().

We fix this by just using broadcast() when receiving a vsync event.

We also add a broadcast() to signal when the state of
mUseSoftwareVSync changes.  This is important particularly for
the transition from hardware to software vsync because the main
vsync waiter might have observed mUseSoftwareVSync == false
and decided to block indefinitely pending a hardware vsync
signal that will never arrive.

Removed a potentially deadlocking wait for a signal in
onScreenReleased().  The function was trying to wait for the last
vsync event from the hardware to be delivered to clients but there
was no guarantee that another thread would signal it to wake up
again afterwards.  (As far as I can tell, the only other other
thread that might wake it up at this point would be a client
application issuing a vsync request.)  We don't really need to wait
here anyhow.  It's enough to set the mUseSoftwareVSync flag,
wake up the thread loop and go.  If there was a pending vsync
timestamp from the hardware, then the thread loop will grab
it and use it then start software vsync on the next iteration.

Bug: 6672102
Change-Id: I7c6abc23bb021d1dfc94f101bd3ce18e3a81a73e

diff --git a/services/surfaceflinger/EventThread.cpp b/services/surfaceflinger/EventThread.cpp
index 47fa2f3..7c1aebe 100644 (file)
--- a/services/surfaceflinger/EventThread.cpp
+++ b/services/surfaceflinger/EventThread.cpp
@@ -60,7 +60,7 @@ status_t EventThread::registerDisplayEventConnection(
         const sp<EventThread::Connection>& connection) {
     Mutex::Autolock _l(mLock);
     mDisplayEventConnections.add(connection);
-    mCondition.signal();
+    mCondition.broadcast();
     return NO_ERROR;
 }
 
@@ -68,7 +68,7 @@ status_t EventThread::unregisterDisplayEventConnection(
         const wp<EventThread::Connection>& connection) {
     Mutex::Autolock _l(mLock);
     mDisplayEventConnections.remove(connection);
-    mCondition.signal();
+    mCondition.broadcast();
     return NO_ERROR;
 }
 
@@ -85,7 +85,7 @@ void EventThread::setVsyncRate(uint32_t count,
         const int32_t new_count = (count == 0) ? -1 : count;
         if (connection->count != new_count) {
             connection->count = new_count;
-            mCondition.signal();
+            mCondition.broadcast();
         }
     }
 }
@@ -95,32 +95,33 @@ void EventThread::requestNextVsync(
     Mutex::Autolock _l(mLock);
     if (connection->count < 0) {
         connection->count = 0;
-        mCondition.signal();
+        mCondition.broadcast();
     }
 }
 
 void EventThread::onScreenReleased() {
     Mutex::Autolock _l(mLock);
-    // wait for an eventual pending vsync to be serviced
     if (!mUseSoftwareVSync) {
-        while (mVSyncTimestamp) {
-            mCondition.wait(mLock);
-        }
+        // disable reliance on h/w vsync
+        mUseSoftwareVSync = true;
+        mCondition.broadcast();
     }
-    // disable reliance on h/w vsync
-    mUseSoftwareVSync = true;
 }
 
 void EventThread::onScreenAcquired() {
     Mutex::Autolock _l(mLock);
-    mUseSoftwareVSync = false;
+    if (mUseSoftwareVSync) {
+        // resume use of h/w vsync
+        mUseSoftwareVSync = false;
+        mCondition.broadcast();
+    }
 }
 
 
 void EventThread::onVSyncReceived(int, nsecs_t timestamp) {
     Mutex::Autolock _l(mLock);
     mVSyncTimestamp = timestamp;
-    mCondition.signal();
+    mCondition.broadcast();
 }
 
 bool EventThread::threadLoop() {