diff options
author | Android Build Coastguard Worker <android-build-coastguard-worker@google.com> | 2022-08-05 03:28:58 +0000 |
---|---|---|
committer | Android Build Coastguard Worker <android-build-coastguard-worker@google.com> | 2022-08-05 03:28:58 +0000 |
commit | f30d12a174d560219cf242bb612cb547791c4fad (patch) | |
tree | a2874736b0a07aebd659afc55c2d5e9026caeedb | |
parent | 294207a436242ba4c6369fa2077ea0870649cf5d (diff) | |
parent | 265864c35c0635942389128139fc1664015bb45f (diff) | |
download | interfaces-android13-qpr1-s3-release.tar.gz |
Snap for 8909196 from 265864c35c0635942389128139fc1664015bb45f to tm-qpr1-releaseandroid-13.0.0_r30android-13.0.0_r29android-13.0.0_r28android-13.0.0_r27android-13.0.0_r24android-13.0.0_r23android-13.0.0_r22android-13.0.0_r21android-13.0.0_r20android-13.0.0_r19android-13.0.0_r18android-13.0.0_r17android-13.0.0_r16android13-qpr1-s8-releaseandroid13-qpr1-s7-releaseandroid13-qpr1-s6-releaseandroid13-qpr1-s5-releaseandroid13-qpr1-s4-releaseandroid13-qpr1-s3-releaseandroid13-qpr1-s2-releaseandroid13-qpr1-s1-releaseandroid13-qpr1-release
Change-Id: Ib4bfaeb72481fe055389f7a2930fa432ac761a32
-rw-r--r-- | suspend/1.0/default/Android.bp | 1 | ||||
-rw-r--r-- | suspend/1.0/default/SystemSuspend.cpp | 155 | ||||
-rw-r--r-- | suspend/1.0/default/SystemSuspend.h | 35 | ||||
-rw-r--r-- | suspend/1.0/default/SystemSuspendUnitTest.cpp | 51 |
4 files changed, 166 insertions, 76 deletions
diff --git a/suspend/1.0/default/Android.bp b/suspend/1.0/default/Android.bp index 9bcbc3c..eca48be 100644 --- a/suspend/1.0/default/Android.bp +++ b/suspend/1.0/default/Android.bp @@ -30,6 +30,7 @@ cc_defaults { cflags: [ "-Wall", "-Werror", + "-Wthread-safety", ], cpp_std: "c++17", } diff --git a/suspend/1.0/default/SystemSuspend.cpp b/suspend/1.0/default/SystemSuspend.cpp index a0e08b4..5e5a745 100644 --- a/suspend/1.0/default/SystemSuspend.cpp +++ b/suspend/1.0/default/SystemSuspend.cpp @@ -158,13 +158,14 @@ SystemSuspend::SystemSuspend(unique_fd wakeupCountFd, unique_fd stateFd, unique_ } bool SystemSuspend::enableAutosuspend(const sp<IBinder>& token) { + auto tokensLock = std::lock_guard(mAutosuspendClientTokensLock); auto autosuspendLock = std::lock_guard(mAutosuspendLock); - bool hasToken = std::find(mDisableAutosuspendTokens.begin(), mDisableAutosuspendTokens.end(), - token) != mDisableAutosuspendTokens.end(); + bool hasToken = std::find(mAutosuspendClientTokens.begin(), mAutosuspendClientTokens.end(), + token) != mAutosuspendClientTokens.end(); if (!hasToken) { - mDisableAutosuspendTokens.push_back(token); + mAutosuspendClientTokens.push_back(token); } if (mAutosuspendEnabled) { @@ -178,7 +179,7 @@ bool SystemSuspend::enableAutosuspend(const sp<IBinder>& token) { } void SystemSuspend::disableAutosuspendLocked() { - mDisableAutosuspendTokens.clear(); + mAutosuspendClientTokens.clear(); if (mAutosuspendEnabled) { mAutosuspendEnabled = false; mAutosuspendCondVar.notify_all(); @@ -187,28 +188,37 @@ void SystemSuspend::disableAutosuspendLocked() { } void SystemSuspend::disableAutosuspend() { + auto tokensLock = std::lock_guard(mAutosuspendClientTokensLock); auto autosuspendLock = std::lock_guard(mAutosuspendLock); disableAutosuspendLocked(); } -bool SystemSuspend::hasAliveAutosuspendTokenLocked() { - mDisableAutosuspendTokens.erase( - std::remove_if(mDisableAutosuspendTokens.begin(), mDisableAutosuspendTokens.end(), +void SystemSuspend::checkAutosuspendClientsLivenessLocked() { + // Ping autosuspend client tokens, remove any dead tokens from the list. + // mAutosuspendLock must not be held when calling this, as that could lead to a deadlock + // if pingBinder() can't be processed by system_server because it's Binder thread pool is + // exhausted and blocked on acquire/release wakelock calls. + mAutosuspendClientTokens.erase( + std::remove_if(mAutosuspendClientTokens.begin(), mAutosuspendClientTokens.end(), [](const sp<IBinder>& token) { return token->pingBinder() != OK; }), - mDisableAutosuspendTokens.end()); + mAutosuspendClientTokens.end()); +} - return !mDisableAutosuspendTokens.empty(); +bool SystemSuspend::hasAliveAutosuspendTokenLocked() { + return !mAutosuspendClientTokens.empty(); } SystemSuspend::~SystemSuspend(void) { + auto tokensLock = std::lock_guard(mAutosuspendClientTokensLock); auto autosuspendLock = std::unique_lock(mAutosuspendLock); // signal autosuspend thread to shut down disableAutosuspendLocked(); // wait for autosuspend thread to exit - mAutosuspendCondVar.wait_for(autosuspendLock, 100ms, - [this] { return !mAutosuspendThreadCreated; }); + mAutosuspendCondVar.wait_for(autosuspendLock, 100ms, [this]() REQUIRES(mAutosuspendLock) { + return !mAutosuspendThreadCreated; + }); } bool SystemSuspend::forceSuspend() { @@ -273,80 +283,99 @@ void SystemSuspend::initAutosuspendLocked() { bool shouldSleep = true; while (true) { - if (!mAutosuspendEnabled) { - mAutosuspendThreadCreated = false; - return; - } - // If we got here by a failed write to /sys/power/wakeup_count; don't sleep - // since we didn't attempt to suspend on the last cycle of this loop. - if (shouldSleep) { - mAutosuspendCondVar.wait_for(autosuspendLock, mSleepTime, - [this] { return !mAutosuspendEnabled; }); - } - - if (!mAutosuspendEnabled) continue; - - string wakeupCount; { - autosuspendLock.unlock(); + base::ScopedLockAssertion autosuspendLocked(mAutosuspendLock); - lseek(mWakeupCountFd, 0, SEEK_SET); - wakeupCount = readFd(mWakeupCountFd); + if (!mAutosuspendEnabled) { + mAutosuspendThreadCreated = false; + return; + } + // If we got here by a failed write to /sys/power/wakeup_count; don't sleep + // since we didn't attempt to suspend on the last cycle of this loop. + if (shouldSleep) { + mAutosuspendCondVar.wait_for( + autosuspendLock, mSleepTime, + [this]() REQUIRES(mAutosuspendLock) { return !mAutosuspendEnabled; }); + } - autosuspendLock.lock(); + if (!mAutosuspendEnabled) continue; + autosuspendLock.unlock(); } - if (wakeupCount.empty()) { - PLOG(ERROR) << "error reading from /sys/power/wakeup_count"; - continue; - } + lseek(mWakeupCountFd, 0, SEEK_SET); + string wakeupCount = readFd(mWakeupCountFd); - shouldSleep = false; + { + autosuspendLock.lock(); + base::ScopedLockAssertion autosuspendLocked(mAutosuspendLock); - mAutosuspendCondVar.wait( - autosuspendLock, [this] { return mSuspendCounter == 0 || !mAutosuspendEnabled; }); - // The mutex is locked and *MUST* remain locked until we write to /sys/power/state. - // Otherwise, a WakeLock might be acquired after we check mSuspendCounter and before we - // write to /sys/power/state. + if (wakeupCount.empty()) { + PLOG(ERROR) << "error reading from /sys/power/wakeup_count"; + continue; + } - if (!mAutosuspendEnabled) continue; + shouldSleep = false; - if (!hasAliveAutosuspendTokenLocked()) { - disableAutosuspendLocked(); - continue; - } + mAutosuspendCondVar.wait(autosuspendLock, [this]() REQUIRES(mAutosuspendLock) { + return mSuspendCounter == 0 || !mAutosuspendEnabled; + }); - if (!WriteStringToFd(wakeupCount, mWakeupCountFd)) { - PLOG(VERBOSE) << "error writing from /sys/power/wakeup_count"; - continue; + if (!mAutosuspendEnabled) continue; + autosuspendLock.unlock(); } - bool success = WriteStringToFd(kSleepState, mStateFd); - shouldSleep = true; + bool success; { - autosuspendLock.unlock(); + auto tokensLock = std::lock_guard(mAutosuspendClientTokensLock); + checkAutosuspendClientsLivenessLocked(); - if (!success) { - PLOG(VERBOSE) << "error writing to /sys/power/state"; + autosuspendLock.lock(); + base::ScopedLockAssertion autosuspendLocked(mAutosuspendLock); + + if (!hasAliveAutosuspendTokenLocked()) { + disableAutosuspendLocked(); + continue; } - struct SuspendTime suspendTime = readSuspendTime(mSuspendTimeFd); - updateSleepTime(success, suspendTime); + // Check suspend counter hasn't increased while checking client liveness + if (mSuspendCounter > 0) { + continue; + } - std::vector<std::string> wakeupReasons = readWakeupReasons(mWakeupReasonsFd); - if (wakeupReasons == std::vector<std::string>({kUnknownWakeup})) { - LOG(INFO) << "Unknown/empty wakeup reason. Re-opening wakeup_reason file."; + // The mutex is locked and *MUST* remain locked until we write to /sys/power/state. + // Otherwise, a WakeLock might be acquired after we check mSuspendCounter and before + // we write to /sys/power/state. - mWakeupReasonsFd = - std::move(reopenFileUsingFd(mWakeupReasonsFd.get(), O_CLOEXEC | O_RDONLY)); + if (!WriteStringToFd(wakeupCount, mWakeupCountFd)) { + PLOG(VERBOSE) << "error writing to /sys/power/wakeup_count"; + continue; } - mWakeupList.update(wakeupReasons); + success = WriteStringToFd(kSleepState, mStateFd); + shouldSleep = true; + + autosuspendLock.unlock(); + } - mControlService->notifyWakeup(success, wakeupReasons); + if (!success) { + PLOG(VERBOSE) << "error writing to /sys/power/state"; + } - // Take the lock before returning to the start of the loop - autosuspendLock.lock(); + struct SuspendTime suspendTime = readSuspendTime(mSuspendTimeFd); + updateSleepTime(success, suspendTime); + + std::vector<std::string> wakeupReasons = readWakeupReasons(mWakeupReasonsFd); + if (wakeupReasons == std::vector<std::string>({kUnknownWakeup})) { + LOG(INFO) << "Unknown/empty wakeup reason. Re-opening wakeup_reason file."; + + mWakeupReasonsFd = + std::move(reopenFileUsingFd(mWakeupReasonsFd.get(), O_CLOEXEC | O_RDONLY)); } + mWakeupList.update(wakeupReasons); + + mControlService->notifyWakeup(success, wakeupReasons); + + // Take the lock before returning to the start of the loop + autosuspendLock.lock(); } }); autosuspendThread.detach(); diff --git a/suspend/1.0/default/SystemSuspend.h b/suspend/1.0/default/SystemSuspend.h index ddcee24..a3f9a00 100644 --- a/suspend/1.0/default/SystemSuspend.h +++ b/suspend/1.0/default/SystemSuspend.h @@ -18,6 +18,7 @@ #define ANDROID_SYSTEM_SYSTEM_SUSPEND_V1_0_H #include <android-base/result.h> +#include <android-base/thread_annotations.h> #include <android-base/unique_fd.h> #include <android/system/suspend/internal/SuspendInfo.h> #include <utils/RefBase.h> @@ -99,27 +100,39 @@ class SystemSuspend : public RefBase { private: ~SystemSuspend(void) override; - void initAutosuspendLocked(); - void disableAutosuspendLocked(); - bool hasAliveAutosuspendTokenLocked(); - std::mutex mAutosuspendLock; - std::condition_variable mAutosuspendCondVar; - uint32_t mSuspendCounter; + std::mutex mAutosuspendClientTokensLock; + std::mutex mAutosuspendLock ACQUIRED_AFTER(mAutosuspendClientTokensLock); + std::mutex mSuspendInfoLock; + + void initAutosuspendLocked() + EXCLUSIVE_LOCKS_REQUIRED(mAutosuspendClientTokensLock, mAutosuspendLock); + void disableAutosuspendLocked() + EXCLUSIVE_LOCKS_REQUIRED(mAutosuspendClientTokensLock, mAutosuspendLock); + void checkAutosuspendClientsLivenessLocked() + EXCLUSIVE_LOCKS_REQUIRED(mAutosuspendClientTokensLock); + bool hasAliveAutosuspendTokenLocked() EXCLUSIVE_LOCKS_REQUIRED(mAutosuspendClientTokensLock); + + std::condition_variable mAutosuspendCondVar GUARDED_BY(mAutosuspendLock); + uint32_t mSuspendCounter GUARDED_BY(mAutosuspendLock); + + std::vector<sp<IBinder>> mAutosuspendClientTokens GUARDED_BY(mAutosuspendClientTokensLock); + std::atomic<bool> mAutosuspendEnabled GUARDED_BY(mAutosuspendLock){false}; + std::atomic<bool> mAutosuspendThreadCreated GUARDED_BY(mAutosuspendLock){false}; + unique_fd mWakeupCountFd; unique_fd mStateFd; unique_fd mSuspendStatsFd; unique_fd mSuspendTimeFd; - std::mutex mSuspendInfoLock; - SuspendInfo mSuspendInfo; + SuspendInfo mSuspendInfo GUARDED_BY(mSuspendInfoLock); const SleepTimeConfig kSleepTimeConfig; // Amount of thread sleep time between consecutive iterations of the suspend loop std::chrono::milliseconds mSleepTime; - int32_t mNumConsecutiveBadSuspends; + int32_t mNumConsecutiveBadSuspends GUARDED_BY(mSuspendInfoLock); // Updates thread sleep time and suspend stats depending on the result of suspend attempt void updateSleepTime(bool success, const struct SuspendTime& suspendTime); @@ -137,10 +150,6 @@ class SystemSuspend : public RefBase { unique_fd mWakeLockFd; unique_fd mWakeUnlockFd; unique_fd mWakeupReasonsFd; - - std::atomic<bool> mAutosuspendEnabled{false}; - std::atomic<bool> mAutosuspendThreadCreated{false}; - std::vector<sp<IBinder>> mDisableAutosuspendTokens; }; } // namespace V1_0 diff --git a/suspend/1.0/default/SystemSuspendUnitTest.cpp b/suspend/1.0/default/SystemSuspendUnitTest.cpp index c779623..f6e216b 100644 --- a/suspend/1.0/default/SystemSuspendUnitTest.cpp +++ b/suspend/1.0/default/SystemSuspendUnitTest.cpp @@ -335,6 +335,57 @@ TEST_F(SystemSuspendTest, BlockAutosuspendIfBinderIsDead) { ASSERT_TRUE(isSystemSuspendBlocked(150)); } +TEST_F(SystemSuspendTest, UnresponsiveClientDoesNotBlockAcquireRelease) { + static std::mutex _lock; + static std::condition_variable inPingBinderCondVar; + static bool inPingBinder = false; + + class UnresponsiveBinder : public BBinder { + android::status_t pingBinder() override { + auto lock = std::unique_lock(_lock); + inPingBinder = true; + inPingBinderCondVar.notify_all(); + + // Block pingBinder until test finishes and releases its lock + inPingBinderCondVar.wait(lock); + return android::UNKNOWN_ERROR; + } + }; + + systemSuspend->disableAutosuspend(); + unblockSystemSuspendFromWakeupCount(); + ASSERT_TRUE(isSystemSuspendBlocked()); + + auto token = sp<UnresponsiveBinder>::make(); + bool enabled = false; + controlServiceInternal->enableAutosuspend(token, &enabled); + unblockSystemSuspendFromWakeupCount(); + + auto lock = std::unique_lock(_lock); + // wait until pingBinder has been called. + if (!inPingBinder) { + inPingBinderCondVar.wait(lock); + } + // let pingBinder finish once we release the test lock + inPingBinderCondVar.notify_all(); + + std::condition_variable wakeLockAcquired; + std::thread( + [this](std::condition_variable& wakeLockAcquired) { + std::shared_ptr<IWakeLock> testLock = acquireWakeLock("testLock"); + testLock->release(); + wakeLockAcquired.notify_all(); + }, + std::ref(wakeLockAcquired)) + .detach(); + + std::mutex _acquireReleaseLock; + auto acquireReleaseLock = std::unique_lock(_acquireReleaseLock); + bool timedOut = wakeLockAcquired.wait_for(acquireReleaseLock, 200ms) == std::cv_status::timeout; + + ASSERT_FALSE(timedOut); +} + TEST_F(SystemSuspendTest, AutosuspendLoop) { checkLoop(5); } |