Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 75c672fc authored by Marco Nelissen's avatar Marco Nelissen
Browse files

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
parent 9dd4a2dd

media/libstagefright/foundation/ALooperRoster.cpp

+19 −7
Original line number Diff line number Diff line
@@ -72,6 +72,9 @@ void ALooperRoster::unregisterHandler(ALooper::handler_id handlerID) { 
}



void ALooperRoster::unregisterStaleHandlers() {



    Vector<sp<ALooper> > activeLoopers;

    {

        Mutex::Autolock autoLock(mLock);



        for (size_t i = mHandlers.size(); i-- > 0;) {
@@ -81,6 +84,15 @@ void ALooperRoster::unregisterStaleHandlers() { 
            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);

            }

        }

    }

}