Rewrite concurrency logic in SurfaceFlinger PowerAdvisor
This patch aims to make concurrency safer by making sure all checks to
"hint session running" use mHintSession directly rather than a proxy
variable, and lock it. This requires all access to
"ensurePowerHintSessionRunning" to be locked, which requires refactoring
some methods, but should make the class much safer. This patch also aims
to ensure the session pointer is being directly checked for validity,
rather than relying on PowerHAL to always pass valid sessions.
Bug: 308030899
Test: atest libsurfaceflinger_unittest:PowerAdvisorTest
Change-Id: I0ebe55ed3c62c57cfd6fb94280beff4b9e8fcc13
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 2f7ff5d..b1598b4 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -721,10 +721,12 @@
}
readPersistentProperties();
- mPowerAdvisor->onBootFinished();
const bool hintSessionEnabled = FlagManager::getInstance().use_adpf_cpu_hint();
mPowerAdvisor->enablePowerHintSession(hintSessionEnabled);
const bool hintSessionUsed = mPowerAdvisor->usePowerHintSession();
+ // Ordering is important here, as onBootFinished signals to PowerAdvisor that concurrency
+ // is safe because its variables are initialized.
+ mPowerAdvisor->onBootFinished();
ALOGD("Power hint is %s",
hintSessionUsed ? "supported" : (hintSessionEnabled ? "unsupported" : "disabled"));
if (hintSessionUsed) {
@@ -734,7 +736,7 @@
if (renderEngineTid.has_value()) {
tidList.emplace_back(*renderEngineTid);
}
- if (!mPowerAdvisor->startPowerHintSession(tidList)) {
+ if (!mPowerAdvisor->startPowerHintSession(std::move(tidList))) {
ALOGW("Cannot start power hint session");
}
}