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

Commit 3b468eca authored by Vishnu Nair's avatar Vishnu Nair
Browse files

Fix sync issue with handling display state changes 

We may miss some state changes if a display state change comes between
processDisplayChangesLocked and commitTransactions. Fix this by grabbing
the state lock for the duration of display updates in commit.

Test: steps in bug

Bug: 330105711, 330103914, 328539539
Merged-In: I4798961551f78d75c45ead6dea5ebca895e5ef7d
Change-Id: I4798961551f78d75c45ead6dea5ebca895e5ef7d
(cherry picked from commit 878911f7)
parent b00cdc55

services/surfaceflinger/SurfaceFlinger.cpp

+25 −6
Original line number Diff line number Diff line
@@ -2256,7 +2256,7 @@ bool SurfaceFlinger::updateLayerSnapshotsLegacy(VsyncId vsyncId, nsecs_t frameTi 
    outTransactionsAreEmpty = !needsTraversal;

    const bool shouldCommit = (getTransactionFlags() & ~eTransactionFlushNeeded) || needsTraversal;

    if (shouldCommit) {

        commitTransactions();

        commitTransactionsLegacy();

    }



    bool mustComposite = latchBuffers() || shouldCommit;
@@ -2380,8 +2380,14 @@ bool SurfaceFlinger::updateLayerSnapshots(VsyncId vsyncId, nsecs_t frameTimeNs, 
        mLayerHierarchyBuilder.update(mLayerLifecycleManager);

    }



    // Keep a copy of the drawing state (that is going to be overwritten

    // by commitTransactionsLocked) outside of mStateLock so that the side

    // effects of the State assignment don't happen with mStateLock held,

    // which can cause deadlocks.

    State drawingState(mDrawingState);

    Mutex::Autolock lock(mStateLock);

    bool mustComposite = false;

    mustComposite |= applyAndCommitDisplayTransactionStates(update.transactions);

    mustComposite |= applyAndCommitDisplayTransactionStatesLocked(update.transactions);



    {

        ATRACE_NAME("LayerSnapshotBuilder:update");
@@ -2420,7 +2426,7 @@ bool SurfaceFlinger::updateLayerSnapshots(VsyncId vsyncId, nsecs_t frameTimeNs, 
    bool newDataLatched = false;

    if (!mLegacyFrontEndEnabled) {

        ATRACE_NAME("DisplayCallbackAndStatsUpdates");

        mustComposite |= applyTransactions(update.transactions, vsyncId);

        mustComposite |= applyTransactionsLocked(update.transactions, vsyncId);

        traverseLegacyLayers([&](Layer* layer) { layer->commitTransaction(); });

        const nsecs_t latchTime = systemTime();

        bool unused = false;
@@ -3257,6 +3263,19 @@ void SurfaceFlinger::computeLayerBounds() { 


void SurfaceFlinger::commitTransactions() {

    ATRACE_CALL();

    mDebugInTransaction = systemTime();



    // Here we're guaranteed that some transaction flags are set

    // so we can call commitTransactionsLocked unconditionally.

    // We clear the flags with mStateLock held to guarantee that

    // mCurrentState won't change until the transaction is committed.

    mScheduler->modulateVsync({}, &VsyncModulator::onTransactionCommit);

    commitTransactionsLocked(clearTransactionFlags(eTransactionMask));

    mDebugInTransaction = 0;

}



void SurfaceFlinger::commitTransactionsLegacy() {

    ATRACE_CALL();



    // Keep a copy of the drawing state (that is going to be overwritten

    // by commitTransactionsLocked) outside of mStateLock so that the side
@@ -5219,9 +5238,8 @@ bool SurfaceFlinger::applyTransactionState(const FrameTimelineInfo& frameTimelin 
    return needsTraversal;

}



bool SurfaceFlinger::applyAndCommitDisplayTransactionStates(

bool SurfaceFlinger::applyAndCommitDisplayTransactionStatesLocked(

        std::vector<TransactionState>& transactions) {

    Mutex::Autolock lock(mStateLock);

    bool needsTraversal = false;

    uint32_t transactionFlags = 0;

    for (auto& transaction : transactions) {
@@ -6009,7 +6027,8 @@ void SurfaceFlinger::initializeDisplays() { 
    if (mLegacyFrontEndEnabled) {

        applyTransactions(transactions, VsyncId{0});

    } else {

        applyAndCommitDisplayTransactionStates(transactions);

        Mutex::Autolock lock(mStateLock);

        applyAndCommitDisplayTransactionStatesLocked(transactions);

    }



    {

services/surfaceflinger/SurfaceFlinger.h

+4 −3
Original line number Diff line number Diff line
@@ -750,7 +750,8 @@ private: 
                                            bool force = false)

            REQUIRES(mStateLock, kMainThreadContext);



    void commitTransactions() EXCLUDES(mStateLock) REQUIRES(kMainThreadContext);

    void commitTransactionsLegacy() EXCLUDES(mStateLock) REQUIRES(kMainThreadContext);

    void commitTransactions() REQUIRES(kMainThreadContext, mStateLock);

    void commitTransactionsLocked(uint32_t transactionFlags)

            REQUIRES(mStateLock, kMainThreadContext);

    void doCommitTransactions() REQUIRES(mStateLock);
@@ -800,8 +801,8 @@ private: 
    bool flushTransactionQueues(VsyncId) REQUIRES(kMainThreadContext);



    bool applyTransactions(std::vector<TransactionState>&, VsyncId) REQUIRES(kMainThreadContext);

    bool applyAndCommitDisplayTransactionStates(std::vector<TransactionState>& transactions)

            REQUIRES(kMainThreadContext);

    bool applyAndCommitDisplayTransactionStatesLocked(std::vector<TransactionState>& transactions)

            REQUIRES(kMainThreadContext, mStateLock);



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

    bool transactionFlushNeeded();