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/CompositionEngine/tests/MockPowerAdvisor.h b/services/surfaceflinger/CompositionEngine/tests/MockPowerAdvisor.h
index f74ef4c..7253354 100644
--- a/services/surfaceflinger/CompositionEngine/tests/MockPowerAdvisor.h
+++ b/services/surfaceflinger/CompositionEngine/tests/MockPowerAdvisor.h
@@ -38,11 +38,10 @@
MOCK_METHOD(void, notifyDisplayUpdateImminentAndCpuReset, (), (override));
MOCK_METHOD(bool, usePowerHintSession, (), (override));
MOCK_METHOD(bool, supportsPowerHintSession, (), (override));
- MOCK_METHOD(bool, ensurePowerHintSessionRunning, (), (override));
MOCK_METHOD(void, updateTargetWorkDuration, (Duration targetDuration), (override));
MOCK_METHOD(void, reportActualWorkDuration, (), (override));
MOCK_METHOD(void, enablePowerHintSession, (bool enabled), (override));
- MOCK_METHOD(bool, startPowerHintSession, (const std::vector<int32_t>& threadIds), (override));
+ MOCK_METHOD(bool, startPowerHintSession, (std::vector<int32_t> && threadIds), (override));
MOCK_METHOD(void, setGpuFenceTime,
(DisplayId displayId, std::unique_ptr<FenceTime>&& fenceTime), (override));
MOCK_METHOD(void, setHwcValidateTiming,
diff --git a/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp b/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp
index e005ad3..dd228b4 100644
--- a/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp
+++ b/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp
@@ -50,7 +50,6 @@
namespace impl {
using aidl::android::hardware::power::Boost;
-using aidl::android::hardware::power::IPowerHintSession;
using aidl::android::hardware::power::Mode;
using aidl::android::hardware::power::SessionHint;
using aidl::android::hardware::power::WorkDuration;
@@ -144,11 +143,13 @@
if (!mBootFinished.load()) {
return;
}
- if (usePowerHintSession() && ensurePowerHintSessionRunning()) {
+ if (usePowerHintSession()) {
std::lock_guard lock(mHintSessionMutex);
- auto ret = mHintSession->sendHint(SessionHint::CPU_LOAD_UP);
- if (!ret.isOk()) {
- mHintSessionRunning = false;
+ if (ensurePowerHintSessionRunning()) {
+ auto ret = mHintSession->sendHint(SessionHint::CPU_LOAD_UP);
+ if (!ret.isOk()) {
+ mHintSession = nullptr;
+ }
}
}
}
@@ -162,11 +163,13 @@
if (mSendUpdateImminent.exchange(false)) {
ALOGV("AIDL notifyDisplayUpdateImminentAndCpuReset");
- if (usePowerHintSession() && ensurePowerHintSessionRunning()) {
+ if (usePowerHintSession()) {
std::lock_guard lock(mHintSessionMutex);
- auto ret = mHintSession->sendHint(SessionHint::CPU_LOAD_RESET);
- if (!ret.isOk()) {
- mHintSessionRunning = false;
+ if (ensurePowerHintSessionRunning()) {
+ auto ret = mHintSession->sendHint(SessionHint::CPU_LOAD_RESET);
+ if (!ret.isOk()) {
+ mHintSession = nullptr;
+ }
}
}
@@ -193,14 +196,12 @@
}
}
-// checks both if it supports and if it's enabled
bool PowerAdvisor::usePowerHintSession() {
// uses cached value since the underlying support and flag are unlikely to change at runtime
return mHintSessionEnabled.value_or(false) && supportsPowerHintSession();
}
bool PowerAdvisor::supportsPowerHintSession() {
- // cache to avoid needing lock every time
if (!mSupportsHintSession.has_value()) {
mSupportsHintSession = getPowerHal().getHintSessionPreferredRate().isOk();
}
@@ -208,10 +209,15 @@
}
bool PowerAdvisor::ensurePowerHintSessionRunning() {
- if (!mHintSessionRunning && !mHintSessionThreadIds.empty() && usePowerHintSession()) {
- startPowerHintSession(mHintSessionThreadIds);
+ if (mHintSession == nullptr && !mHintSessionThreadIds.empty() && usePowerHintSession()) {
+ auto ret = getPowerHal().createHintSession(getpid(), static_cast<int32_t>(getuid()),
+ mHintSessionThreadIds, mTargetDuration.ns());
+
+ if (ret.isOk()) {
+ mHintSession = ret.value();
+ }
}
- return mHintSessionRunning;
+ return mHintSession != nullptr;
}
void PowerAdvisor::updateTargetWorkDuration(Duration targetDuration) {
@@ -223,15 +229,16 @@
{
mTargetDuration = targetDuration;
if (sTraceHintSessionData) ATRACE_INT64("Time target", targetDuration.ns());
- if (ensurePowerHintSessionRunning() && (targetDuration != mLastTargetDurationSent)) {
+ if (targetDuration == mLastTargetDurationSent) return;
+ std::lock_guard lock(mHintSessionMutex);
+ if (ensurePowerHintSessionRunning()) {
ALOGV("Sending target time: %" PRId64 "ns", targetDuration.ns());
mLastTargetDurationSent = targetDuration;
- std::lock_guard lock(mHintSessionMutex);
auto ret = mHintSession->updateTargetWorkDuration(targetDuration.ns());
if (!ret.isOk()) {
ALOGW("Failed to set power hint target work duration with error: %s",
ret.getDescription().c_str());
- mHintSessionRunning = false;
+ mHintSession = nullptr;
}
}
}
@@ -244,21 +251,12 @@
}
ATRACE_CALL();
std::optional<Duration> actualDuration = estimateWorkDuration();
- if (!actualDuration.has_value() || actualDuration < 0ns || !ensurePowerHintSessionRunning()) {
+ if (!actualDuration.has_value() || actualDuration < 0ns) {
ALOGV("Failed to send actual work duration, skipping");
return;
}
actualDuration = std::make_optional(*actualDuration + sTargetSafetyMargin);
mActualDuration = actualDuration;
- WorkDuration duration;
- duration.workPeriodStartTimestampNanos = mCommitStartTimes[0].ns();
- // TODO(b/284324521): Correctly calculate total duration.
- duration.durationNanos = actualDuration->ns();
- duration.cpuDurationNanos = actualDuration->ns();
- // TODO(b/284324521): Calculate RenderEngine GPU time.
- duration.gpuDurationNanos = 0;
- duration.timeStampNanos = TimePoint::now().ns();
- mHintSessionQueue.push_back(duration);
if (sTraceHintSessionData) {
ATRACE_INT64("Measured duration", actualDuration->ns());
@@ -274,13 +272,34 @@
actualDuration->ns(), mLastTargetDurationSent.ns(),
Duration{*actualDuration - mLastTargetDurationSent}.ns());
+ if (mTimingTestingMode) {
+ mDelayReportActualMutexAcquisitonPromise.get_future().wait();
+ mDelayReportActualMutexAcquisitonPromise = std::promise<bool>{};
+ }
+
{
std::lock_guard lock(mHintSessionMutex);
+ if (!ensurePowerHintSessionRunning()) {
+ ALOGV("Hint session not running and could not be started, skipping");
+ return;
+ }
+
+ WorkDuration duration{
+ .timeStampNanos = TimePoint::now().ns(),
+ // TODO(b/284324521): Correctly calculate total duration.
+ .durationNanos = actualDuration->ns(),
+ .workPeriodStartTimestampNanos = mCommitStartTimes[0].ns(),
+ .cpuDurationNanos = actualDuration->ns(),
+ // TODO(b/284324521): Calculate RenderEngine GPU time.
+ .gpuDurationNanos = 0,
+ };
+ mHintSessionQueue.push_back(duration);
+
auto ret = mHintSession->reportActualWorkDuration(mHintSessionQueue);
if (!ret.isOk()) {
ALOGW("Failed to report actual work durations with error: %s",
ret.getDescription().c_str());
- mHintSessionRunning = false;
+ mHintSession = nullptr;
return;
}
}
@@ -291,7 +310,8 @@
mHintSessionEnabled = enabled;
}
-bool PowerAdvisor::startPowerHintSession(const std::vector<int32_t>& threadIds) {
+bool PowerAdvisor::startPowerHintSession(std::vector<int32_t>&& threadIds) {
+ mHintSessionThreadIds = threadIds;
if (!mBootFinished.load()) {
return false;
}
@@ -299,25 +319,14 @@
ALOGI("Cannot start power hint session: disabled or unsupported");
return false;
}
- if (mHintSessionRunning) {
+ LOG_ALWAYS_FATAL_IF(mHintSessionThreadIds.empty(),
+ "No thread IDs provided to power hint session!");
+ std::lock_guard lock(mHintSessionMutex);
+ if (mHintSession != nullptr) {
ALOGE("Cannot start power hint session: already running");
return false;
}
- LOG_ALWAYS_FATAL_IF(threadIds.empty(), "No thread IDs provided to power hint session!");
- {
- std::lock_guard lock(mHintSessionMutex);
- mHintSession = nullptr;
- mHintSessionThreadIds = threadIds;
-
- auto ret = getPowerHal().createHintSession(getpid(), static_cast<int32_t>(getuid()),
- threadIds, mTargetDuration.ns());
-
- if (ret.isOk()) {
- mHintSessionRunning = true;
- mHintSession = ret.value();
- }
- }
- return mHintSessionRunning;
+ return ensurePowerHintSessionRunning();
}
void PowerAdvisor::setGpuFenceTime(DisplayId displayId, std::unique_ptr<FenceTime>&& fenceTime) {
diff --git a/services/surfaceflinger/DisplayHardware/PowerAdvisor.h b/services/surfaceflinger/DisplayHardware/PowerAdvisor.h
index 05e4c8b..0276e44 100644
--- a/services/surfaceflinger/DisplayHardware/PowerAdvisor.h
+++ b/services/surfaceflinger/DisplayHardware/PowerAdvisor.h
@@ -46,16 +46,15 @@
// Initializes resources that cannot be initialized on construction
virtual void init() = 0;
+ // Used to indicate that power hints can now be reported
virtual void onBootFinished() = 0;
virtual void setExpensiveRenderingExpected(DisplayId displayId, bool expected) = 0;
virtual bool isUsingExpensiveRendering() = 0;
- virtual void notifyCpuLoadUp() = 0;
- virtual void notifyDisplayUpdateImminentAndCpuReset() = 0;
- // Checks both if it supports and if it's enabled
+ // Checks both if it's supported and if it's enabled; this is thread-safe since its values are
+ // set before onBootFinished, which gates all methods that run on threads other than SF main
virtual bool usePowerHintSession() = 0;
virtual bool supportsPowerHintSession() = 0;
- virtual bool ensurePowerHintSessionRunning() = 0;
// Sends a power hint that updates to the target work duration for the frame
virtual void updateTargetWorkDuration(Duration targetDuration) = 0;
// Sends a power hint for the actual known work duration at the end of the frame
@@ -63,7 +62,7 @@
// Sets whether the power hint session is enabled
virtual void enablePowerHintSession(bool enabled) = 0;
// Initializes the power hint session
- virtual bool startPowerHintSession(const std::vector<int32_t>& threadIds) = 0;
+ virtual bool startPowerHintSession(std::vector<int32_t>&& threadIds) = 0;
// Provides PowerAdvisor with a copy of the gpu fence so it can determine the gpu end time
virtual void setGpuFenceTime(DisplayId displayId, std::unique_ptr<FenceTime>&& fenceTime) = 0;
// Reports the start and end times of a hwc validate call this frame for a given display
@@ -94,6 +93,12 @@
virtual void setDisplays(std::vector<DisplayId>& displayIds) = 0;
// Sets the target duration for the entire pipeline including the gpu
virtual void setTotalFrameTargetWorkDuration(Duration targetDuration) = 0;
+
+ // --- The following methods may run on threads besides SF main ---
+ // Send a hint about an upcoming increase in the CPU workload
+ virtual void notifyCpuLoadUp() = 0;
+ // Send a hint about the imminent start of a new CPU workload
+ virtual void notifyDisplayUpdateImminentAndCpuReset() = 0;
};
namespace impl {
@@ -109,16 +114,13 @@
void onBootFinished() override;
void setExpensiveRenderingExpected(DisplayId displayId, bool expected) override;
bool isUsingExpensiveRendering() override { return mNotifiedExpensiveRendering; };
- void notifyCpuLoadUp() override;
- void notifyDisplayUpdateImminentAndCpuReset() override;
bool usePowerHintSession() override;
bool supportsPowerHintSession() override;
- bool ensurePowerHintSessionRunning() override;
void updateTargetWorkDuration(Duration targetDuration) override;
void reportActualWorkDuration() override;
void enablePowerHintSession(bool enabled) override;
- bool startPowerHintSession(const std::vector<int32_t>& threadIds) override;
- void setGpuFenceTime(DisplayId displayId, std::unique_ptr<FenceTime>&& fenceTime);
+ bool startPowerHintSession(std::vector<int32_t>&& threadIds) override;
+ void setGpuFenceTime(DisplayId displayId, std::unique_ptr<FenceTime>&& fenceTime) override;
void setHwcValidateTiming(DisplayId displayId, TimePoint validateStartTime,
TimePoint validateEndTime) override;
void setHwcPresentTiming(DisplayId displayId, TimePoint presentStartTime,
@@ -128,13 +130,16 @@
void setExpectedPresentTime(TimePoint expectedPresentTime) override;
void setSfPresentTiming(TimePoint presentFenceTime, TimePoint presentEndTime) override;
void setHwcPresentDelayedTime(DisplayId displayId, TimePoint earliestFrameStartTime) override;
-
void setFrameDelay(Duration frameDelayDuration) override;
void setCommitStart(TimePoint commitStartTime) override;
void setCompositeEnd(TimePoint compositeEndTime) override;
void setDisplays(std::vector<DisplayId>& displayIds) override;
void setTotalFrameTargetWorkDuration(Duration targetDuration) override;
+ // --- The following methods may run on threads besides SF main ---
+ void notifyCpuLoadUp() override;
+ void notifyDisplayUpdateImminentAndCpuReset() override;
+
private:
friend class PowerAdvisorTest;
@@ -220,6 +225,7 @@
// this normalizes them together and takes the max of the two
Duration combineTimingEstimates(Duration totalDuration, Duration flingerDuration);
+ bool ensurePowerHintSessionRunning() REQUIRES(mHintSessionMutex);
std::unordered_map<DisplayId, DisplayTimingData> mDisplayTimingData;
// Current frame's delay
@@ -242,9 +248,10 @@
// Ensure powerhal connection is initialized
power::PowerHalController& getPowerHal();
+ // These variables are set before mBootFinished and never mutated after, so it's safe to access
+ // from threaded methods.
std::optional<bool> mHintSessionEnabled;
std::optional<bool> mSupportsHintSession;
- bool mHintSessionRunning = false;
std::mutex mHintSessionMutex;
std::shared_ptr<aidl::android::hardware::power::IPowerHintSession> mHintSession
@@ -261,6 +268,11 @@
// The list of thread ids, stored so we can restart the session from this class if needed
std::vector<int32_t> mHintSessionThreadIds;
Duration mLastTargetDurationSent = kDefaultTargetDuration;
+
+ // Used to manage the execution ordering of reportActualWorkDuration for concurrency testing
+ std::promise<bool> mDelayReportActualMutexAcquisitonPromise;
+ bool mTimingTestingMode = false;
+
// Whether we should emit ATRACE_INT data for hint sessions
static const bool sTraceHintSessionData;
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");
}
}
diff --git a/services/surfaceflinger/tests/unittests/PowerAdvisorTest.cpp b/services/surfaceflinger/tests/unittests/PowerAdvisorTest.cpp
index 85f66f4..9c66a97 100644
--- a/services/surfaceflinger/tests/unittests/PowerAdvisorTest.cpp
+++ b/services/surfaceflinger/tests/unittests/PowerAdvisorTest.cpp
@@ -24,6 +24,7 @@
#include <powermanager/PowerHalWrapper.h>
#include <ui/DisplayId.h>
#include <chrono>
+#include <future>
#include "TestableSurfaceFlinger.h"
#include "mock/DisplayHardware/MockIPowerHintSession.h"
#include "mock/DisplayHardware/MockPowerHalController.h"
@@ -40,11 +41,14 @@
class PowerAdvisorTest : public testing::Test {
public:
void SetUp() override;
- void startPowerHintSession();
+ void startPowerHintSession(bool returnValidSession = true);
void fakeBasicFrameTiming(TimePoint startTime, Duration vsyncPeriod);
void setExpectedTiming(Duration totalFrameTargetDuration, TimePoint expectedPresentTime);
Duration getFenceWaitDelayDuration(bool skipValidate);
Duration getErrorMargin();
+ void setTimingTestingMode(bool testinMode);
+ void allowReportActualToAcquireMutex();
+ bool sessionExists();
protected:
TestableSurfaceFlinger mFlinger;
@@ -53,6 +57,11 @@
std::shared_ptr<MockIPowerHintSession> mMockPowerHintSession;
};
+bool PowerAdvisorTest::sessionExists() {
+ std::scoped_lock lock(mPowerAdvisor->mHintSessionMutex);
+ return mPowerAdvisor->mHintSession != nullptr;
+}
+
void PowerAdvisorTest::SetUp() {
mPowerAdvisor = std::make_unique<impl::PowerAdvisor>(*mFlinger.flinger());
mPowerAdvisor->mPowerHal = std::make_unique<NiceMock<MockPowerHalController>>();
@@ -62,14 +71,20 @@
.WillByDefault(Return(HalResult<int64_t>::fromStatus(binder::Status::ok(), 16000)));
}
-void PowerAdvisorTest::startPowerHintSession() {
- const std::vector<int32_t> threadIds = {1, 2, 3};
+void PowerAdvisorTest::startPowerHintSession(bool returnValidSession) {
mMockPowerHintSession = ndk::SharedRefBase::make<NiceMock<MockIPowerHintSession>>();
- ON_CALL(*mMockPowerHalController, createHintSession)
- .WillByDefault(Return(HalResult<std::shared_ptr<IPowerHintSession>>::
- fromStatus(binder::Status::ok(), mMockPowerHintSession)));
+ if (returnValidSession) {
+ ON_CALL(*mMockPowerHalController, createHintSession)
+ .WillByDefault(
+ Return(HalResult<std::shared_ptr<IPowerHintSession>>::
+ fromStatus(binder::Status::ok(), mMockPowerHintSession)));
+ } else {
+ ON_CALL(*mMockPowerHalController, createHintSession)
+ .WillByDefault(Return(HalResult<std::shared_ptr<IPowerHintSession>>::
+ fromStatus(binder::Status::ok(), nullptr)));
+ }
mPowerAdvisor->enablePowerHintSession(true);
- mPowerAdvisor->startPowerHintSession(threadIds);
+ mPowerAdvisor->startPowerHintSession({1, 2, 3});
ON_CALL(*mMockPowerHintSession, updateTargetWorkDuration)
.WillByDefault(Return(testing::ByMove(ndk::ScopedAStatus::ok())));
}
@@ -86,6 +101,14 @@
mPowerAdvisor->updateTargetWorkDuration(vsyncPeriod);
}
+void PowerAdvisorTest::setTimingTestingMode(bool testingMode) {
+ mPowerAdvisor->mTimingTestingMode = testingMode;
+}
+
+void PowerAdvisorTest::allowReportActualToAcquireMutex() {
+ mPowerAdvisor->mDelayReportActualMutexAcquisitonPromise.set_value(true);
+}
+
Duration PowerAdvisorTest::getFenceWaitDelayDuration(bool skipValidate) {
return (skipValidate ? PowerAdvisor::kFenceWaitStartDelaySkippedValidate
: PowerAdvisor::kFenceWaitStartDelayValidated);
@@ -221,5 +244,131 @@
mPowerAdvisor->reportActualWorkDuration();
}
+TEST_F(PowerAdvisorTest, hintSessionValidWhenNullFromPowerHAL) {
+ mPowerAdvisor->onBootFinished();
+
+ startPowerHintSession(false);
+
+ std::vector<DisplayId> displayIds{PhysicalDisplayId::fromPort(42u)};
+
+ // 60hz
+ const Duration vsyncPeriod{std::chrono::nanoseconds(1s) / 60};
+ const Duration presentDuration = 5ms;
+ const Duration postCompDuration = 1ms;
+
+ TimePoint startTime{100ns};
+
+ // advisor only starts on frame 2 so do an initial no-op frame
+ fakeBasicFrameTiming(startTime, vsyncPeriod);
+ setExpectedTiming(vsyncPeriod, startTime + vsyncPeriod);
+ mPowerAdvisor->setDisplays(displayIds);
+ mPowerAdvisor->setSfPresentTiming(startTime, startTime + presentDuration);
+ mPowerAdvisor->setCompositeEnd(startTime + presentDuration + postCompDuration);
+
+ // increment the frame
+ startTime += vsyncPeriod;
+
+ const Duration expectedDuration = getErrorMargin() + presentDuration + postCompDuration;
+ EXPECT_CALL(*mMockPowerHintSession,
+ reportActualWorkDuration(ElementsAre(
+ Field(&WorkDuration::durationNanos, Eq(expectedDuration.ns())))))
+ .Times(0);
+ fakeBasicFrameTiming(startTime, vsyncPeriod);
+ setExpectedTiming(vsyncPeriod, startTime + vsyncPeriod);
+ mPowerAdvisor->setDisplays(displayIds);
+ mPowerAdvisor->setHwcValidateTiming(displayIds[0], startTime + 1ms, startTime + 1500us);
+ mPowerAdvisor->setHwcPresentTiming(displayIds[0], startTime + 2ms, startTime + 2500us);
+ mPowerAdvisor->setSfPresentTiming(startTime, startTime + presentDuration);
+ mPowerAdvisor->reportActualWorkDuration();
+}
+
+TEST_F(PowerAdvisorTest, hintSessionOnlyCreatedOnce) {
+ EXPECT_CALL(*mMockPowerHalController, createHintSession(_, _, _, _)).Times(1);
+ mPowerAdvisor->onBootFinished();
+ startPowerHintSession();
+ mPowerAdvisor->startPowerHintSession({1, 2, 3});
+}
+
+TEST_F(PowerAdvisorTest, hintSessionTestNotifyReportRace) {
+ // notifyDisplayUpdateImminentAndCpuReset or notifyCpuLoadUp gets called in background
+ // reportActual gets called during callback and sees true session, passes ensure
+ // first notify finishes, setting value to true. Another async method gets called, acquires the
+ // lock between reportactual finishing ensure and acquiring the lock itself, and sets session to
+ // nullptr. reportActual acquires the lock, and the session is now null, so it does nullptr
+ // deref
+
+ mPowerAdvisor->onBootFinished();
+ startPowerHintSession();
+
+ // --- fake a bunch of timing data
+ std::vector<DisplayId> displayIds{PhysicalDisplayId::fromPort(42u)};
+ // 60hz
+ const Duration vsyncPeriod{std::chrono::nanoseconds(1s) / 60};
+ const Duration presentDuration = 5ms;
+ const Duration postCompDuration = 1ms;
+ TimePoint startTime{100ns};
+ // advisor only starts on frame 2 so do an initial no-op frame
+ fakeBasicFrameTiming(startTime, vsyncPeriod);
+ setExpectedTiming(vsyncPeriod, startTime + vsyncPeriod);
+ mPowerAdvisor->setDisplays(displayIds);
+ mPowerAdvisor->setSfPresentTiming(startTime, startTime + presentDuration);
+ mPowerAdvisor->setCompositeEnd(startTime + presentDuration + postCompDuration);
+ // increment the frame
+ startTime += vsyncPeriod;
+ fakeBasicFrameTiming(startTime, vsyncPeriod);
+ setExpectedTiming(vsyncPeriod, startTime + vsyncPeriod);
+ mPowerAdvisor->setDisplays(displayIds);
+ mPowerAdvisor->setHwcValidateTiming(displayIds[0], startTime + 1ms, startTime + 1500us);
+ mPowerAdvisor->setHwcPresentTiming(displayIds[0], startTime + 2ms, startTime + 2500us);
+ mPowerAdvisor->setSfPresentTiming(startTime, startTime + presentDuration);
+ // --- Done faking timing data
+
+ setTimingTestingMode(true);
+ std::promise<bool> letSendHintFinish;
+
+ ON_CALL(*mMockPowerHintSession, sendHint).WillByDefault([&letSendHintFinish] {
+ letSendHintFinish.get_future().wait();
+ return ndk::ScopedAStatus::fromExceptionCode(-127);
+ });
+
+ ON_CALL(*mMockPowerHintSession, reportActualWorkDuration).WillByDefault([] {
+ return ndk::ScopedAStatus::fromExceptionCode(-127);
+ });
+
+ ON_CALL(*mMockPowerHalController, createHintSession)
+ .WillByDefault(Return(
+ HalResult<std::shared_ptr<IPowerHintSession>>::
+ fromStatus(ndk::ScopedAStatus::fromExceptionCode(-127), nullptr)));
+
+ // First background call, to notice the session is down
+ auto firstHint = std::async(std::launch::async, [this] {
+ mPowerAdvisor->notifyCpuLoadUp();
+ return true;
+ });
+ std::this_thread::sleep_for(10ms);
+
+ // Call reportActual while callback is resolving to try and sneak past ensure
+ auto reportActual =
+ std::async(std::launch::async, [this] { mPowerAdvisor->reportActualWorkDuration(); });
+
+ std::this_thread::sleep_for(10ms);
+ // Let the first call finish
+ letSendHintFinish.set_value(true);
+ letSendHintFinish = std::promise<bool>{};
+ firstHint.wait();
+
+ // Do the second notify call, to ensure the session is nullptr
+ auto secondHint = std::async(std::launch::async, [this] {
+ mPowerAdvisor->notifyCpuLoadUp();
+ return true;
+ });
+ letSendHintFinish.set_value(true);
+ secondHint.wait();
+ // Let report finish, potentially dereferencing
+ allowReportActualToAcquireMutex();
+ reportActual.wait();
+ EXPECT_EQ(sessionExists(), false);
+}
+
} // namespace
} // namespace android::Hwc2::impl
diff --git a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockPowerAdvisor.h b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockPowerAdvisor.h
index d635508..8e8eb1d 100644
--- a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockPowerAdvisor.h
+++ b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockPowerAdvisor.h
@@ -36,11 +36,10 @@
MOCK_METHOD(void, notifyDisplayUpdateImminentAndCpuReset, (), (override));
MOCK_METHOD(bool, usePowerHintSession, (), (override));
MOCK_METHOD(bool, supportsPowerHintSession, (), (override));
- MOCK_METHOD(bool, ensurePowerHintSessionRunning, (), (override));
MOCK_METHOD(void, updateTargetWorkDuration, (Duration targetDuration), (override));
MOCK_METHOD(void, reportActualWorkDuration, (), (override));
MOCK_METHOD(void, enablePowerHintSession, (bool enabled), (override));
- MOCK_METHOD(bool, startPowerHintSession, (const std::vector<int32_t>& threadIds), (override));
+ MOCK_METHOD(bool, startPowerHintSession, (std::vector<int32_t> && threadIds), (override));
MOCK_METHOD(void, setGpuFenceTime,
(DisplayId displayId, std::unique_ptr<FenceTime>&& fenceTime), (override));
MOCK_METHOD(void, setHwcValidateTiming,