commit 1686374dbe5cae834fa206f0bdba5637de7aa5fd
author Dominik Laskowski <domlaskowski@google.com> Thu Sep 29 11:57:40 2022 -0400
committer Dominik Laskowski <domlaskowski@google.com> Fri Mar 10 14:45:09 2023 -0500
tree 388eff7f85c0ebe2f2968500660b62cdefc3f726
parent 14956dc43288483d823f07786349278f3e46cbe1

SF: Commit initial DisplayState under lock

The main thread must `applyTransactions` under `mStateLock` during
`onInitializeDisplays`, since a Binder thread could be concurrently
writing to `mCurrentState.displays` while creating or destroying a
virtual display.

Fixes: 244460929
Test: Boot (sporadic crash with no repro steps)
Change-Id: I4f8bac4af7635b9b93c7b79826285a26a109cf9e

services/surfaceflinger/SurfaceFlinger.cpp

diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 54889fa..267637b 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -4224,7 +4224,7 @@
 
 bool SurfaceFlinger::applyTransactions(std::vector<TransactionState>& transactions,
                                        VsyncId vsyncId) {
-    Mutex::Autolock _l(mStateLock);
+    Mutex::Autolock lock(mStateLock);
     return applyTransactionsLocked(transactions, vsyncId);
 }
 
@@ -4505,7 +4505,7 @@
 
 bool SurfaceFlinger::applyAndCommitDisplayTransactionStates(
         std::vector<TransactionState>& transactions) {
-    Mutex::Autolock _l(mStateLock);
+    Mutex::Autolock lock(mStateLock);
     bool needsTraversal = false;
     uint32_t transactionFlags = 0;
     for (auto& transaction : transactions) {
@@ -5246,7 +5246,7 @@
 }
 
 void SurfaceFlinger::onInitializeDisplays() {
-    const auto display = getDefaultDisplayDeviceLocked();
+    const auto display = FTL_FAKE_GUARD(mStateLock, getDefaultDisplayDeviceLocked());
     if (!display) return;
 
     const sp<IBinder> token = display->getDisplayToken().promote();
@@ -5254,13 +5254,13 @@
 
     TransactionState state;
     state.inputWindowCommands = mInputWindowCommands;
-    nsecs_t now = systemTime();
+    const nsecs_t now = systemTime();
     state.desiredPresentTime = now;
     state.postTime = now;
     state.permissions = layer_state_t::ACCESS_SURFACE_FLINGER;
     state.originPid = mPid;
     state.originUid = static_cast<int>(getuid());
-    uint64_t transactionId = (((uint64_t)mPid) << 32) | mUniqueTransactionId++;
+    const uint64_t transactionId = (static_cast<uint64_t>(mPid) << 32) | mUniqueTransactionId++;
     state.id = transactionId;
 
     // reset screen orientation and use primary layer stack
@@ -5280,21 +5280,22 @@
     std::vector<TransactionState> transactions;
     transactions.emplace_back(state);
 
-    // It should be on the main thread, apply it directly.
     if (mLegacyFrontEndEnabled) {
-        applyTransactionsLocked(transactions, /*vsyncId=*/{0});
+        applyTransactions(transactions, VsyncId{0});
     } else {
         applyAndCommitDisplayTransactionStates(transactions);
     }
 
-    setPowerModeInternal(display, hal::PowerMode::ON);
+    {
+        ftl::FakeGuard guard(mStateLock);
+        setPowerModeInternal(display, hal::PowerMode::ON);
+    }
 }
 
 void SurfaceFlinger::initializeDisplays() {
     // Async since we may be called from the main thread.
     static_cast<void>(mScheduler->schedule(
-            [this]() FTL_FAKE_GUARD(mStateLock)
-                    FTL_FAKE_GUARD(kMainThreadContext) { onInitializeDisplays(); }));
+            [this]() FTL_FAKE_GUARD(kMainThreadContext) { onInitializeDisplays(); }));
 }
 
 void SurfaceFlinger::setPowerModeInternal(const sp<DisplayDevice>& display, hal::PowerMode mode) {