diff options
-rw-r--r-- | services/surfaceflinger/SurfaceFlinger.cpp | 31 | ||||
-rw-r--r-- | services/surfaceflinger/SurfaceFlinger.h | 7 | ||||
-rw-r--r-- | services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h | 2 |
3 files changed, 30 insertions, 10 deletions
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index f7cc13e7c1..b980a65255 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -2252,7 +2252,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, } } + // 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; @@ -3266,6 +3272,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 @@ -5070,9 +5089,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) { @@ -5864,7 +5882,8 @@ void SurfaceFlinger::initializeDisplays() { if (mLegacyFrontEndEnabled) { applyTransactions(transactions, VsyncId{0}); } else { - applyAndCommitDisplayTransactionStates(transactions); + Mutex::Autolock lock(mStateLock); + applyAndCommitDisplayTransactionStatesLocked(transactions); } { diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 6b4440193b..61360a70df 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -751,7 +751,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); @@ -804,8 +805,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(); diff --git a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h index 4fc39cc912..81fbc847a3 100644 --- a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h +++ b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h @@ -610,7 +610,7 @@ public: { ftl::FakeGuard guard(kMainThreadContext); - mFlinger->commitTransactions(); + mFlinger->commitTransactionsLegacy(); mFlinger->flushTransactionQueues(getFuzzedVsyncId(mFdp)); scheduler::FrameTargeter frameTargeter(displayId, mFdp.ConsumeBool()); |