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

Commit 1283885c authored by Jorim Jaggi's avatar Jorim Jaggi
Browse files

Ensure reportFrameMetrics not being called on deleted instance 

Since onSurfaceStatsAvailable gets called on binder-thread, we
need to ensure that instance doesn't get released while
onSurfaceStatsAvailable is calling reportFrameMetrics.

We do this by introducing a lock for register/unregister/callback,
such than when unregister completes, there won't be any "hanging"
callback anymore such that the callback can't be called anymore
on deleted instances.

Test: Boots
Bug: 188934435
Change-Id: I73e2d4b04cc74a152054cf7620a5c541683cb5e6
parent 266ca8e1

libs/gui/SurfaceComposerClient.cpp

+8 −5
Original line number Diff line number Diff line
@@ -182,12 +182,12 @@ CallbackId TransactionCompletedListener::addCallbackFunction( 


void TransactionCompletedListener::addJankListener(const sp<JankDataListener>& listener,

                                                   sp<SurfaceControl> surfaceControl) {

    std::lock_guard<std::mutex> lock(mMutex);

    std::scoped_lock<std::recursive_mutex> lock(mJankListenerMutex);

    mJankListeners.insert({surfaceControl->getHandle(), listener});

}



void TransactionCompletedListener::removeJankListener(const sp<JankDataListener>& listener) {

    std::lock_guard<std::mutex> lock(mMutex);

    std::scoped_lock<std::recursive_mutex> lock(mJankListenerMutex);

    for (auto it = mJankListeners.begin(); it != mJankListeners.end();) {

        if (it->second == listener) {

            it = mJankListeners.erase(it);
@@ -242,7 +242,6 @@ void TransactionCompletedListener::addSurfaceControlToCallbacks( 


void TransactionCompletedListener::onTransactionCompleted(ListenerStats listenerStats) {

    std::unordered_map<CallbackId, CallbackTranslation, CallbackIdHash> callbacksMap;

    std::multimap<sp<IBinder>, sp<JankDataListener>> jankListenersMap;

    std::multimap<sp<IBinder>, SurfaceStatsCallbackEntry> surfaceListeners;

    {

        std::lock_guard<std::mutex> lock(mMutex);
@@ -259,7 +258,6 @@ void TransactionCompletedListener::onTransactionCompleted(ListenerStats listener 
         * sp<SurfaceControl> that could possibly exist for the callbacks.

         */

        callbacksMap = mCallbacks;

        jankListenersMap = mJankListeners;

        surfaceListeners = mSurfaceStatsListeners;

        for (const auto& transactionStats : listenerStats.transactionStats) {

            for (auto& callbackId : transactionStats.callbackIds) {
@@ -348,7 +346,12 @@ void TransactionCompletedListener::onTransactionCompleted(ListenerStats listener 
            }



            if (surfaceStats.jankData.empty()) continue;

            auto jankRange = jankListenersMap.equal_range(surfaceStats.surfaceControl);



            // Acquire jank listener lock such that we guarantee that after calling unregister,

            // there won't be any further callback.

            std::scoped_lock<std::recursive_mutex> lock(mJankListenerMutex);

            auto copy = mJankListeners;

            auto jankRange = copy.equal_range(surfaceStats.surfaceControl);

            for (auto it = jankRange.first; it != jankRange.second; it++) {

                it->second->onJankDataAvailable(surfaceStats.jankData);

            }

libs/gui/include/gui/SurfaceComposerClient.h

+7 −1
Original line number Diff line number Diff line
@@ -651,6 +651,9 @@ class TransactionCompletedListener : public BnTransactionCompletedListener { 


    std::mutex mMutex;



    // This lock needs to be recursive so we can unregister a callback from within that callback.

    std::recursive_mutex mJankListenerMutex;



    bool mListening GUARDED_BY(mMutex) = false;



    int64_t mCallbackIdCounter GUARDED_BY(mMutex) = 1;
@@ -673,7 +676,10 @@ class TransactionCompletedListener : public BnTransactionCompletedListener { 


    std::unordered_map<CallbackId, CallbackTranslation, CallbackIdHash> mCallbacks

            GUARDED_BY(mMutex);

    std::multimap<sp<IBinder>, sp<JankDataListener>> mJankListeners GUARDED_BY(mMutex);



    // This is protected by mJankListenerMutex, but GUARDED_BY isn't supported for

    // std::recursive_mutex

    std::multimap<sp<IBinder>, sp<JankDataListener>> mJankListeners;

    std::unordered_map<uint64_t /* graphicsBufferId */, ReleaseBufferCallback>

            mReleaseBufferCallbacks GUARDED_BY(mMutex);

    std::multimap<sp<IBinder>, SurfaceStatsCallbackEntry>