SF: Enforce thread safety for SurfaceFlinger class

Add thread annotations to display state and accessors. Add macros and
RAII helpers as escape hatches that are stricter and more meaningful
than NO_THREAD_SAFETY_ANALYSIS, and emit error on use of the latter.

Bug: 123715322
Test: Build
Change-Id: Ibada81998d70c940c7406ef292b2d487fb02189d
Merged-In: Ibada81998d70c940c7406ef292b2d487fb02189d
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 4cd1c9b..e65b80a 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -125,6 +125,19 @@
 #include "android-base/parseint.h"
 #include "android-base/stringprintf.h"
 
+#define MAIN_THREAD ACQUIRE(mStateLock) RELEASE(mStateLock)
+
+#define ON_MAIN_THREAD(expr)                                       \
+    [&] {                                                          \
+        LOG_FATAL_IF(std::this_thread::get_id() != mMainThreadId); \
+        UnnecessaryLock lock(mStateLock);                          \
+        return (expr);                                             \
+    }()
+
+#undef NO_THREAD_SAFETY_ANALYSIS
+#define NO_THREAD_SAFETY_ANALYSIS \
+    _Pragma("GCC error \"Prefer MAIN_THREAD macros or {Conditional,Timed,Unnecessary}Lock.\"")
+
 namespace android {
 
 using namespace std::string_literals;
@@ -173,12 +186,12 @@
 #pragma clang diagnostic pop
 
 template <typename Mutex>
-struct ConditionalLockGuard {
-    ConditionalLockGuard(Mutex& mutex, bool lock) : mutex(mutex), lock(lock) {
+struct SCOPED_CAPABILITY ConditionalLockGuard {
+    ConditionalLockGuard(Mutex& mutex, bool lock) ACQUIRE(mutex) : mutex(mutex), lock(lock) {
         if (lock) mutex.lock();
     }
 
-    ~ConditionalLockGuard() {
+    ~ConditionalLockGuard() RELEASE() {
         if (lock) mutex.unlock();
     }
 
@@ -188,6 +201,27 @@
 
 using ConditionalLock = ConditionalLockGuard<Mutex>;
 
+struct SCOPED_CAPABILITY TimedLock {
+    TimedLock(Mutex& mutex, nsecs_t timeout, const char* whence) ACQUIRE(mutex)
+          : mutex(mutex), status(mutex.timedLock(timeout)) {
+        ALOGE_IF(!locked(), "%s timed out locking: %s (%d)", whence, strerror(-status), status);
+    }
+
+    ~TimedLock() RELEASE() {
+        if (locked()) mutex.unlock();
+    }
+
+    bool locked() const { return status == NO_ERROR; }
+
+    Mutex& mutex;
+    const status_t status;
+};
+
+struct SCOPED_CAPABILITY UnnecessaryLock {
+    explicit UnnecessaryLock(Mutex& mutex) ACQUIRE(mutex) {}
+    ~UnnecessaryLock() RELEASE() {}
+};
+
 // TODO(b/141333600): Consolidate with HWC2::Display::Config::Builder::getDefaultDensity.
 constexpr float FALLBACK_DENSITY = ACONFIGURATION_DENSITY_TV;
 
@@ -991,7 +1025,7 @@
     }
 
     auto future = schedule([=]() -> status_t {
-        const auto display = getDisplayDeviceLocked(displayToken);
+        const auto display = ON_MAIN_THREAD(getDisplayDeviceLocked(displayToken));
         if (!display) {
             ALOGE("Attempt to set allowed display configs for invalid display token %p",
                   displayToken.get());
@@ -1176,7 +1210,7 @@
 }
 
 status_t SurfaceFlinger::setActiveColorMode(const sp<IBinder>& displayToken, ColorMode mode) {
-    schedule([=] {
+    schedule([=]() MAIN_THREAD {
         Vector<ColorMode> modes;
         getDisplayColorModes(displayToken, &modes);
         bool exists = std::find(std::begin(modes), std::end(modes), mode) != std::end(modes);
@@ -1222,7 +1256,7 @@
 }
 
 void SurfaceFlinger::setAutoLowLatencyMode(const sp<IBinder>& displayToken, bool on) {
-    static_cast<void>(schedule([=] {
+    static_cast<void>(schedule([=]() MAIN_THREAD {
         if (const auto displayId = getPhysicalDisplayIdLocked(displayToken)) {
             getHwComposer().setAutoLowLatencyMode(*displayId, on);
         } else {
@@ -1253,7 +1287,7 @@
 }
 
 void SurfaceFlinger::setGameContentType(const sp<IBinder>& displayToken, bool on) {
-    static_cast<void>(schedule([=] {
+    static_cast<void>(schedule([=]() MAIN_THREAD {
         if (const auto displayId = getPhysicalDisplayIdLocked(displayToken)) {
             const auto type = on ? hal::ContentType::GAME : hal::ContentType::NONE;
             getHwComposer().setContentType(*displayId, type);
@@ -1343,7 +1377,7 @@
 status_t SurfaceFlinger::setDisplayContentSamplingEnabled(const sp<IBinder>& displayToken,
                                                           bool enable, uint8_t componentMask,
                                                           uint64_t maxFrames) {
-    return schedule([=]() -> status_t {
+    return schedule([=]() MAIN_THREAD -> status_t {
                if (const auto displayId = getPhysicalDisplayIdLocked(displayToken)) {
                    return getHwComposer().setDisplayContentSamplingEnabled(*displayId, enable,
                                                                            componentMask,
@@ -1412,13 +1446,9 @@
     return mScheduler->injectVSync(when, calculateExpectedPresentTime(when)) ? NO_ERROR : BAD_VALUE;
 }
 
-status_t SurfaceFlinger::getLayerDebugInfo(std::vector<LayerDebugInfo>* outLayers) const
-        NO_THREAD_SAFETY_ANALYSIS {
-    // Try to acquire a lock for 1s, fail gracefully
-    const status_t err = mStateLock.timedLock(s2ns(1));
-    const bool locked = (err == NO_ERROR);
-    if (!locked) {
-        ALOGE("LayerDebugInfo: SurfaceFlinger unresponsive (%s [%d]) - exit", strerror(-err), err);
+status_t SurfaceFlinger::getLayerDebugInfo(std::vector<LayerDebugInfo>* outLayers) const {
+    TimedLock lock(mStateLock, s2ns(1), __FUNCTION__);
+    if (!lock.locked()) {
         return TIMED_OUT;
     }
 
@@ -1427,7 +1457,6 @@
     mCurrentState.traverseInZOrder(
             [&](Layer* layer) { outLayers->push_back(layer->getLayerDebugInfo(display.get())); });
 
-    mStateLock.unlock();
     return NO_ERROR;
 }
 
@@ -1484,7 +1513,7 @@
         return BAD_VALUE;
     }
 
-    return promise::chain(schedule([=] {
+    return promise::chain(schedule([=]() MAIN_THREAD {
                if (const auto displayId = getPhysicalDisplayIdLocked(displayToken)) {
                    return getHwComposer().setDisplayBrightness(*displayId, brightness);
                } else {
@@ -1597,7 +1626,7 @@
 }
 
 void SurfaceFlinger::onHotplugReceived(int32_t sequenceId, hal::HWDisplayId hwcDisplayId,
-                                       hal::Connection connection) NO_THREAD_SAFETY_ANALYSIS {
+                                       hal::Connection connection) {
     ALOGV("%s(%d, %" PRIu64 ", %s)", __FUNCTION__, sequenceId, hwcDisplayId,
           connection == hal::Connection::CONNECTED ? "connected" : "disconnected");
 
@@ -1650,8 +1679,7 @@
 
     // Enable / Disable HWVsync from the main thread to avoid race conditions with
     // display power state.
-    static_cast<void>(
-            schedule([=]() NO_THREAD_SAFETY_ANALYSIS { setPrimaryVsyncEnabledInternal(enabled); }));
+    static_cast<void>(schedule([=]() MAIN_THREAD { setPrimaryVsyncEnabledInternal(enabled); }));
 }
 
 void SurfaceFlinger::setPrimaryVsyncEnabledInternal(bool enabled) {
@@ -1667,7 +1695,6 @@
     }
 }
 
-// Note: it is assumed the caller holds |mStateLock| when this is called
 void SurfaceFlinger::resetDisplayState() {
     mScheduler->disableHardwareVsync(true);
     // Clear the drawing state so that the logic inside of
@@ -1760,7 +1787,7 @@
     setTransactionFlags(eDisplayTransactionNeeded);
 }
 
-sp<Fence> SurfaceFlinger::previousFrameFence() NO_THREAD_SAFETY_ANALYSIS {
+sp<Fence> SurfaceFlinger::previousFrameFence() {
     // We are storing the last 2 present fences. If sf's phase offset is to be
     // woken up before the actual vsync but targeting the next vsync, we need to check
     // fence N-2
@@ -1768,7 +1795,7 @@
                                                 : mPreviousPresentFences[1];
 }
 
-bool SurfaceFlinger::previousFramePending(int graceTimeMs) NO_THREAD_SAFETY_ANALYSIS {
+bool SurfaceFlinger::previousFramePending(int graceTimeMs) {
     ATRACE_CALL();
     const sp<Fence>& fence = previousFrameFence();
 
@@ -1782,7 +1809,7 @@
     return status == -ETIME;
 }
 
-nsecs_t SurfaceFlinger::previousFramePresentTime() NO_THREAD_SAFETY_ANALYSIS {
+nsecs_t SurfaceFlinger::previousFramePresentTime() {
     const sp<Fence>& fence = previousFrameFence();
 
     if (fence == Fence::NO_FENCE) {
@@ -1800,8 +1827,7 @@
     return mVSyncModulator->getOffsets().sf > 0 ? presentTime : presentTime + stats.vsyncPeriod;
 }
 
-void SurfaceFlinger::onMessageReceived(int32_t what,
-                                       nsecs_t expectedVSyncTime) NO_THREAD_SAFETY_ANALYSIS {
+void SurfaceFlinger::onMessageReceived(int32_t what, nsecs_t expectedVSyncTime) {
     ATRACE_CALL();
     switch (what) {
         case MessageQueue::INVALIDATE: {
@@ -1815,7 +1841,7 @@
     }
 }
 
-void SurfaceFlinger::onMessageInvalidate(nsecs_t expectedVSyncTime) NO_THREAD_SAFETY_ANALYSIS {
+void SurfaceFlinger::onMessageInvalidate(nsecs_t expectedVSyncTime) {
     ATRACE_CALL();
 
     const nsecs_t frameStart = systemTime();
@@ -1888,7 +1914,7 @@
         // We received the present fence from the HWC, so we assume it successfully updated
         // the config, hence we update SF.
         mSetActiveConfigPending = false;
-        setActiveConfigInternal();
+        ON_MAIN_THREAD(setActiveConfigInternal());
     }
 
     if (framePending && mPropagateBackpressure) {
@@ -1907,12 +1933,12 @@
             std::chrono::duration_cast<std::chrono::nanoseconds>(1s).count();
     // If we're in a user build then don't push any atoms
     if (!mIsUserBuild && mMissedFrameJankCount > 0) {
-        const auto displayDevice = getDefaultDisplayDeviceLocked();
+        const auto display = ON_MAIN_THREAD(getDefaultDisplayDeviceLocked());
         // Only report jank when the display is on, as displays in DOZE
         // power mode may operate at a different frame rate than is
         // reported in their config, which causes noticeable (but less
         // severe) jank.
-        if (displayDevice && displayDevice->getPowerMode() == hal::PowerMode::ON) {
+        if (display && display->getPowerMode() == hal::PowerMode::ON) {
             const nsecs_t currentTime = systemTime();
             const nsecs_t jankDuration = currentTime - mMissedFrameJankStart;
             if (jankDuration > kMinJankyDuration && jankDuration < kMaxJankyDuration) {
@@ -1969,7 +1995,7 @@
         mScheduler->chooseRefreshRateForContent();
     }
 
-    performSetActiveConfig();
+    ON_MAIN_THREAD(performSetActiveConfig());
 
     updateCursorAsync();
     updateInputFlinger();
@@ -2018,8 +2044,9 @@
     mRefreshPending = false;
 
     compositionengine::CompositionRefreshArgs refreshArgs;
-    refreshArgs.outputs.reserve(mDisplays.size());
-    for (const auto& [_, display] : mDisplays) {
+    const auto& displays = ON_MAIN_THREAD(mDisplays);
+    refreshArgs.outputs.reserve(displays.size());
+    for (const auto& [_, display] : displays) {
         refreshArgs.outputs.push_back(display->getCompositionDisplay());
     }
     mDrawingState.traverseInZOrder([&refreshArgs](Layer* layer) {
@@ -2074,21 +2101,18 @@
 
     const bool prevFrameHadDeviceComposition = mHadDeviceComposition;
 
-    mHadClientComposition =
-            std::any_of(mDisplays.cbegin(), mDisplays.cend(), [](const auto& tokenDisplayPair) {
-                auto& displayDevice = tokenDisplayPair.second;
-                return displayDevice->getCompositionDisplay()->getState().usesClientComposition &&
-                        !displayDevice->getCompositionDisplay()->getState().reusedClientComposition;
-            });
-    mHadDeviceComposition =
-            std::any_of(mDisplays.cbegin(), mDisplays.cend(), [](const auto& tokenDisplayPair) {
-                auto& displayDevice = tokenDisplayPair.second;
-                return displayDevice->getCompositionDisplay()->getState().usesDeviceComposition;
-            });
+    mHadClientComposition = std::any_of(displays.cbegin(), displays.cend(), [](const auto& pair) {
+        const auto& state = pair.second->getCompositionDisplay()->getState();
+        return state.usesClientComposition && !state.reusedClientComposition;
+    });
+    mHadDeviceComposition = std::any_of(displays.cbegin(), displays.cend(), [](const auto& pair) {
+        const auto& state = pair.second->getCompositionDisplay()->getState();
+        return state.usesDeviceComposition;
+    });
     mReusedClientComposition =
-            std::any_of(mDisplays.cbegin(), mDisplays.cend(), [](const auto& tokenDisplayPair) {
-                auto& displayDevice = tokenDisplayPair.second;
-                return displayDevice->getCompositionDisplay()->getState().reusedClientComposition;
+            std::any_of(displays.cbegin(), displays.cend(), [](const auto& pair) {
+                const auto& state = pair.second->getCompositionDisplay()->getState();
+                return state.reusedClientComposition;
             });
 
     // Only report a strategy change if we move in and out of composition with hw overlays
@@ -2194,14 +2218,14 @@
     for (auto& layer : mLayersWithQueuedFrames) {
         layer->releasePendingBuffer(dequeueReadyTime);
     }
-    // |mStateLock| not needed as we are on the main thread
-    const auto displayDevice = getDefaultDisplayDeviceLocked();
+
+    const auto* display = ON_MAIN_THREAD(getDefaultDisplayDeviceLocked()).get();
 
     getBE().mGlCompositionDoneTimeline.updateSignalTimes();
     std::shared_ptr<FenceTime> glCompositionDoneFenceTime;
-    if (displayDevice && displayDevice->getCompositionDisplay()->getState().usesClientComposition) {
+    if (display && display->getCompositionDisplay()->getState().usesClientComposition) {
         glCompositionDoneFenceTime =
-                std::make_shared<FenceTime>(displayDevice->getCompositionDisplay()
+                std::make_shared<FenceTime>(display->getCompositionDisplay()
                                                     ->getRenderSurface()
                                                     ->getClientTargetAcquireFence());
         getBE().mGlCompositionDoneTimeline.push(glCompositionDoneFenceTime);
@@ -2211,9 +2235,8 @@
 
     getBE().mDisplayTimeline.updateSignalTimes();
     mPreviousPresentFences[1] = mPreviousPresentFences[0];
-    mPreviousPresentFences[0] = displayDevice
-            ? getHwComposer().getPresentFence(*displayDevice->getId())
-            : Fence::NO_FENCE;
+    mPreviousPresentFences[0] =
+            display ? getHwComposer().getPresentFence(*display->getId()) : Fence::NO_FENCE;
     auto presentFenceTime = std::make_shared<FenceTime>(mPreviousPresentFences[0]);
     getBE().mDisplayTimeline.push(presentFenceTime);
 
@@ -2232,9 +2255,8 @@
     }
 
     mDrawingState.traverse([&](Layer* layer) {
-        const bool frameLatched =
-                layer->onPostComposition(displayDevice.get(), glCompositionDoneFenceTime,
-                                         presentFenceTime, compositorTiming);
+        const bool frameLatched = layer->onPostComposition(display, glCompositionDoneFenceTime,
+                                                           presentFenceTime, compositorTiming);
         if (frameLatched) {
             recordBufferingStats(layer->getName(), layer->getOccupancyHistory(false));
         }
@@ -2243,14 +2265,15 @@
     mTransactionCompletedThread.addPresentFence(mPreviousPresentFences[0]);
     mTransactionCompletedThread.sendCallbacks();
 
-    if (displayDevice && displayDevice->isPrimary() &&
-        displayDevice->getPowerMode() == hal::PowerMode::ON && presentFenceTime->isValid()) {
+    if (display && display->isPrimary() && display->getPowerMode() == hal::PowerMode::ON &&
+        presentFenceTime->isValid()) {
         mScheduler->addPresentFence(presentFenceTime);
     }
 
+    const bool isDisplayConnected = display && getHwComposer().isConnected(*display->getId());
+
     if (!hasSyncFramework) {
-        if (displayDevice && getHwComposer().isConnected(*displayDevice->getId()) &&
-            displayDevice->isPoweredOn()) {
+        if (isDisplayConnected && display->isPoweredOn()) {
             mScheduler->enableHardwareVsync();
         }
     }
@@ -2261,11 +2284,10 @@
         if (presentFenceTime->isValid()) {
             mAnimFrameTracker.setActualPresentFence(
                     std::move(presentFenceTime));
-        } else if (displayDevice && getHwComposer().isConnected(*displayDevice->getId())) {
+        } else if (isDisplayConnected) {
             // The HWC doesn't support present fences, so use the refresh
             // timestamp instead.
-            const nsecs_t presentTime =
-                    getHwComposer().getRefreshTimestamp(*displayDevice->getId());
+            const nsecs_t presentTime = getHwComposer().getRefreshTimestamp(*display->getId());
             mAnimFrameTracker.setActualPresentTime(presentTime);
         }
         mAnimFrameTracker.advanceFrame();
@@ -2286,8 +2308,7 @@
     const size_t appConnections = mScheduler->getEventThreadConnectionCount(mAppConnectionHandle);
     mTimeStats->recordDisplayEventConnectionCount(sfConnections + appConnections);
 
-    if (displayDevice && getHwComposer().isConnected(*displayDevice->getId()) &&
-        !displayDevice->isPoweredOn()) {
+    if (isDisplayConnected && !display->isPoweredOn()) {
         return;
     }
 
@@ -2343,7 +2364,7 @@
 }
 
 void SurfaceFlinger::computeLayerBounds() {
-    for (const auto& pair : mDisplays) {
+    for (const auto& pair : ON_MAIN_THREAD(mDisplays)) {
         const auto& displayDevice = pair.second;
         const auto display = displayDevice->getCompositionDisplay();
         for (const auto& layer : mDrawingState.layersSortedByZ) {
@@ -2358,10 +2379,8 @@
     }
 }
 
-void SurfaceFlinger::postFrame()
-{
-    // |mStateLock| not needed as we are on the main thread
-    const auto display = getDefaultDisplayDeviceLocked();
+void SurfaceFlinger::postFrame() {
+    const auto display = ON_MAIN_THREAD(getDefaultDisplayDeviceLocked());
     if (display && getHwComposer().isConnected(*display->getId())) {
         uint32_t flipCount = display->getPageFlipCount();
         if (flipCount % LOG_FRAME_STATS_PERIOD == 0) {
@@ -2789,7 +2808,7 @@
         sp<const DisplayDevice> hintDisplay;
         uint32_t currentlayerStack = 0;
         bool first = true;
-        mCurrentState.traverse([&](Layer* layer) {
+        mCurrentState.traverse([&](Layer* layer) REQUIRES(mStateLock) {
             // NOTE: we rely on the fact that layers are sorted by
             // layerStack first (so we don't have to traverse the list
             // of displays for every layer).
@@ -2902,10 +2921,9 @@
     mPendingInputWindowCommands.clear();
 }
 
-void SurfaceFlinger::updateCursorAsync()
-{
+void SurfaceFlinger::updateCursorAsync() {
     compositionengine::CompositionRefreshArgs refreshArgs;
-    for (const auto& [_, display] : mDisplays) {
+    for (const auto& [_, display] : ON_MAIN_THREAD(mDisplays)) {
         if (display->getId()) {
             refreshArgs.outputs.push_back(display->getCompositionDisplay());
         }
@@ -2915,7 +2933,7 @@
 }
 
 void SurfaceFlinger::changeRefreshRate(const RefreshRate& refreshRate,
-                                       Scheduler::ConfigEvent event) NO_THREAD_SAFETY_ANALYSIS {
+                                       Scheduler::ConfigEvent event) {
     // If this is called from the main thread mStateLock must be locked before
     // Currently the only way to call this function from the main thread is from
     // Sheduler::chooseRefreshRateForContent
@@ -3041,7 +3059,7 @@
 }
 
 void SurfaceFlinger::invalidateLayerStack(const sp<const Layer>& layer, const Region& dirty) {
-    for (const auto& [token, displayDevice] : mDisplays) {
+    for (const auto& [token, displayDevice] : ON_MAIN_THREAD(mDisplays)) {
         auto display = displayDevice->getCompositionDisplay();
         if (display->belongsInOutput(layer->getLayerStack(), layer->getPrimaryDisplayOnly())) {
             display->editState().dirtyRegion.orSelf(dirty);
@@ -4137,7 +4155,7 @@
 
 void SurfaceFlinger::initializeDisplays() {
     // Async since we may be called from the main thread.
-    static_cast<void>(schedule([this]() NO_THREAD_SAFETY_ANALYSIS { onInitializeDisplays(); }));
+    static_cast<void>(schedule([this]() MAIN_THREAD { onInitializeDisplays(); }));
 }
 
 void SurfaceFlinger::setVsyncEnabledInHWC(DisplayId displayId, hal::Vsync enabled) {
@@ -4228,7 +4246,7 @@
 }
 
 void SurfaceFlinger::setPowerMode(const sp<IBinder>& displayToken, int mode) {
-    schedule([=]() NO_THREAD_SAFETY_ANALYSIS {
+    schedule([=]() MAIN_THREAD {
         const auto display = getDisplayDeviceLocked(displayToken);
         if (!display) {
             ALOGE("Attempt to set power mode %d for invalid display token %p", mode,
@@ -4241,8 +4259,7 @@
     }).wait();
 }
 
-status_t SurfaceFlinger::doDump(int fd, const DumpArgs& args,
-                                bool asProto) NO_THREAD_SAFETY_ANALYSIS {
+status_t SurfaceFlinger::doDump(int fd, const DumpArgs& args, bool asProto) {
     std::string result;
 
     IPCThreadState* ipc = IPCThreadState::self();
@@ -4254,18 +4271,6 @@
         StringAppendF(&result, "Permission Denial: can't dump SurfaceFlinger from pid=%d, uid=%d\n",
                       pid, uid);
     } else {
-        // Try to get the main lock, but give up after one second
-        // (this would indicate SF is stuck, but we want to be able to
-        // print something in dumpsys).
-        status_t err = mStateLock.timedLock(s2ns(1));
-        bool locked = (err == NO_ERROR);
-        if (!locked) {
-            StringAppendF(&result,
-                          "SurfaceFlinger appears to be unresponsive (%s [%d]), dumping anyways "
-                          "(no locks held)\n",
-                          strerror(-err), err);
-        }
-
         static const std::unordered_map<std::string, Dumper> dumpers = {
                 {"--display-id"s, dumper(&SurfaceFlinger::dumpDisplayIdentificationData)},
                 {"--dispsync"s,
@@ -4283,18 +4288,23 @@
 
         const auto flag = args.empty() ? ""s : std::string(String8(args[0]));
 
-        const auto it = dumpers.find(flag);
-        if (it != dumpers.end()) {
-            (it->second)(args, asProto, result);
-        } else if (!asProto) {
-            dumpAllLocked(args, result);
+        bool dumpLayers = true;
+        {
+            TimedLock lock(mStateLock, s2ns(1), __FUNCTION__);
+            if (!lock.locked()) {
+                StringAppendF(&result, "Dumping without lock after timeout: %s (%d)\n",
+                              strerror(-lock.status), lock.status);
+            }
+
+            if (const auto it = dumpers.find(flag); it != dumpers.end()) {
+                (it->second)(args, asProto, result);
+                dumpLayers = false;
+            } else if (!asProto) {
+                dumpAllLocked(args, result);
+            }
         }
 
-        if (locked) {
-            mStateLock.unlock();
-        }
-
-        if (it == dumpers.end()) {
+        if (dumpLayers) {
             const LayersProto layersProto = dumpProtoFromMainThread();
             if (asProto) {
                 result.append(layersProto.SerializeAsString());
@@ -4568,7 +4578,7 @@
 
 LayersProto SurfaceFlinger::dumpDrawingStateProto(uint32_t traceFlags) const {
     // If context is SurfaceTracing thread, mTracingLock blocks display transactions on main thread.
-    const auto display = getDefaultDisplayDeviceLocked();
+    const auto display = ON_MAIN_THREAD(getDefaultDisplayDeviceLocked());
 
     LayersProto layersProto;
     for (const sp<Layer>& layer : mDrawingState.layersSortedByZ) {
@@ -5404,7 +5414,7 @@
     return NO_ERROR;
 }
 
-const sp<DisplayDevice> SurfaceFlinger::getDisplayByIdOrLayerStack(uint64_t displayOrLayerStack) {
+sp<DisplayDevice> SurfaceFlinger::getDisplayByIdOrLayerStack(uint64_t displayOrLayerStack) {
     const sp<IBinder> displayToken = getPhysicalDisplayTokenLocked(DisplayId{displayOrLayerStack});
     if (displayToken) {
         return getDisplayDeviceLocked(displayToken);
@@ -5414,7 +5424,7 @@
     return getDisplayByLayerStack(displayOrLayerStack);
 }
 
-const sp<DisplayDevice> SurfaceFlinger::getDisplayByLayerStack(uint64_t layerStack) {
+sp<DisplayDevice> SurfaceFlinger::getDisplayByLayerStack(uint64_t layerStack) {
     for (const auto& [token, display] : mDisplays) {
         if (display->getLayerStack() == layerStack) {
             return display;
@@ -5989,7 +5999,7 @@
     }
 
     auto future = schedule([=]() -> status_t {
-        const auto display = getDisplayDeviceLocked(displayToken);
+        const auto display = ON_MAIN_THREAD(getDisplayDeviceLocked(displayToken));
         if (!display) {
             ALOGE("Attempt to set desired display configs for invalid display token %p",
                   displayToken.get());
@@ -6144,7 +6154,7 @@
         return BAD_VALUE;
     }
 
-    static_cast<void>(schedule([=]() NO_THREAD_SAFETY_ANALYSIS {
+    static_cast<void>(schedule([=] {
         Mutex::Autolock lock(mStateLock);
         if (authenticateSurfaceTextureLocked(surface)) {
             sp<Layer> layer = (static_cast<MonitoredProducer*>(surface.get()))->getLayer();
@@ -6173,8 +6183,7 @@
         sp<IBinder> token;
 
         if (mFrameRateFlexibilityTokenCount == 0) {
-            // |mStateLock| not needed as we are on the main thread
-            const auto display = getDefaultDisplayDeviceLocked();
+            const auto display = ON_MAIN_THREAD(getDefaultDisplayDeviceLocked());
 
             // This is a little racy, but not in a way that hurts anything. As we grab the
             // defaultConfig from the display manager policy, we could be setting a new display
@@ -6224,8 +6233,7 @@
         mFrameRateFlexibilityTokenCount--;
         ALOGD("Frame rate flexibility token released. count=%d", mFrameRateFlexibilityTokenCount);
         if (mFrameRateFlexibilityTokenCount == 0) {
-            // |mStateLock| not needed as we are on the main thread
-            const auto display = getDefaultDisplayDeviceLocked();
+            const auto display = ON_MAIN_THREAD(getDefaultDisplayDeviceLocked());
             constexpr bool kOverridePolicy = true;
             status_t result = setDesiredDisplayConfigSpecsInternal(display, {}, kOverridePolicy);
             LOG_ALWAYS_FATAL_IF(result < 0, "Failed releasing frame rate flexibility token");