BLASTBufferQueue: Don't use mNextTransaction on Binder thread.
The "mUseNextTransaction" boolean was intended to ensure that we only
used mNextTransaction when calling processBuffer from frameAvailable
and not when processing the queue from the transaction callback. However
this didn't quite work because we can end up returning early from
processNextBuffer() and then the next call (coming from the Transation callback)
will use mNextTransaction from the binder thread. If we randomly access
mNextTransaction from random threads it's very difficult for other threads
to use, so we avoid this directly.
Test: Existing tests pass
Bug: 153120755
Change-Id: Ic261d1fdf3f1458f208face572f2780c1043192c
diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp
index a8384ac..b0d9521 100644
--- a/libs/gui/BLASTBufferQueue.cpp
+++ b/libs/gui/BLASTBufferQueue.cpp
@@ -189,13 +189,13 @@
mPendingReleaseItem.item = std::move(mSubmitted.front());
mSubmitted.pop();
- processNextBufferLocked();
+ processNextBufferLocked(false);
mCallbackCV.notify_all();
decStrong((void*)transactionCallbackThunk);
}
-void BLASTBufferQueue::processNextBufferLocked() {
+void BLASTBufferQueue::processNextBufferLocked(bool useNextTransaction) {
ATRACE_CALL();
if (mNumFrameAvailable == 0 || mNumAcquired == MAX_ACQUIRED_BUFFERS + 1) {
return;
@@ -209,7 +209,7 @@
SurfaceComposerClient::Transaction localTransaction;
bool applyTransaction = true;
SurfaceComposerClient::Transaction* t = &localTransaction;
- if (mNextTransaction != nullptr && mUseNextTransaction) {
+ if (mNextTransaction != nullptr && useNextTransaction) {
t = mNextTransaction;
mNextTransaction = nullptr;
applyTransaction = false;
@@ -274,16 +274,14 @@
while (mNumFrameAvailable > 0 || mNumAcquired == MAX_ACQUIRED_BUFFERS + 1) {
mCallbackCV.wait(_lock);
}
- mUseNextTransaction = true;
}
// add to shadow queue
mNumFrameAvailable++;
- processNextBufferLocked();
+ processNextBufferLocked(true);
}
void BLASTBufferQueue::setNextTransaction(SurfaceComposerClient::Transaction* t) {
std::lock_guard _lock{mMutex};
- mUseNextTransaction = false;
mNextTransaction = t;
}
diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h
index 64c21e0..b902615 100644
--- a/libs/gui/include/gui/BLASTBufferQueue.h
+++ b/libs/gui/include/gui/BLASTBufferQueue.h
@@ -90,7 +90,7 @@
BLASTBufferQueue& operator = (const BLASTBufferQueue& rhs);
BLASTBufferQueue(const BLASTBufferQueue& rhs);
- void processNextBufferLocked() REQUIRES(mMutex);
+ void processNextBufferLocked(bool useNextTransaction) REQUIRES(mMutex);
Rect computeCrop(const BufferItem& item);
sp<SurfaceControl> mSurfaceControl;
@@ -123,8 +123,6 @@
sp<BLASTBufferItemConsumer> mBufferItemConsumer;
SurfaceComposerClient::Transaction* mNextTransaction GUARDED_BY(mMutex);
-
- bool mUseNextTransaction = false;
};
} // namespace android