SF: Fix a race between layer creation and apply transaction
Between commitCreatedLayers and applyTransactions in the main
thread, the client could create a new layer and queue a transaction.
This will mean a layer transaction can be applied before the layer
can be committed.
Fix this by flushing the transactions to be applied before
committing any new layers.
Test: presubmit
Fixes: b/262336014
Change-Id: Id2848af4fbb7afe4e6a20a48f6a8a13f322e1cd7
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index e8dc7d4..fbb2dee 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2250,9 +2250,17 @@
bool needsTraversal = false;
if (clearTransactionFlags(eTransactionFlushNeeded)) {
+ // Locking:
+ // 1. to prevent onHandleDestroyed from being called while the state lock is held,
+ // we must keep a copy of the transactions (specifically the composer
+ // states) around outside the scope of the lock.
+ // 2. Transactions and created layers do not share a lock. To prevent applying
+ // transactions with layers still in the createdLayer queue, flush the transactions
+ // before committing the created layers.
+ std::vector<TransactionState> transactions = mTransactionHandler.flushTransactions();
needsTraversal |= commitMirrorDisplays(vsyncId);
needsTraversal |= commitCreatedLayers(vsyncId);
- needsTraversal |= flushTransactionQueues(vsyncId);
+ needsTraversal |= applyTransactions(transactions, vsyncId);
}
const bool shouldCommit =
@@ -3948,19 +3956,20 @@
std::bind(&SurfaceFlinger::transactionReadyBufferCheck, this, std::placeholders::_1));
}
+// For tests only
bool SurfaceFlinger::flushTransactionQueues(VsyncId vsyncId) {
- // to prevent onHandleDestroyed from being called while the lock is held,
- // we must keep a copy of the transactions (specifically the composer
- // states) around outside the scope of the lock
std::vector<TransactionState> transactions = mTransactionHandler.flushTransactions();
- {
- Mutex::Autolock _l(mStateLock);
- return applyTransactions(transactions, vsyncId);
- }
+ return applyTransactions(transactions, vsyncId);
}
bool SurfaceFlinger::applyTransactions(std::vector<TransactionState>& transactions,
VsyncId vsyncId) {
+ Mutex::Autolock _l(mStateLock);
+ return applyTransactionsLocked(transactions, vsyncId);
+}
+
+bool SurfaceFlinger::applyTransactionsLocked(std::vector<TransactionState>& transactions,
+ VsyncId vsyncId) {
bool needsTraversal = false;
// Now apply all transactions.
for (auto& transaction : transactions) {