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

Commit 6ed2ea83 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: Ifc5357bd181e0cd065cdecd0188836a35f87b3e2
parent 1283885c

libs/gui/SurfaceComposerClient.cpp

+13 −9
Original line number Diff line number Diff line
@@ -210,13 +210,13 @@ void TransactionCompletedListener::removeReleaseBufferCallback(uint64_t graphicB 


void TransactionCompletedListener::addSurfaceStatsListener(void* context, void* cookie,

        sp<SurfaceControl> surfaceControl, SurfaceStatsCallback listener) {

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

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

    mSurfaceStatsListeners.insert({surfaceControl->getHandle(),

            SurfaceStatsCallbackEntry(context, cookie, listener)});

}



void TransactionCompletedListener::removeSurfaceStatsListener(void* context, void* cookie) {

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

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

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

        auto [itContext, itCookie, itListener] = it->second;

        if (itContext == context && itCookie == cookie) {
@@ -242,7 +242,6 @@ void TransactionCompletedListener::addSurfaceControlToCallbacks( 


void TransactionCompletedListener::onTransactionCompleted(ListenerStats listenerStats) {

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

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

    {

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

         */

        callbacksMap = mCallbacks;

        surfaceListeners = mSurfaceStatsListeners;

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

            for (auto& callbackId : transactionStats.callbackIds) {

                mCallbacks.erase(callbackId);
@@ -338,12 +336,18 @@ void TransactionCompletedListener::onTransactionCompleted(ListenerStats listener 
                             surfaceControlStats);

        }

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

            auto listenerRange = surfaceListeners.equal_range(surfaceStats.surfaceControl);

            {

                // Acquire surface stats listener lock such that we guarantee that after calling

                // unregister, there won't be any further callback.

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

                auto listenerRange = mSurfaceStatsListeners.equal_range(

                        surfaceStats.surfaceControl);

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

                    auto entry = it->second;

                    entry.callback(entry.context, transactionStats.latchTime,

                        transactionStats.presentFence, surfaceStats);

                }

            }



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

libs/gui/include/gui/SurfaceComposerClient.h

+7 −2
Original line number Diff line number Diff line
@@ -654,6 +654,9 @@ class TransactionCompletedListener : public BnTransactionCompletedListener { 
    // This lock needs to be recursive so we can unregister a callback from within that callback.

    std::recursive_mutex mJankListenerMutex;



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

    std::recursive_mutex mSurfaceStatsListenerMutex;



    bool mListening GUARDED_BY(mMutex) = false;



    int64_t mCallbackIdCounter GUARDED_BY(mMutex) = 1;
@@ -682,8 +685,10 @@ class TransactionCompletedListener : public BnTransactionCompletedListener { 
    std::multimap<sp<IBinder>, sp<JankDataListener>> mJankListeners;

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

            mReleaseBufferCallbacks GUARDED_BY(mMutex);

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

                mSurfaceStatsListeners GUARDED_BY(mMutex);



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

    // std::recursive_mutex

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



public:

    static sp<TransactionCompletedListener> getInstance();