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

Commit 5312ec10 authored by Patrick Williams's avatar Patrick Williams
Browse files

Replace BBQ callback thunks with lambdas 

The lambdas allows for better StrongPointer/RefBase safety by replacing explicit incStrong/decStrong calls with RAII. Captured StrongPointers are created using sp<T>::fromExisting which helps detect RefBase errors such as creating a StrongPointer from an already deleted RefBase object.

Bug: 361588984
Flag: EXEMPT refactor
Test: presubmits
Change-Id: I1a0c0df882ec2a3f91f857c446418f3fb51689c1
parent 4efab5d7

libs/gui/BLASTBufferQueue.cpp

+28 −37
Original line number Diff line number Diff line
@@ -306,14 +306,12 @@ static std::optional<SurfaceControlStats> findMatchingStat( 
    return std::nullopt;

}



static void transactionCommittedCallbackThunk(void* context, nsecs_t latchTime,

                                              const sp<Fence>& presentFence,

TransactionCompletedCallbackTakesContext BLASTBufferQueue::makeTransactionCommittedCallbackThunk() {

    return [bbq = sp<BLASTBufferQueue>::fromExisting(

                    this)](void* /*context*/, nsecs_t latchTime, const sp<Fence>& presentFence,

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

    if (context == nullptr) {

        return;

    }

    sp<BLASTBufferQueue> bq = static_cast<BLASTBufferQueue*>(context);

    bq->transactionCommittedCallback(latchTime, presentFence, stats);

        bbq->transactionCommittedCallback(latchTime, presentFence, stats);

    };

}



void BLASTBufferQueue::transactionCommittedCallback(nsecs_t /*latchTime*/,
@@ -346,19 +344,15 @@ void BLASTBufferQueue::transactionCommittedCallback(nsecs_t /*latchTime*/, 
            BQA_LOGE("No matching SurfaceControls found: mSurfaceControlsWithPendingCallback was "

                     "empty.");

        }

        decStrong((void*)transactionCommittedCallbackThunk);

    }

}



static void transactionCallbackThunk(void* context, nsecs_t latchTime,

                                     const sp<Fence>& presentFence,

TransactionCompletedCallbackTakesContext BLASTBufferQueue::makeTransactionCallbackThunk() {

    return [bbq = sp<BLASTBufferQueue>::fromExisting(

                    this)](void* /*context*/, nsecs_t latchTime, const sp<Fence>& presentFence,

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

    if (context == nullptr) {

        return;

    }

    auto bq = static_cast<BLASTBufferQueue*>(context);

    bq->transactionCallback(latchTime, presentFence, stats);

    bq->decStrong((void*)transactionCallbackThunk);

        bbq->transactionCallback(latchTime, presentFence, stats);

    };

}



void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp<Fence>& /*presentFence*/,
@@ -421,15 +415,17 @@ void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp<Fence 
// BBQ. This is because if the BBQ is destroyed, then the buffers will be released by the client.

// So we pass in a weak pointer to the BBQ and if it still alive, then we release the buffer.

// Otherwise, this is a no-op.

static void releaseBufferCallbackThunk(wp<BLASTBufferQueue> context, const ReleaseCallbackId& id,

                                       const sp<Fence>& releaseFence,

ReleaseBufferCallback BLASTBufferQueue::makeReleaseBufferCallbackThunk() {

    return [weakBbq = wp<BLASTBufferQueue>::fromExisting(

                    this)](const ReleaseCallbackId& id, const sp<Fence>& releaseFence,

                           std::optional<uint32_t> currentMaxAcquiredBufferCount) {

    sp<BLASTBufferQueue> blastBufferQueue = context.promote();

    if (blastBufferQueue) {

        blastBufferQueue->releaseBufferCallback(id, releaseFence, currentMaxAcquiredBufferCount);

    } else {

        sp<BLASTBufferQueue> bbq = weakBbq.promote();

        if (!bbq) {

            ALOGV("releaseBufferCallbackThunk %s blastBufferQueue is dead", id.to_string().c_str());

            return;

        }

        bbq->releaseBufferCallback(id, releaseFence, currentMaxAcquiredBufferCount);

    };

}



void BLASTBufferQueue::flushShadowQueue() {
@@ -609,9 +605,6 @@ status_t BLASTBufferQueue::acquireNextBufferLocked( 
        t->notifyProducerDisconnect(mSurfaceControl);

    }



    // Ensure BLASTBufferQueue stays alive until we receive the transaction complete callback.

    incStrong((void*)transactionCallbackThunk);



    // Only update mSize for destination bounds if the incoming buffer matches the requested size.

    // Otherwise, it could cause stretching since the destination bounds will update before the

    // buffer with the new size is acquired.
@@ -624,9 +617,7 @@ status_t BLASTBufferQueue::acquireNextBufferLocked( 
                           bufferItem.mGraphicBuffer->getHeight(), bufferItem.mTransform,

                           bufferItem.mScalingMode, crop);



    auto releaseBufferCallback =

            std::bind(releaseBufferCallbackThunk, wp<BLASTBufferQueue>(this) /* callbackContext */,

                      std::placeholders::_1, std::placeholders::_2, std::placeholders::_3);

    auto releaseBufferCallback = makeReleaseBufferCallbackThunk();

    sp<Fence> fence = bufferItem.mFence ? new Fence(bufferItem.mFence->dup()) : Fence::NO_FENCE;



    nsecs_t dequeueTime = -1;
@@ -644,7 +635,7 @@ status_t BLASTBufferQueue::acquireNextBufferLocked( 
    t->setDataspace(mSurfaceControl, static_cast<ui::Dataspace>(bufferItem.mDataSpace));

    t->setHdrMetadata(mSurfaceControl, bufferItem.mHdrMetadata);

    t->setSurfaceDamageRegion(mSurfaceControl, bufferItem.mSurfaceDamage);

    t->addTransactionCompletedCallback(transactionCallbackThunk, static_cast<void*>(this));

    t->addTransactionCompletedCallback(makeTransactionCallbackThunk(), nullptr);



    mSurfaceControlsWithPendingCallback.push(mSurfaceControl);
@@ -804,9 +795,9 @@ void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) { 


            // Only need a commit callback when syncing to ensure the buffer that's synced has been

            // sent to SF

            incStrong((void*)transactionCommittedCallbackThunk);

            mSyncTransaction->addTransactionCommittedCallback(transactionCommittedCallbackThunk,

                                                              static_cast<void*>(this));

            mSyncTransaction

                    ->addTransactionCommittedCallback(makeTransactionCommittedCallbackThunk(),

                                                      nullptr);

            if (mAcquireSingleBuffer) {

                prevCallback = mTransactionReadyCallback;

                prevTransaction = mSyncTransaction;

libs/gui/SurfaceComposerClient.cpp

+7 −4
Original line number Diff line number Diff line
@@ -2051,7 +2051,8 @@ SurfaceComposerClient::Transaction::setFrameRateSelectionPriority(const sp<Surfa 
SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::addTransactionCallback(

        TransactionCompletedCallbackTakesContext callback, void* callbackContext,

        CallbackId::Type callbackType) {

    auto callbackWithContext = std::bind(callback, callbackContext, std::placeholders::_1,

    auto callbackWithContext =

            std::bind(std::move(callback), callbackContext, std::placeholders::_1,

                      std::placeholders::_2, std::placeholders::_3);

    const auto& surfaceControls = mListenerCallbacks[mTransactionCompletedListener].surfaceControls;
@@ -2066,13 +2067,15 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::addTrans 
SurfaceComposerClient::Transaction&

SurfaceComposerClient::Transaction::addTransactionCompletedCallback(

        TransactionCompletedCallbackTakesContext callback, void* callbackContext) {

    return addTransactionCallback(callback, callbackContext, CallbackId::Type::ON_COMPLETE);

    return addTransactionCallback(std::move(callback), callbackContext,

                                  CallbackId::Type::ON_COMPLETE);

}



SurfaceComposerClient::Transaction&

SurfaceComposerClient::Transaction::addTransactionCommittedCallback(

        TransactionCompletedCallbackTakesContext callback, void* callbackContext) {

    return addTransactionCallback(callback, callbackContext, CallbackId::Type::ON_COMMIT);

    return addTransactionCallback(std::move(callback), callbackContext,

                                  CallbackId::Type::ON_COMMIT);

}



SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::notifyProducerDisconnect(

libs/gui/include/gui/BLASTBufferQueue.h

+6 −0
Original line number Diff line number Diff line
@@ -103,15 +103,21 @@ public: 
    void onFrameDequeued(const uint64_t) override;

    void onFrameCancelled(const uint64_t) override;



    TransactionCompletedCallbackTakesContext makeTransactionCommittedCallbackThunk();

    void transactionCommittedCallback(nsecs_t latchTime, const sp<Fence>& presentFence,

                                      const std::vector<SurfaceControlStats>& stats);



    TransactionCompletedCallbackTakesContext makeTransactionCallbackThunk();

    virtual void transactionCallback(nsecs_t latchTime, const sp<Fence>& presentFence,

                                     const std::vector<SurfaceControlStats>& stats);



    ReleaseBufferCallback makeReleaseBufferCallbackThunk();

    void releaseBufferCallback(const ReleaseCallbackId& id, const sp<Fence>& releaseFence,

                               std::optional<uint32_t> currentMaxAcquiredBufferCount);

    void releaseBufferCallbackLocked(const ReleaseCallbackId& id, const sp<Fence>& releaseFence,

                                     std::optional<uint32_t> currentMaxAcquiredBufferCount,

                                     bool fakeRelease) REQUIRES(mMutex);



    bool syncNextTransaction(std::function<void(SurfaceComposerClient::Transaction*)> callback,

                             bool acquireSingleBuffer = true);

    void stopContinuousSyncTransaction();