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

Commit 0fd0fca4 authored by Vishnu Nair's avatar Vishnu Nair
Browse files

DO NOT MERGE: 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: I9987614ade29456453de3610782a645ea9db0892
parent 0077d6d8

services/surfaceflinger/SurfaceFlinger.cpp

+23 −4
Original line number Diff line number Diff line
@@ -2116,8 +2116,16 @@ bool SurfaceFlinger::commit(nsecs_t frameTime, int64_t vsyncId, nsecs_t expected 


        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 = flushTransactions();

            needsTraversal |= commitCreatedLayers();

            needsTraversal |= flushTransactionQueues(vsyncId);

            needsTraversal |= applyTransactions(transactions, vsyncId);

        }



        const bool shouldCommit =
@@ -3774,7 +3782,7 @@ int SurfaceFlinger::flushPendingTransactionQueues( 
    return transactionsPendingBarrier;

}



bool SurfaceFlinger::flushTransactionQueues(int64_t vsyncId) {

std::vector<TransactionState> SurfaceFlinger::flushTransactions() {

    // 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
@@ -3868,14 +3876,25 @@ bool SurfaceFlinger::flushTransactionQueues(int64_t vsyncId) { 
                flushUnsignaledPendingTransactionQueues(transactions, bufferLayersReadyToPresent,

                                                        applyTokensWithUnsignaledTransactions);

            }



            return applyTransactions(transactions, vsyncId);

        }

    }

    return transactions;

}



// for test only

bool SurfaceFlinger::flushTransactionQueues(int64_t vsyncId) {

    std::vector<TransactionState> transactions = flushTransactions();

    return applyTransactions(transactions, vsyncId);

}



bool SurfaceFlinger::applyTransactions(std::vector<TransactionState>& transactions,

                                       int64_t vsyncId) {

    Mutex::Autolock _l(mStateLock);

    return applyTransactionsLocked(transactions, vsyncId);

}



bool SurfaceFlinger::applyTransactionsLocked(std::vector<TransactionState>& transactions,

                                             int64_t vsyncId) {

    bool needsTraversal = false;

    // Now apply all transactions.

    for (auto& transaction : transactions) {

services/surfaceflinger/SurfaceFlinger.h

+5 −1
Original line number Diff line number Diff line
@@ -770,6 +770,9 @@ private: 
            REQUIRES(mStateLock);

    // flush pending transaction that was presented after desiredPresentTime.

    bool flushTransactionQueues(int64_t vsyncId);



    std::vector<TransactionState> flushTransactions();



    // Returns true if there is at least one transaction that needs to be flushed

    bool transactionFlushNeeded();
@@ -818,7 +821,8 @@ private: 
                               size_t totalTXapplied) const;

    bool stopTransactionProcessing(const std::unordered_set<sp<IBinder>, SpHash<IBinder>>&

                                           applyTokensWithUnsignaledTransactions) const;

    bool applyTransactions(std::vector<TransactionState>& transactions, int64_t vsyncId)

    bool applyTransactions(std::vector<TransactionState>& transactions, int64_t vsyncId);

    bool applyTransactionsLocked(std::vector<TransactionState>& transactions, int64_t vsyncId)

            REQUIRES(mStateLock);

    uint32_t setDisplayStateLocked(const DisplayState& s) REQUIRES(mStateLock);

    uint32_t addInputWindowCommands(const InputWindowCommands& inputWindowCommands)