fix a deadlock when removing a DisplayEventConnection
the deadlock would happen when the pipe became invalid and SF
trying to remove the connection from its list.
we know make sure to process events without holding a lock.
Change-Id: I39927ed8824fc7811e16db3c7608a2ebc72d9642
diff --git a/services/surfaceflinger/EventThread.cpp b/services/surfaceflinger/EventThread.cpp
index edb06ba..42477a9 100644
--- a/services/surfaceflinger/EventThread.cpp
+++ b/services/surfaceflinger/EventThread.cpp
@@ -60,36 +60,51 @@
return NO_ERROR;
}
+status_t EventThread::removeDisplayEventConnection(
+ const wp<DisplayEventConnection>& connection) {
+ Mutex::Autolock _l(mLock);
+ mDisplayEventConnections.remove(connection);
+ return NO_ERROR;
+}
+
bool EventThread::threadLoop() {
nsecs_t timestamp;
- Mutex::Autolock _l(mLock);
- do {
- // wait for listeners
- while (!mDisplayEventConnections.size()) {
- mCondition.wait(mLock);
- }
-
- // wait for vsync
- mLock.unlock();
- timestamp = mHw.waitForVSync();
- mLock.lock();
-
- // make sure we still have some listeners
- } while (!mDisplayEventConnections.size());
-
-
- // dispatch vsync events to listeners...
- mDeliveredEvents++;
- const size_t count = mDisplayEventConnections.size();
-
DisplayEventReceiver::Event vsync;
- vsync.header.type = DisplayEventReceiver::DISPLAY_EVENT_VSYNC;
- vsync.header.timestamp = timestamp;
- vsync.vsync.count = mDeliveredEvents;
+ SortedVector<wp<DisplayEventConnection> > displayEventConnections;
+ { // scope for the lock
+ Mutex::Autolock _l(mLock);
+ do {
+ // wait for listeners
+ while (!mDisplayEventConnections.size()) {
+ mCondition.wait(mLock);
+ }
+
+ // wait for vsync
+ mLock.unlock();
+ timestamp = mHw.waitForVSync();
+ mLock.lock();
+
+ // make sure we still have some listeners
+ } while (!mDisplayEventConnections.size());
+
+
+ // dispatch vsync events to listeners...
+ mDeliveredEvents++;
+
+ vsync.header.type = DisplayEventReceiver::DISPLAY_EVENT_VSYNC;
+ vsync.header.timestamp = timestamp;
+ vsync.vsync.count = mDeliveredEvents;
+
+ // make a copy of our connection list, so we can
+ // dispatch events without holding mLock
+ displayEventConnections = mDisplayEventConnections;
+ }
+
+ const size_t count = displayEventConnections.size();
for (size_t i=0 ; i<count ; i++) {
- sp<DisplayEventConnection> conn(mDisplayEventConnections.itemAt(i).promote());
+ sp<DisplayEventConnection> conn(displayEventConnections.itemAt(i).promote());
// make sure the connection didn't die
if (conn != NULL) {
status_t err = conn->postEvent(vsync);
@@ -103,11 +118,18 @@
// 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.
- mDisplayEventConnections.remove(conn);
+ removeDisplayEventConnection(displayEventConnections.itemAt(i));
}
+ } else {
+ // somehow the connection is dead, but we still have it in our list
+ // just clean the list.
+ removeDisplayEventConnection(displayEventConnections.itemAt(i));
}
}
+ // clear all our references without holding mLock
+ displayEventConnections.clear();
+
return true;
}