Snap for 8909196 from 265864c35c0635942389128139fc1664015bb45f to tm-d4-releaseandroid-13.0.0_r48android-13.0.0_r47android-13.0.0_r46android13-d4-s2-releaseandroid13-d4-s1-releaseandroid13-d4-release

Change-Id: Ie0b5fa7fd5cbff73706b609bb5f6a443dabfc16c

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);
 }