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
diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp
index 1fb59fd..f8e3fd0 100644
--- a/libs/gui/BLASTBufferQueue.cpp
+++ b/libs/gui/BLASTBufferQueue.cpp
@@ -306,14 +306,12 @@
return std::nullopt;
}
-static void transactionCommittedCallbackThunk(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);
+TransactionCompletedCallbackTakesContext BLASTBufferQueue::makeTransactionCommittedCallbackThunk() {
+ return [bbq = sp<BLASTBufferQueue>::fromExisting(
+ this)](void* /*context*/, nsecs_t latchTime, const sp<Fence>& presentFence,
+ const std::vector<SurfaceControlStats>& stats) {
+ bbq->transactionCommittedCallback(latchTime, presentFence, stats);
+ };
}
void BLASTBufferQueue::transactionCommittedCallback(nsecs_t /*latchTime*/,
@@ -346,19 +344,15 @@
BQA_LOGE("No matching SurfaceControls found: mSurfaceControlsWithPendingCallback was "
"empty.");
}
- decStrong((void*)transactionCommittedCallbackThunk);
}
}
-static void transactionCallbackThunk(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);
+TransactionCompletedCallbackTakesContext BLASTBufferQueue::makeTransactionCallbackThunk() {
+ return [bbq = sp<BLASTBufferQueue>::fromExisting(
+ this)](void* /*context*/, nsecs_t latchTime, const sp<Fence>& presentFence,
+ const std::vector<SurfaceControlStats>& stats) {
+ bbq->transactionCallback(latchTime, presentFence, stats);
+ };
}
void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp<Fence>& /*presentFence*/,
@@ -421,15 +415,17 @@
// 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,
- std::optional<uint32_t> currentMaxAcquiredBufferCount) {
- sp<BLASTBufferQueue> blastBufferQueue = context.promote();
- if (blastBufferQueue) {
- blastBufferQueue->releaseBufferCallback(id, releaseFence, currentMaxAcquiredBufferCount);
- } else {
- ALOGV("releaseBufferCallbackThunk %s blastBufferQueue is dead", id.to_string().c_str());
- }
+ReleaseBufferCallback BLASTBufferQueue::makeReleaseBufferCallbackThunk() {
+ return [weakBbq = wp<BLASTBufferQueue>::fromExisting(
+ this)](const ReleaseCallbackId& id, const sp<Fence>& releaseFence,
+ std::optional<uint32_t> currentMaxAcquiredBufferCount) {
+ 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 @@
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 @@
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 @@
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 @@
// 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;