Merge "Fix double release of ScopedWakelock" into rvc-qpr-dev

sensors/common/default/2.X/multihal/HalProxy.cpp

@@ -693,6 +693,10 @@ void HalProxy::decrementRefCountAndMaybeReleaseWakelock(size_t delta,
                                                         int64_t timeoutStart /* = -1 */) {
     if (!mThreadsRun.load()) return;
     std::lock_guard<std::recursive_mutex> lockGuard(mWakelockMutex);
+    if (delta > mWakelockRefCount) {
+        ALOGE("Decrementing wakelock ref count by %zu when count is %zu",
+              delta, mWakelockRefCount);
+    }
     if (timeoutStart == -1) timeoutStart = mWakelockTimeoutResetTime;
     if (mWakelockRefCount == 0 || timeoutStart < mWakelockTimeoutResetTime) return;
     mWakelockRefCount -= std::min(mWakelockRefCount, delta);

sensors/common/default/2.X/multihal/ScopedWakelock.cpp

@@ -28,6 +28,21 @@ int64_t getTimeNow() {
             .count();
 }
 
+ScopedWakelock::ScopedWakelock(ScopedWakelock&& other) {
+    *this = std::move(other);
+}
+
+ScopedWakelock& ScopedWakelock::operator=(ScopedWakelock&& other) {
+    mRefCounter = other.mRefCounter;
+    mCreatedAtTimeNs = other.mCreatedAtTimeNs;
+    mLocked = other.mLocked;
+
+    other.mRefCounter = nullptr;
+    other.mCreatedAtTimeNs = 0;
+    other.mLocked = false;
+    return *this;
+}
+
 ScopedWakelock::ScopedWakelock(IScopedWakelockRefCounter* refCounter, bool locked)
     : mRefCounter(refCounter), mLocked(locked) {
     if (mLocked) {

sensors/common/default/2.X/multihal/include/V2_0/ScopedWakelock.h

@@ -81,14 +81,15 @@ class IScopedWakelockRefCounter : public RefBase {
  */
 class ScopedWakelock {
   public:
-    ScopedWakelock(ScopedWakelock&&) = default;
-    ScopedWakelock& operator=(ScopedWakelock&&) = default;
+    ScopedWakelock(ScopedWakelock&& other);
+    ScopedWakelock& operator=(ScopedWakelock&& other);
     virtual ~ScopedWakelock();
 
     bool isLocked() const { return mLocked; }
 
   private:
     friend class HalProxyCallbackBase;
+    friend class ScopedWakelockTest;
     IScopedWakelockRefCounter* mRefCounter;
     int64_t mCreatedAtTimeNs;
     bool mLocked;

sensors/common/default/2.X/multihal/tests/Android.bp

@@ -90,7 +90,10 @@ cc_test_library {
 
 cc_test {
     name: "android.hardware.sensors@2.X-halproxy-unit-tests",
-    srcs: ["HalProxy_test.cpp"],
+    srcs: [
+        "HalProxy_test.cpp",
+        "ScopedWakelock_test.cpp",
+    ],
     vendor: true,
     header_libs: [
         "android.hardware.sensors@2.X-shared-utils",

sensors/common/default/2.X/multihal/tests/ScopedWakelock_test.cpp

@@ -0,0 +1,110 @@
+//
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <gtest/gtest.h>
+
+#include "V2_0/ScopedWakelock.h"
+
+namespace android {
+namespace hardware {
+namespace sensors {
+namespace V2_0 {
+namespace implementation {
+
+class RefCounter : public IScopedWakelockRefCounter {
+  public:
+    size_t incCount = 0;
+    size_t decCount = 0;
+
+    bool incrementRefCountAndMaybeAcquireWakelock(size_t /* delta */,
+                                                  int64_t* /* timeoutStart */) override {
+        incCount++;
+        return true;
+    }
+
+    void decrementRefCountAndMaybeReleaseWakelock(size_t /* delta */,
+                                                  int64_t /* timeoutStart */) override {
+        decCount++;
+    }
+};
+
+class ScopedWakelockTest : public testing::Test {
+  public:
+    ScopedWakelock createScopedWakelock(bool locked) {
+        return ScopedWakelock(&mRefCounter, locked);
+    }
+
+    RefCounter mRefCounter;
+};
+
+TEST_F(ScopedWakelockTest, UnlockedAfterMoved) {
+    ScopedWakelock wakelock = createScopedWakelock(false /* locked */);
+
+    ScopedWakelock movedWakelock(std::move(wakelock));
+
+    EXPECT_FALSE(wakelock.isLocked());
+    EXPECT_FALSE(movedWakelock.isLocked());
+}
+
+TEST_F(ScopedWakelockTest, LockedAfterMoved) {
+    ScopedWakelock wakelock = createScopedWakelock(true /* locked */);
+
+    ScopedWakelock movedWakelock(std::move(wakelock));
+
+    EXPECT_FALSE(wakelock.isLocked());
+    EXPECT_TRUE(movedWakelock.isLocked());
+}
+
+TEST_F(ScopedWakelockTest, Locked) {
+    ScopedWakelock wakelock = createScopedWakelock(true /* locked */);
+
+    EXPECT_TRUE(wakelock.isLocked());
+}
+
+TEST_F(ScopedWakelockTest, Unlocked) {
+    ScopedWakelock wakelock = createScopedWakelock(false /* locked */);
+
+    EXPECT_FALSE(wakelock.isLocked());
+}
+
+TEST_F(ScopedWakelockTest, ScopedLocked) {
+    { createScopedWakelock(true /* locked */); }
+
+    EXPECT_EQ(mRefCounter.incCount, 1);
+    EXPECT_EQ(mRefCounter.decCount, 1);
+}
+
+TEST_F(ScopedWakelockTest, ScopedUnlockIsNoop) {
+    { createScopedWakelock(false /* locked */); }
+
+    EXPECT_EQ(mRefCounter.incCount, 0);
+    EXPECT_EQ(mRefCounter.decCount, 0);
+}
+
+TEST_F(ScopedWakelockTest, ScopedLockedMove) {
+    {
+        ScopedWakelock wakelock = createScopedWakelock(true /* locked */);
+        ScopedWakelock movedWakelock(std::move(wakelock));
+    }
+
+    EXPECT_EQ(mRefCounter.incCount, 1);
+    EXPECT_EQ(mRefCounter.decCount, 1);
+}
+
+}  // namespace implementation
+}  // namespace V2_0
+}  // namespace sensors
+}  // namespace hardware
+}  // namespace android