Merge "Fix deadlock in SF." into jb-mr1-dev

services/surfaceflinger/EventThread.cpp

@@ -36,10 +36,9 @@ namespace android {
 
 EventThread::EventThread(const sp<SurfaceFlinger>& flinger)
     : mFlinger(flinger),
-      mLastVSyncTimestamp(0),
       mVSyncTimestamp(0),
       mUseSoftwareVSync(false),
-      mDeliveredEvents(0),
+      mVSyncCount(0),
       mDebugVsyncEnabled(false) {
 }
 
@@ -116,133 +115,122 @@ void EventThread::onScreenAcquired() {
 void EventThread::onVSyncReceived(int, nsecs_t timestamp) {
     Mutex::Autolock _l(mLock);
     mVSyncTimestamp = timestamp;
+    mVSyncCount++;
     mCondition.broadcast();
 }
 
 bool EventThread::threadLoop() {
 
     nsecs_t timestamp;
+    size_t vsyncCount;
+    size_t activeEvents;
     DisplayEventReceiver::Event vsync;
-    Vector< wp<EventThread::Connection> > displayEventConnections;
+    Vector< sp<EventThread::Connection> > activeConnections;
 
     do {
         Mutex::Autolock _l(mLock);
-        do {
+        // latch VSYNC event if any
-            // latch VSYNC event if any
+        timestamp = mVSyncTimestamp;
-            timestamp = mVSyncTimestamp;
+        mVSyncTimestamp = 0;
-            mVSyncTimestamp = 0;
 
-            // check if we should be waiting for VSYNC events
+        // check if we should be waiting for VSYNC events
-            bool waitForNextVsync = false;
+        activeEvents = 0;
-            size_t count = mDisplayEventConnections.size();
+        bool waitForNextVsync = false;
-            for (size_t i=0 ; i<count ; i++) {
+        size_t count = mDisplayEventConnections.size();
-                sp<Connection> connection =
+        for (size_t i=0 ; i<count ; i++) {
-                        mDisplayEventConnections.itemAt(i).promote();
+            sp<Connection> connection(mDisplayEventConnections[i].promote());
-                if (connection!=0 && connection->count >= 0) {
+            if (connection != NULL) {
+                activeConnections.add(connection);
+                if (connection->count >= 0) {
                     // at least one continuous mode or active one-shot event
                     waitForNextVsync = true;
+                    activeEvents++;
                     break;
                 }
             }
-
-            if (timestamp) {
-                if (!waitForNextVsync) {
-                    // we received a VSYNC but we have no clients
-                    // don't report it, and disable VSYNC events
-                    disableVSyncLocked();
-                } else {
-                    // report VSYNC event
-                    break;
-                }
-            } else {
-                // never disable VSYNC events immediately, instead
-                // we'll wait to receive the event and we'll
-                // reevaluate whether we need to dispatch it and/or
-                // disable VSYNC events then.
-                if (waitForNextVsync) {
-                    // enable
-                    enableVSyncLocked();
-                }
-            }
-
-            // wait for something to happen
-            if (mUseSoftwareVSync && waitForNextVsync) {
-                // h/w vsync cannot be used (screen is off), so we use
-                // a  timeout instead. it doesn't matter how imprecise this
-                // is, we just need to make sure to serve the clients
-                if (mCondition.waitRelative(mLock, ms2ns(16)) == TIMED_OUT) {
-                    mVSyncTimestamp = systemTime(SYSTEM_TIME_MONOTONIC);
-                }
-            } else {
-                mCondition.wait(mLock);
-            }
-        } while(true);
-
-        // process vsync event
-        mDeliveredEvents++;
-        mLastVSyncTimestamp = timestamp;
-
-        // now see if we still need to report this VSYNC event
-        const size_t count = mDisplayEventConnections.size();
-        for (size_t i=0 ; i<count ; i++) {
-            bool reportVsync = false;
-            sp<Connection> connection =
-                    mDisplayEventConnections.itemAt(i).promote();
-            if (connection == 0)
-                continue;
-
-            const int32_t count = connection->count;
-            if (count >= 1) {
-                if (count==1 || (mDeliveredEvents % count) == 0) {
-                    // continuous event, and time to report it
-                    reportVsync = true;
-                }
-            } else if (count >= -1) {
-                if (count == 0) {
-                    // fired this time around
-                    reportVsync = true;
-                }
-                connection->count--;
-            }
-            if (reportVsync) {
-                displayEventConnections.add(connection);
-            }
         }
-    } while (!displayEventConnections.size());
+
+        if (timestamp) {
+            if (!waitForNextVsync) {
+                // we received a VSYNC but we have no clients
+                // don't report it, and disable VSYNC events
+                disableVSyncLocked();
+            } else {
+                // report VSYNC event
+                break;
+            }
+        } else {
+            // never disable VSYNC events immediately, instead
+            // we'll wait to receive the event and we'll
+            // reevaluate whether we need to dispatch it and/or
+            // disable VSYNC events then.
+            if (waitForNextVsync) {
+                // enable
+                enableVSyncLocked();
+            }
+        }
+
+        // wait for something to happen
+        if (mUseSoftwareVSync && waitForNextVsync) {
+            // h/w vsync cannot be used (screen is off), so we use
+            // a  timeout instead. it doesn't matter how imprecise this
+            // is, we just need to make sure to serve the clients
+            if (mCondition.waitRelative(mLock, ms2ns(16)) == TIMED_OUT) {
+                mVSyncTimestamp = systemTime(SYSTEM_TIME_MONOTONIC);
+                mVSyncCount++;
+            }
+        } else {
+            mCondition.wait(mLock);
+        }
+        vsyncCount = mVSyncCount;
+    } while (!activeConnections.size());
+
+    if (!activeEvents) {
+        // no events to return. start over.
+        // (here we make sure to exit the scope of this function
+        //  so that we release our Connection references)
+        return true;
+    }
 
     // dispatch vsync events to listeners...
     vsync.header.type = DisplayEventReceiver::DISPLAY_EVENT_VSYNC;
     vsync.header.timestamp = timestamp;
-    vsync.vsync.count = mDeliveredEvents;
+    vsync.vsync.count = vsyncCount;
 
-    const size_t count = displayEventConnections.size();
+    const size_t count = activeConnections.size();
     for (size_t i=0 ; i<count ; i++) {
-        sp<Connection> conn(displayEventConnections[i].promote());
+        const sp<Connection>& conn(activeConnections[i]);
-        // make sure the connection didn't die
+        // now see if we still need to report this VSYNC event
-        if (conn != NULL) {
+        const int32_t vcount = conn->count;
-            status_t err = conn->postEvent(vsync);
+        if (vcount >= 0) {
-            if (err == -EAGAIN || err == -EWOULDBLOCK) {
+            bool reportVsync = false;
-                // The destination doesn't accept events anymore, it's probably
+            if (vcount == 0) {
-                // full. For now, we just drop the events on the floor.
+                // fired this time around
-                // Note that some events cannot be dropped and would have to be
+                conn->count = -1;
-                // re-sent later. Right-now we don't have the ability to do
+                reportVsync = true;
-                // this, but it doesn't matter for VSYNC.
+            } else if (vcount == 1 || (vsyncCount % vcount) == 0) {
-            } else if (err < 0) {
+                // continuous event, and time to report it
-                // handle any other error on the pipe as fatal. the only
+                reportVsync = true;
-                // reasonable thing to do is to clean-up this connection.
+            }
-                // The most common error we'll get here is -EPIPE.
+
-                removeDisplayEventConnection(displayEventConnections[i]);
+            if (reportVsync) {
+                status_t err = conn->postEvent(vsync);
+                if (err == -EAGAIN || err == -EWOULDBLOCK) {
+                    // The destination doesn't accept events anymore, it's probably
+                    // full. For now, we just drop the events on the floor.
+                    // Note that some events cannot be dropped and would have to be
+                    // re-sent later. Right-now we don't have the ability to do
+                    // this, but it doesn't matter for VSYNC.
+                } else if (err < 0) {
+                    // handle any other error on the pipe as fatal. the only
+                    // reasonable thing to do is to clean-up this connection.
+                    // The most common error we'll get here is -EPIPE.
+                    removeDisplayEventConnection(activeConnections[i]);
+                }
             }
-        } else {
-            // somehow the connection is dead, but we still have it in our list
-            // just clean the list.
-            removeDisplayEventConnection(displayEventConnections[i]);
         }
     }
 
-    // clear all our references without holding mLock
-    displayEventConnections.clear();
-
     return true;
 }
 
@@ -273,7 +261,7 @@ void EventThread::dump(String8& result, char* buffer, size_t SIZE) const {
     result.appendFormat("  soft-vsync: %s\n",
             mUseSoftwareVSync?"enabled":"disabled");
     result.appendFormat("  numListeners=%u,\n  events-delivered: %u\n",
-            mDisplayEventConnections.size(), mDeliveredEvents);
+            mDisplayEventConnections.size(), mVSyncCount);
     for (size_t i=0 ; i<mDisplayEventConnections.size() ; i++) {
         sp<Connection> connection =
                 mDisplayEventConnections.itemAt(i).promote();

services/surfaceflinger/EventThread.h

@@ -100,12 +100,9 @@ private:
 
     // protected by mLock
     SortedVector< wp<Connection> > mDisplayEventConnections;
-    nsecs_t mLastVSyncTimestamp;
     nsecs_t mVSyncTimestamp;
     bool mUseSoftwareVSync;
-
+    size_t mVSyncCount;
-    // main thread only
-    size_t mDeliveredEvents;
 
     // for debugging
     bool mDebugVsyncEnabled;