SF: Enforce thread safety of active mode access
Provide two RefreshRateConfigs APIs for retrieving the active mode:
getActiveMode for access from the main thread (which does not need
to lock nor copy), and getActiveModePtr for other threads.
Bug: 241285191
Test: Build (-Wthread-safety)
Change-Id: If156d85861ec2d82a394ba181314a6ba3048974f
diff --git a/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp b/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
index a48c921..d270655 100644
--- a/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
+++ b/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
@@ -27,6 +27,7 @@
#include <android-base/properties.h>
#include <android-base/stringprintf.h>
#include <ftl/enum.h>
+#include <ftl/fake_guard.h>
#include <utils/Trace.h>
#include "../SurfaceFlingerProperties.h"
@@ -325,6 +326,8 @@
const Policy* policy = getCurrentPolicyLocked();
const auto& defaultMode = mDisplayModes.get(policy->defaultMode)->get();
+ const auto& activeMode = *getActiveModeItLocked()->second;
+
// If the default mode group is different from the group of current mode,
// this means a layer requesting a seamed mode switch just disappeared and
// we should switch back to the default group.
@@ -332,7 +335,7 @@
// of the current mode, in order to prevent unnecessary seamed mode switches
// (e.g. when pausing a video playback).
const auto anchorGroup =
- seamedFocusedLayers > 0 ? mActiveModeIt->second->getGroup() : defaultMode->getGroup();
+ seamedFocusedLayers > 0 ? activeMode.getGroup() : defaultMode->getGroup();
// Consider the touch event if there are no Explicit* layers. Otherwise wait until after we've
// selected a refresh rate to see if we should apply touch boost.
@@ -387,12 +390,12 @@
for (auto& [modeIt, overallScore, fixedRateBelowThresholdLayersScore] : scores) {
const auto& [id, mode] = *modeIt;
- const bool isSeamlessSwitch = mode->getGroup() == mActiveModeIt->second->getGroup();
+ const bool isSeamlessSwitch = mode->getGroup() == activeMode.getGroup();
if (layer.seamlessness == Seamlessness::OnlySeamless && !isSeamlessSwitch) {
ALOGV("%s ignores %s to avoid non-seamless switch. Current mode = %s",
formatLayerInfo(layer, weight).c_str(), to_string(*mode).c_str(),
- to_string(*mActiveModeIt->second).c_str());
+ to_string(activeMode).c_str());
continue;
}
@@ -401,7 +404,7 @@
ALOGV("%s ignores %s because it's not focused and the switch is going to be seamed."
" Current mode = %s",
formatLayerInfo(layer, weight).c_str(), to_string(*mode).c_str(),
- to_string(*mActiveModeIt->second).c_str());
+ to_string(activeMode).c_str());
continue;
}
@@ -413,7 +416,7 @@
const bool isInPolicyForDefault = mode->getGroup() == anchorGroup;
if (layer.seamlessness == Seamlessness::Default && !isInPolicyForDefault) {
ALOGV("%s ignores %s. Current mode = %s", formatLayerInfo(layer, weight).c_str(),
- to_string(*mode).c_str(), to_string(*mActiveModeIt->second).c_str());
+ to_string(*mode).c_str(), to_string(activeMode).c_str());
continue;
}
@@ -676,7 +679,7 @@
const DisplayModePtr& current = desiredActiveModeId
? mDisplayModes.get(*desiredActiveModeId)->get()
- : mActiveModeIt->second;
+ : getActiveModeItLocked()->second;
const DisplayModePtr& min = mMinRefreshRateModeIt->second;
if (current == min) {
@@ -688,16 +691,17 @@
}
const DisplayModePtr& RefreshRateConfigs::getMinRefreshRateByPolicyLocked() const {
+ const auto& activeMode = *getActiveModeItLocked()->second;
+
for (const DisplayModeIterator modeIt : mPrimaryRefreshRates) {
const auto& mode = modeIt->second;
- if (mActiveModeIt->second->getGroup() == mode->getGroup()) {
+ if (activeMode.getGroup() == mode->getGroup()) {
return mode;
}
}
- ALOGE("Can't find min refresh rate by policy with the same mode group"
- " as the current mode %s",
- to_string(*mActiveModeIt->second).c_str());
+ ALOGE("Can't find min refresh rate by policy with the same mode group as the current mode %s",
+ to_string(activeMode).c_str());
// Default to the lowest refresh rate.
return mPrimaryRefreshRates.front()->second;
@@ -708,6 +712,11 @@
return getMaxRefreshRateByPolicyLocked();
}
+const DisplayModePtr& RefreshRateConfigs::getMaxRefreshRateByPolicyLocked() const {
+ const int anchorGroup = getActiveModeItLocked()->second->getGroup();
+ return getMaxRefreshRateByPolicyLocked(anchorGroup);
+}
+
const DisplayModePtr& RefreshRateConfigs::getMaxRefreshRateByPolicyLocked(int anchorGroup) const {
for (auto it = mPrimaryRefreshRates.rbegin(); it != mPrimaryRefreshRates.rend(); ++it) {
const auto& mode = (*it)->second;
@@ -716,17 +725,28 @@
}
}
- ALOGE("Can't find max refresh rate by policy with the same mode group"
- " as the current mode %s",
- to_string(*mActiveModeIt->second).c_str());
+ const auto& activeMode = *getActiveModeItLocked()->second;
+ ALOGE("Can't find max refresh rate by policy with the same mode group as the current mode %s",
+ to_string(activeMode).c_str());
// Default to the highest refresh rate.
return mPrimaryRefreshRates.back()->second;
}
-DisplayModePtr RefreshRateConfigs::getActiveMode() const {
+DisplayModePtr RefreshRateConfigs::getActiveModePtr() const {
std::lock_guard lock(mLock);
- return mActiveModeIt->second;
+ return getActiveModeItLocked()->second;
+}
+
+const DisplayMode& RefreshRateConfigs::getActiveMode() const {
+ // Reads from kMainThreadContext do not require mLock.
+ ftl::FakeGuard guard(mLock);
+ return *mActiveModeIt->second;
+}
+
+DisplayModeIterator RefreshRateConfigs::getActiveModeItLocked() const {
+ // Reads under mLock do not require kMainThreadContext.
+ return FTL_FAKE_GUARD(kMainThreadContext, mActiveModeIt);
}
void RefreshRateConfigs::setActiveModeId(DisplayModeId modeId) {
@@ -744,7 +764,7 @@
Config config)
: mKnownFrameRates(constructKnownFrameRates(modes)), mConfig(config) {
initializeIdleTimer();
- updateDisplayModes(std::move(modes), activeModeId);
+ FTL_FAKE_GUARD(kMainThreadContext, updateDisplayModes(std::move(modes), activeModeId));
}
void RefreshRateConfigs::initializeIdleTimer() {
@@ -976,7 +996,7 @@
std::lock_guard lock(mLock);
- const auto activeModeId = mActiveModeIt->first;
+ const auto activeModeId = getActiveModeItLocked()->first;
result += " activeModeId="s;
result += std::to_string(activeModeId.value());
diff --git a/services/surfaceflinger/Scheduler/RefreshRateConfigs.h b/services/surfaceflinger/Scheduler/RefreshRateConfigs.h
index a79002e..b2cfb03 100644
--- a/services/surfaceflinger/Scheduler/RefreshRateConfigs.h
+++ b/services/surfaceflinger/Scheduler/RefreshRateConfigs.h
@@ -31,6 +31,7 @@
#include "DisplayHardware/HWComposer.h"
#include "Scheduler/OneShotTimer.h"
#include "Scheduler/StrongTyping.h"
+#include "ThreadContext.h"
namespace android::scheduler {
@@ -207,8 +208,11 @@
// uses the primary range, not the app request range.
DisplayModePtr getMaxRefreshRateByPolicy() const EXCLUDES(mLock);
- void setActiveModeId(DisplayModeId) EXCLUDES(mLock);
- DisplayModePtr getActiveMode() const EXCLUDES(mLock);
+ void setActiveModeId(DisplayModeId) EXCLUDES(mLock) REQUIRES(kMainThreadContext);
+
+ // See mActiveModeIt for thread safety.
+ DisplayModePtr getActiveModePtr() const EXCLUDES(mLock);
+ const DisplayMode& getActiveMode() const REQUIRES(kMainThreadContext);
// Returns a known frame rate that is the closest to frameRate
Fps findClosestKnownFrameRate(Fps frameRate) const;
@@ -332,6 +336,9 @@
void constructAvailableRefreshRates() REQUIRES(mLock);
+ // See mActiveModeIt for thread safety.
+ DisplayModeIterator getActiveModeItLocked() const REQUIRES(mLock);
+
std::pair<DisplayModePtr, GlobalSignals> getBestRefreshRateLocked(
const std::vector<LayerRequirement>&, GlobalSignals) const REQUIRES(mLock);
@@ -345,10 +352,8 @@
// Returns the highest refresh rate according to the current policy. May change at runtime. Only
// uses the primary range, not the app request range.
+ const DisplayModePtr& getMaxRefreshRateByPolicyLocked() const REQUIRES(mLock);
const DisplayModePtr& getMaxRefreshRateByPolicyLocked(int anchorGroup) const REQUIRES(mLock);
- const DisplayModePtr& getMaxRefreshRateByPolicyLocked() const REQUIRES(mLock) {
- return getMaxRefreshRateByPolicyLocked(mActiveModeIt->second->getGroup());
- }
const Policy* getCurrentPolicyLocked() const REQUIRES(mLock);
bool isPolicyValidLocked(const Policy& policy) const REQUIRES(mLock);
@@ -361,7 +366,8 @@
float calculateNonExactMatchingLayerScoreLocked(const LayerRequirement&, Fps refreshRate) const
REQUIRES(mLock);
- void updateDisplayModes(DisplayModes, DisplayModeId activeModeId) EXCLUDES(mLock);
+ void updateDisplayModes(DisplayModes, DisplayModeId activeModeId) EXCLUDES(mLock)
+ REQUIRES(kMainThreadContext);
void initializeIdleTimer();
@@ -377,7 +383,10 @@
// is also dependent, so must be reset as well.
DisplayModes mDisplayModes GUARDED_BY(mLock);
- DisplayModeIterator mActiveModeIt GUARDED_BY(mLock);
+ // Written under mLock exclusively from kMainThreadContext, so reads from kMainThreadContext
+ // need not be under mLock.
+ DisplayModeIterator mActiveModeIt GUARDED_BY(mLock) GUARDED_BY(kMainThreadContext);
+
DisplayModeIterator mMinRefreshRateModeIt GUARDED_BY(mLock);
DisplayModeIterator mMaxRefreshRateModeIt GUARDED_BY(mLock);
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index 55ae013..bec39a7 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -182,7 +182,7 @@
impl::EventThread::GetVsyncPeriodFunction Scheduler::makeGetVsyncPeriodFunction() const {
return [this](uid_t uid) {
- const Fps refreshRate = holdRefreshRateConfigs()->getActiveMode()->getFps();
+ const Fps refreshRate = holdRefreshRateConfigs()->getActiveModePtr()->getFps();
const nsecs_t currentPeriod = mVsyncSchedule->period().ns() ?: refreshRate.getPeriodNsecs();
const auto frameRate = getFrameRateOverride(uid);
@@ -320,7 +320,7 @@
// mode change is in progress. In that case we shouldn't dispatch an event
// as it will be dispatched when the current mode changes.
if (std::scoped_lock lock(mRefreshRateConfigsLock);
- mRefreshRateConfigs->getActiveMode() != mPolicy.mode) {
+ mRefreshRateConfigs->getActiveModePtr() != mPolicy.mode) {
return;
}
@@ -453,7 +453,7 @@
if (now - last > kIgnoreDelay) {
const auto refreshRate = [&] {
std::scoped_lock lock(mRefreshRateConfigsLock);
- return mRefreshRateConfigs->getActiveMode()->getFps();
+ return mRefreshRateConfigs->getActiveModePtr()->getFps();
}();
resyncToHardwareVsync(false, refreshRate);
}
@@ -577,7 +577,7 @@
// magic number
const Fps refreshRate = [&] {
std::scoped_lock lock(mRefreshRateConfigsLock);
- return mRefreshRateConfigs->getActiveMode()->getFps();
+ return mRefreshRateConfigs->getActiveModePtr()->getFps();
}();
constexpr Fps FPS_THRESHOLD_FOR_KERNEL_TIMER = 65_Hz;
diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h
index eb3856d..afb3459 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.h
+++ b/services/surfaceflinger/Scheduler/Scheduler.h
@@ -229,7 +229,7 @@
nsecs_t getVsyncPeriodFromRefreshRateConfigs() const EXCLUDES(mRefreshRateConfigsLock) {
std::scoped_lock lock(mRefreshRateConfigsLock);
- return mRefreshRateConfigs->getActiveMode()->getFps().getPeriodNsecs();
+ return mRefreshRateConfigs->getActiveModePtr()->getFps().getPeriodNsecs();
}
// Returns the framerate of the layer with the given sequence ID