power: Fix race condition between Looper and destructor
1. Clean all messages before add new.
2. Insteading of using `this`, use the unique mStaleHandler sp so Looper
can hold the sp to keep the instance alive until the last message
done.
Test: Manual Test
Bug: 219965773
Change-Id: Ic039146f0b966c1f27d86b121d4b72b75ff360e5
diff --git a/aidl/power-libperfmgr/PowerHintSession.cpp b/aidl/power-libperfmgr/PowerHintSession.cpp
index a23e943..0c6f360 100644
--- a/aidl/power-libperfmgr/PowerHintSession.cpp
+++ b/aidl/power-libperfmgr/PowerHintSession.cpp
@@ -39,7 +39,6 @@
using ::android::base::StringPrintf;
using std::chrono::duration_cast;
using std::chrono::nanoseconds;
-using std::literals::chrono_literals::operator""s;
constexpr char kPowerHalAdpfPidPOver[] = "vendor.mediatek.powerhal.adpf.pid_p.over";
constexpr char kPowerHalAdpfPidPUnder[] = "vendor.mediatek.powerhal.adpf.pid_p.under";
@@ -167,6 +166,7 @@
sz = sz = StringPrintf("adpf.%s-active", idstr.c_str());
ATRACE_INT(sz.c_str(), 0);
}
+ mStaleHandler->setSessionDead();
delete mDescriptor;
}
@@ -210,6 +210,10 @@
}
ndk::ScopedAStatus PowerHintSession::pause() {
+ if (mSessionClosed) {
+ ALOGE("Error: session is dead");
+ return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE);
+ }
if (!mDescriptor->is_active.load())
return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE);
// Reset to default uclamp value.
@@ -225,6 +229,10 @@
}
ndk::ScopedAStatus PowerHintSession::resume() {
+ if (mSessionClosed) {
+ ALOGE("Error: session is dead");
+ return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE);
+ }
if (mDescriptor->is_active.load())
return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE);
mDescriptor->is_active.store(true);
@@ -245,7 +253,6 @@
if (!mSessionClosed.compare_exchange_strong(sessionClosedExpectedToBe, true)) {
return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE);
}
- PowerHintMonitor::getInstance()->getLooper()->removeMessages(mStaleHandler);
setUclamp(0);
PowerSessionManager::getInstance()->removePowerSession(this);
updateUniveralBoostMode();
@@ -253,6 +260,10 @@
}
ndk::ScopedAStatus PowerHintSession::updateTargetWorkDuration(int64_t targetDurationNanos) {
+ if (mSessionClosed) {
+ ALOGE("Error: session is dead");
+ return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE);
+ }
if (targetDurationNanos <= 0) {
ALOGE("Error: targetDurationNanos(%" PRId64 ") should bigger than 0", targetDurationNanos);
return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT);
@@ -275,6 +286,10 @@
ndk::ScopedAStatus PowerHintSession::reportActualWorkDuration(
const std::vector<WorkDuration> &actualDurations) {
+ if (mSessionClosed) {
+ ALOGE("Error: session is dead");
+ return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE);
+ }
if (mDescriptor->duration.count() == 0LL) {
ALOGE("Expect to call updateTargetWorkDuration() first.");
return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE);
@@ -441,8 +456,9 @@
}
if (!mIsMonitoringStale.load()) {
auto next = getStaleTime();
+ PowerHintMonitor::getInstance()->getLooper()->removeMessages(mSession->mStaleHandler);
PowerHintMonitor::getInstance()->getLooper()->sendMessageDelayed(
- duration_cast<nanoseconds>(next - now).count(), this, NULL);
+ duration_cast<nanoseconds>(next - now).count(), mSession->mStaleHandler, NULL);
mIsMonitoringStale.store(true);
}
if (ATRACE_ENABLED()) {
@@ -460,6 +476,9 @@
void PowerHintSession::StaleHandler::handleMessage(const Message &) {
std::lock_guard<std::mutex> guard(mStaleLock);
+ if (mIsSessionDead) {
+ return;
+ }
auto now = std::chrono::steady_clock::now();
auto when = getStaleTime();
// Check if the session is stale based on the last_updated_time.
@@ -469,8 +488,15 @@
return;
}
// Schedule for the next checking time.
+ PowerHintMonitor::getInstance()->getLooper()->removeMessages(mSession->mStaleHandler);
PowerHintMonitor::getInstance()->getLooper()->sendMessageDelayed(
- duration_cast<nanoseconds>(when - now).count(), this, NULL);
+ duration_cast<nanoseconds>(when - now).count(), mSession->mStaleHandler, NULL);
+}
+
+void PowerHintSession::StaleHandler::setSessionDead() {
+ std::lock_guard<std::mutex> guard(mStaleLock);
+ PowerHintMonitor::getInstance()->getLooper()->removeMessages(mSession->mStaleHandler);
+ mIsSessionDead = true;
}
} // namespace pixel
diff --git a/aidl/power-libperfmgr/PowerHintSession.h b/aidl/power-libperfmgr/PowerHintSession.h
index e3c0f84..52c2163 100644
--- a/aidl/power-libperfmgr/PowerHintSession.h
+++ b/aidl/power-libperfmgr/PowerHintSession.h
@@ -86,16 +86,21 @@
class StaleHandler : public MessageHandler {
public:
StaleHandler(PowerHintSession *session)
- : mSession(session), mIsMonitoringStale(false), mLastUpdatedTime(steady_clock::now()) {}
+ : mSession(session),
+ mIsMonitoringStale(false),
+ mLastUpdatedTime(steady_clock::now()),
+ mIsSessionDead(false) {}
void handleMessage(const Message &message) override;
void updateStaleTimer();
time_point<steady_clock> getStaleTime();
+ void setSessionDead();
private:
PowerHintSession *mSession;
std::atomic<bool> mIsMonitoringStale;
std::atomic<time_point<steady_clock>> mLastUpdatedTime;
std::mutex mStaleLock;
+ bool mIsSessionDead;
};
private: