SF: Fix thread safety for RefreshRateOverlay
Lock mStateLock when reading/writing the pointer, and writing layer
state. Destroy the layer on the main thread. Notify overlay when
viewport changes to avoid display lookup on refresh rate change.
Bug: 123715322
Test: Toggle overlay on flame.
Change-Id: I019c5cd49c94182f2c4364b0299fa3aa7783bd15
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 87e73a0..bc87887 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -582,14 +582,13 @@
LOG_EVENT_LONG(LOGTAG_SF_STOP_BOOTANIM,
ns2ms(systemTime(SYSTEM_TIME_MONOTONIC)));
- static_cast<void>(schedule([this]() NO_THREAD_SAFETY_ANALYSIS {
+ static_cast<void>(schedule([this] {
readPersistentProperties();
mPowerAdvisor.onBootFinished();
mBootStage = BootStage::FINISHED;
if (property_get_bool("sf.debug.show_refresh_rate_overlay", false)) {
- mRefreshRateOverlay = std::make_unique<RefreshRateOverlay>(*this);
- mRefreshRateOverlay->changeRefreshRate(mRefreshRateConfigs->getCurrentRefreshRate());
+ enableRefreshRateOverlay(true);
}
}));
}
@@ -934,9 +933,8 @@
}
if (isPrimary) {
- std::lock_guard<std::mutex> lock(mActiveConfigLock);
- if (mDesiredActiveConfigChanged) {
- return mDesiredActiveConfig.configId.value();
+ if (const auto config = getDesiredActiveConfig()) {
+ return config->configId.value();
}
}
@@ -1063,14 +1061,7 @@
ATRACE_CALL();
ALOGV("performSetActiveConfig");
// Store the local variable to release the lock.
- const auto desiredActiveConfig = [&]() -> std::optional<ActiveConfigInfo> {
- std::lock_guard<std::mutex> lock(mActiveConfigLock);
- if (mDesiredActiveConfigChanged) {
- return mDesiredActiveConfig;
- }
- return std::nullopt;
- }();
-
+ const auto desiredActiveConfig = getDesiredActiveConfig();
if (!desiredActiveConfig) {
// No desired active config pending to be applied
return;
@@ -1605,7 +1596,7 @@
}
void SurfaceFlinger::onHotplugReceived(int32_t sequenceId, hal::HWDisplayId hwcDisplayId,
- hal::Connection connection) {
+ hal::Connection connection) NO_THREAD_SAFETY_ANALYSIS {
ALOGV("%s(%d, %" PRIu64 ", %s)", __FUNCTION__, sequenceId, hwcDisplayId,
connection == hal::Connection::CONNECTED ? "connected" : "disconnected");
@@ -2679,9 +2670,14 @@
if (currentState.width != drawingState.width ||
currentState.height != drawingState.height) {
display->setDisplaySize(currentState.width, currentState.height);
+
if (display->isPrimary()) {
mScheduler->onPrimaryDisplayAreaChanged(currentState.width * currentState.height);
}
+
+ if (mRefreshRateOverlay) {
+ mRefreshRateOverlay->setViewport(display->getSize());
+ }
}
}
}
@@ -5229,15 +5225,15 @@
return NO_ERROR;
}
case 1034: {
- n = data.readInt32();
- if (n == 1 && !mRefreshRateOverlay) {
- mRefreshRateOverlay = std::make_unique<RefreshRateOverlay>(*this);
- auto& current = mRefreshRateConfigs->getCurrentRefreshRate();
- mRefreshRateOverlay->changeRefreshRate(current);
- } else if (n == 0) {
- mRefreshRateOverlay.reset();
- } else {
- reply->writeBool(mRefreshRateOverlay != nullptr);
+ switch (n = data.readInt32()) {
+ case 0:
+ case 1:
+ enableRefreshRateOverlay(static_cast<bool>(n));
+ break;
+ default: {
+ Mutex::Autolock lock(mStateLock);
+ reply->writeBool(mRefreshRateOverlay != nullptr);
+ }
}
return NO_ERROR;
}
@@ -5285,29 +5281,26 @@
void SurfaceFlinger::kernelTimerChanged(bool expired) {
static bool updateOverlay =
property_get_bool("debug.sf.kernel_idle_timer_update_overlay", true);
- if (!updateOverlay || !mRefreshRateOverlay) return;
+ if (!updateOverlay) return;
+ if (Mutex::Autolock lock(mStateLock); !mRefreshRateOverlay) return;
// Update the overlay on the main thread to avoid race conditions with
// mRefreshRateConfigs->getCurrentRefreshRate()
- static_cast<void>(schedule([=]() NO_THREAD_SAFETY_ANALYSIS {
- if (mRefreshRateOverlay) {
+ static_cast<void>(schedule([=] {
+ const auto desiredActiveConfig = getDesiredActiveConfig();
+ const auto& current = desiredActiveConfig
+ ? mRefreshRateConfigs->getRefreshRateFromConfigId(desiredActiveConfig->configId)
+ : mRefreshRateConfigs->getCurrentRefreshRate();
+ const auto& min = mRefreshRateConfigs->getMinRefreshRate();
+
+ if (current != min) {
const auto kernelTimerEnabled = property_get_bool(KERNEL_IDLE_TIMER_PROP, false);
const bool timerExpired = kernelTimerEnabled && expired;
- const auto& current = [this]() -> const RefreshRate& {
- std::lock_guard<std::mutex> lock(mActiveConfigLock);
- if (mDesiredActiveConfigChanged) {
- return mRefreshRateConfigs->getRefreshRateFromConfigId(
- mDesiredActiveConfig.configId);
- }
- return mRefreshRateConfigs->getCurrentRefreshRate();
- }();
- const auto& min = mRefreshRateConfigs->getMinRefreshRate();
-
- if (current != min) {
+ if (Mutex::Autolock lock(mStateLock); mRefreshRateOverlay) {
mRefreshRateOverlay->changeRefreshRate(timerExpired ? min : current);
- mEventQueue->invalidate();
}
+ mEventQueue->invalidate();
}
}));
}
@@ -6224,6 +6217,29 @@
}));
}
+void SurfaceFlinger::enableRefreshRateOverlay(bool enable) {
+ static_cast<void>(schedule([=] {
+ std::unique_ptr<RefreshRateOverlay> overlay;
+ if (enable) {
+ overlay = std::make_unique<RefreshRateOverlay>(*this);
+ }
+
+ {
+ Mutex::Autolock lock(mStateLock);
+
+ // Destroy the layer of the current overlay, if any, outside the lock.
+ mRefreshRateOverlay.swap(overlay);
+ if (!mRefreshRateOverlay) return;
+
+ if (const auto display = getDefaultDisplayDeviceLocked()) {
+ mRefreshRateOverlay->setViewport(display->getSize());
+ }
+
+ mRefreshRateOverlay->changeRefreshRate(mRefreshRateConfigs->getCurrentRefreshRate());
+ }
+ }));
+}
+
} // namespace android
#if defined(__gl_h_)