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

Commit 2274a314 authored by Arthur Hung's avatar Arthur Hung
Browse files

Fix potential synchronized race between SF and InputFlinger 

The 'CountDownLatch' is used to control the synchronization behavior if
a transaction is synchronous or contains the syncInputWindow command,
while updating input windows asynchronously, if there is another
synchronized transaction queued and commited before input windows
updated, it may cause the transaction return early and cause other input
function such as 'transferTouchFocus' failed.

This CL distinguish the transaction and input windows updating
synchronization that could help to signal the CountDownLatch in correct
situation.

Test: atest SurfaceFlinger_tests
Test: atest CrossDragAndDropTests --rerun-until-failure 50
Bug: 183877670
Bug: 183877512
Bug: 183880412
Change-Id: I68e124006e86d8b9c4b8c528418b9db2065b21de
parent 0021fac8

services/surfaceflinger/SurfaceFlinger.cpp

+7 −5
Original line number Original line Diff line number Diff line
@@ -3105,7 +3105,7 @@ void SurfaceFlinger::setVsyncConfig(const VsyncModulator::VsyncConfig& config, 




void SurfaceFlinger::commitTransaction() {

void SurfaceFlinger::commitTransaction() {

    commitTransactionLocked();

    commitTransactionLocked();

    signalSynchronousTransactions();

    signalSynchronousTransactions(CountDownLatch::eSyncTransaction);

    mAnimTransactionPending = false;

    mAnimTransactionPending = false;

}

}
@@ -3527,7 +3527,9 @@ void SurfaceFlinger::queueTransaction(TransactionState& state) { 
    // Generate a CountDownLatch pending state if this is a synchronous transaction.

    // Generate a CountDownLatch pending state if this is a synchronous transaction.

    if ((state.flags & eSynchronous) || state.inputWindowCommands.syncInputWindows) {

    if ((state.flags & eSynchronous) || state.inputWindowCommands.syncInputWindows) {

        state.transactionCommittedSignal = std::make_shared<CountDownLatch>(

        state.transactionCommittedSignal = std::make_shared<CountDownLatch>(

                (state.inputWindowCommands.syncInputWindows ? 2 : 1));

                (state.inputWindowCommands.syncInputWindows

                         ? (CountDownLatch::eSyncInputWindows | CountDownLatch::eSyncTransaction)

                         : CountDownLatch::eSyncTransaction));

    }

    }





    mTransactionQueue.emplace(state);

    mTransactionQueue.emplace(state);
@@ -3552,10 +3554,10 @@ void SurfaceFlinger::waitForSynchronousTransaction( 
    }

    }

}

}





void SurfaceFlinger::signalSynchronousTransactions() {

void SurfaceFlinger::signalSynchronousTransactions(const uint32_t flag) {

    for (auto it = mTransactionCommittedSignals.begin();

    for (auto it = mTransactionCommittedSignals.begin();

         it != mTransactionCommittedSignals.end();) {

         it != mTransactionCommittedSignals.end();) {

        if ((*it)->countDown() == 0) {

        if ((*it)->countDown(flag)) {

            it = mTransactionCommittedSignals.erase(it);

            it = mTransactionCommittedSignals.erase(it);

        } else {

        } else {

            it++;

            it++;
@@ -6175,7 +6177,7 @@ status_t SurfaceFlinger::renderScreenImplLocked( 




void SurfaceFlinger::setInputWindowsFinished() {

void SurfaceFlinger::setInputWindowsFinished() {

    Mutex::Autolock _l(mStateLock);

    Mutex::Autolock _l(mStateLock);

    signalSynchronousTransactions();

    signalSynchronousTransactions(CountDownLatch::eSyncInputWindows);

}

}





// ---------------------------------------------------------------------------

// ---------------------------------------------------------------------------

services/surfaceflinger/SurfaceFlinger.h

+17 −10
Original line number Original line Diff line number Diff line
@@ -467,24 +467,31 @@ private: 




    class CountDownLatch {

    class CountDownLatch {

    public:

    public:

        explicit CountDownLatch(int32_t count) : mCount(count) {}

        enum {

            eSyncTransaction = 1 << 0,

            eSyncInputWindows = 1 << 1,

        };

        explicit CountDownLatch(uint32_t flags) : mFlags(flags) {}





        int32_t countDown() {

        // True if there is no waiting condition after count down.

        bool countDown(uint32_t flag) {

            std::unique_lock<std::mutex> lock(mMutex);

            std::unique_lock<std::mutex> lock(mMutex);

            if (mCount == 0) {

            if (mFlags == 0) {

                return 0;

                return true;

            }

            }

            if (--mCount == 0) {

            mFlags &= ~flag;

            if (mFlags == 0) {

                mCountDownComplete.notify_all();

                mCountDownComplete.notify_all();

                return true;

            }

            }

            return mCount;

            return false;

        }

        }





        // Return true if triggered.

        // Return true if triggered.

        bool wait_until(const std::chrono::seconds& timeout) const {

        bool wait_until(const std::chrono::seconds& timeout) const {

            std::unique_lock<std::mutex> lock(mMutex);

            std::unique_lock<std::mutex> lock(mMutex);

            const auto untilTime = std::chrono::system_clock::now() + timeout;

            const auto untilTime = std::chrono::system_clock::now() + timeout;

            while (mCount != 0) {

            while (mFlags != 0) {

                // Conditional variables can be woken up sporadically, so we check count

                // Conditional variables can be woken up sporadically, so we check count

                // to verify the wakeup was triggered by |countDown|.

                // to verify the wakeup was triggered by |countDown|.

                if (std::cv_status::timeout == mCountDownComplete.wait_until(lock, untilTime)) {

                if (std::cv_status::timeout == mCountDownComplete.wait_until(lock, untilTime)) {
@@ -495,7 +502,7 @@ private: 
        }

        }





    private:

    private:

        int32_t mCount;

        uint32_t mFlags;

        mutable std::condition_variable mCountDownComplete;

        mutable std::condition_variable mCountDownComplete;

        mutable std::mutex mMutex;

        mutable std::mutex mMutex;

    };

    };
@@ -1124,7 +1131,7 @@ private: 
    // Add transaction to the Transaction Queue

    // Add transaction to the Transaction Queue

    void queueTransaction(TransactionState& state) EXCLUDES(mQueueLock);

    void queueTransaction(TransactionState& state) EXCLUDES(mQueueLock);

    void waitForSynchronousTransaction(const CountDownLatch& transactionCommittedSignal);

    void waitForSynchronousTransaction(const CountDownLatch& transactionCommittedSignal);

    void signalSynchronousTransactions();

    void signalSynchronousTransactions(const uint32_t flag);





    /*

    /*

     * Generic Layer Metadata

     * Generic Layer Metadata