Fix potential deadlock in unregisterStaleHandlers()
The scenario is that a call to unregisterStaleHandlers() is in progress,
and is holding a temporary sp<ALooper> reference to an active ALooper inside
of the loop. At this point the only other remaining external reference to
the ALooper goes away, so the temporary sp<ALooper> in the loop is now
the only reference keeping that object alive. When the loop iterates and
the sp<> goes out of scope, the ALooper destructor is called, which in turn
calls unregisterStaleHandlers again, resulting in a recursive lock.
Bug: 17300093
Change-Id: I116f2ffab4ae7c43b6bcf54a367ae6f9d77c9626
diff --git a/media/libstagefright/foundation/ALooperRoster.cpp b/media/libstagefright/foundation/ALooperRoster.cpp
index 0c181ff..0f44b52 100644
--- a/media/libstagefright/foundation/ALooperRoster.cpp
+++ b/media/libstagefright/foundation/ALooperRoster.cpp
@@ -72,15 +72,27 @@
}
void ALooperRoster::unregisterStaleHandlers() {
- Mutex::Autolock autoLock(mLock);
- for (size_t i = mHandlers.size(); i-- > 0;) {
- const HandlerInfo &info = mHandlers.valueAt(i);
+ Vector<sp<ALooper> > activeLoopers;
+ {
+ Mutex::Autolock autoLock(mLock);
- sp<ALooper> looper = info.mLooper.promote();
- if (looper == NULL) {
- ALOGV("Unregistering stale handler %d", mHandlers.keyAt(i));
- mHandlers.removeItemsAt(i);
+ for (size_t i = mHandlers.size(); i-- > 0;) {
+ const HandlerInfo &info = mHandlers.valueAt(i);
+
+ sp<ALooper> looper = info.mLooper.promote();
+ if (looper == NULL) {
+ ALOGV("Unregistering stale handler %d", mHandlers.keyAt(i));
+ mHandlers.removeItemsAt(i);
+ } else {
+ // At this point 'looper' might be the only sp<> keeping
+ // the object alive. To prevent it from going out of scope
+ // and having ~ALooper call this method again recursively
+ // and then deadlocking because of the Autolock above, add
+ // it to a Vector which will go out of scope after the lock
+ // has been released.
+ activeLoopers.add(looper);
+ }
}
}
}