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

Commit 4e04c3a3 authored by Vishnu Nair's avatar Vishnu Nair
Browse files

SurfaceComposerClient: Handle transaction apply sync timeouts 

We switched to using transaction complete callbacks to handle
transaction#apply(/*sync*/=true). The callback object was
destroyed after waiting for the callback. In the event of a
timeout, the callback would access an invalid object.

Fix this by using ref counted object to ensure the context
remains valid.

Fixes: 261679196
Test: atest SurfaceFlinger_test
Change-Id: I4f840214672dd4051cb57b9551bf20802cc90890
parent 52363196

libs/gui/SurfaceComposerClient.cpp

+15 −12
Original line number Original line Diff line number Diff line
@@ -971,14 +971,16 @@ void SurfaceComposerClient::Transaction::cacheBuffers() { 




class SyncCallback {

class SyncCallback {

public:

public:

    static void function(void* callbackContext, nsecs_t /* latchTime */,

    static auto getCallback(std::shared_ptr<SyncCallback>& callbackContext) {

        return [callbackContext](void* /* unused context */, nsecs_t /* latchTime */,

                                 const sp<Fence>& /* presentFence */,

                                 const sp<Fence>& /* presentFence */,

                                 const std::vector<SurfaceControlStats>& /* stats */) {

                                 const std::vector<SurfaceControlStats>& /* stats */) {

            if (!callbackContext) {

            if (!callbackContext) {

                ALOGE("failed to get callback context for SyncCallback");

                ALOGE("failed to get callback context for SyncCallback");

                return;

            }

            }

        SyncCallback* helper = static_cast<SyncCallback*>(callbackContext);

            LOG_ALWAYS_FATAL_IF(sem_post(&callbackContext->mSemaphore), "sem_post failed");

        LOG_ALWAYS_FATAL_IF(sem_post(&helper->mSemaphore), "sem_post failed");

        };

    }

    }

    ~SyncCallback() {

    ~SyncCallback() {

        if (mInitialized) {

        if (mInitialized) {
@@ -1013,10 +1015,11 @@ status_t SurfaceComposerClient::Transaction::apply(bool synchronous, bool oneWay 
        return mStatus;

        return mStatus;

    }

    }





    SyncCallback syncCallback;

    std::shared_ptr<SyncCallback> syncCallback = std::make_shared<SyncCallback>();

    if (synchronous) {

    if (synchronous) {

        syncCallback.init();

        syncCallback->init();

        addTransactionCommittedCallback(syncCallback.function, syncCallback.getContext());

        addTransactionCommittedCallback(SyncCallback::getCallback(syncCallback),

                                        /*callbackContext=*/nullptr);

    }

    }





    bool hasListenerCallbacks = !mListenerCallbacks.empty();

    bool hasListenerCallbacks = !mListenerCallbacks.empty();
@@ -1092,7 +1095,7 @@ status_t SurfaceComposerClient::Transaction::apply(bool synchronous, bool oneWay 
    clear();

    clear();





    if (synchronous) {

    if (synchronous) {

        syncCallback.wait();

        syncCallback->wait();

    }

    }





    mStatus = NO_ERROR;

    mStatus = NO_ERROR;