Merge "libutils: LightRefBase: incStrongRequireStrong" am: 1c65d40154 am: b31a05bb47 am: 77b47c4185 am: 55862852fe

Original change: https://android-review.googlesource.com/c/platform/system/core/+/1696268

Change-Id: Ied8436e131d709a8f5e5b0c9bb34abffdb4cd7c3

libutils/Android.bp

@@ -142,6 +142,7 @@ cc_library {
         "Errors.cpp",
         "FileMap.cpp",
         "JenkinsHash.cpp",
+        "LightRefBase.cpp",
         "NativeHandle.cpp",
         "Printer.cpp",
         "RefBase.cpp",

libutils/LightRefBase.cpp

@@ -0,0 +1,29 @@
+/*
+ * Copyright (C) 2021 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.
+ */
+
+#define LOG_TAG "LightRefBase"
+
+#include <utils/LightRefBase.h>
+
+#include <log/log.h>
+
+namespace android {
+
+void LightRefBase_reportIncStrongRequireStrongFailed(const void* thiz) {
+    LOG_ALWAYS_FATAL("incStrongRequireStrong() called on %p which isn't already owned", thiz);
+}
+
+}  // namespace android

libutils/StrongPointer_test.cpp

@@ -30,17 +30,34 @@ class SPFoo : virtual public RefBase {
     ~SPFoo() {
         *mDeleted = true;
     }
-private:
+
+  private:
     bool* mDeleted;
 };
 
-TEST(StrongPointer, move) {
+class SPLightFoo : virtual public VirtualLightRefBase {
+  public:
+    explicit SPLightFoo(bool* deleted_check) : mDeleted(deleted_check) { *mDeleted = false; }
+
+    ~SPLightFoo() { *mDeleted = true; }
+
+  private:
+    bool* mDeleted;
+};
+
+template <typename T>
+class StrongPointer : public ::testing::Test {};
+
+using RefBaseTypes = ::testing::Types<SPFoo, SPLightFoo>;
+TYPED_TEST_CASE(StrongPointer, RefBaseTypes);
+
+TYPED_TEST(StrongPointer, move) {
     bool isDeleted;
-    sp<SPFoo> sp1 = sp<SPFoo>::make(&isDeleted);
-    SPFoo* foo = sp1.get();
+    sp<TypeParam> sp1 = sp<TypeParam>::make(&isDeleted);
+    TypeParam* foo = sp1.get();
     ASSERT_EQ(1, foo->getStrongCount());
     {
-        sp<SPFoo> sp2 = std::move(sp1);
+        sp<TypeParam> sp2 = std::move(sp1);
         ASSERT_EQ(1, foo->getStrongCount()) << "std::move failed, incremented refcnt";
         ASSERT_EQ(nullptr, sp1.get()) << "std::move failed, sp1 is still valid";
         // The strong count isn't increasing, let's double check the old object
@@ -50,33 +67,42 @@ TEST(StrongPointer, move) {
     ASSERT_FALSE(isDeleted) << "deleted too early! still has a reference!";
     {
         // Now let's double check it deletes on time
-        sp<SPFoo> sp2 = std::move(sp1);
+        sp<TypeParam> sp2 = std::move(sp1);
     }
     ASSERT_TRUE(isDeleted) << "foo was leaked!";
 }
 
-TEST(StrongPointer, NullptrComparison) {
-    sp<SPFoo> foo;
+TYPED_TEST(StrongPointer, NullptrComparison) {
+    sp<TypeParam> foo;
     ASSERT_EQ(foo, nullptr);
     ASSERT_EQ(nullptr, foo);
 }
 
-TEST(StrongPointer, PointerComparison) {
+TYPED_TEST(StrongPointer, PointerComparison) {
     bool isDeleted;
-    sp<SPFoo> foo = sp<SPFoo>::make(&isDeleted);
+    sp<TypeParam> foo = sp<TypeParam>::make(&isDeleted);
     ASSERT_EQ(foo.get(), foo);
     ASSERT_EQ(foo, foo.get());
     ASSERT_NE(nullptr, foo);
     ASSERT_NE(foo, nullptr);
 }
 
-TEST(StrongPointer, AssertStrongRefExists) {
-    // uses some other refcounting method, or non at all
+TYPED_TEST(StrongPointer, Deleted) {
     bool isDeleted;
-    SPFoo* foo = new SPFoo(&isDeleted);
+    sp<TypeParam> foo = sp<TypeParam>::make(&isDeleted);
 
-    // can only get a valid sp<> object when you construct it as an sp<> object
-    EXPECT_DEATH(sp<SPFoo>::fromExisting(foo), "");
+    auto foo2 = sp<TypeParam>::fromExisting(foo.get());
 
+    EXPECT_FALSE(isDeleted);
+    foo = nullptr;
+    EXPECT_FALSE(isDeleted);
+    foo2 = nullptr;
+    EXPECT_TRUE(isDeleted);
+}
+
+TYPED_TEST(StrongPointer, AssertStrongRefExists) {
+    bool isDeleted;
+    TypeParam* foo = new TypeParam(&isDeleted);
+    EXPECT_DEATH(sp<TypeParam>::fromExisting(foo), "");
     delete foo;
 }

libutils/include/utils/LightRefBase.h

@@ -28,6 +28,8 @@ namespace android {
 
 class ReferenceRenamer;
 
+void LightRefBase_reportIncStrongRequireStrongFailed(const void* thiz);
+
 template <class T>
 class LightRefBase
 {
@@ -36,6 +38,11 @@ public:
     inline void incStrong(__attribute__((unused)) const void* id) const {
         mCount.fetch_add(1, std::memory_order_relaxed);
     }
+    inline void incStrongRequireStrong(__attribute__((unused)) const void* id) const {
+        if (0 == mCount.fetch_add(1, std::memory_order_relaxed)) {
+            LightRefBase_reportIncStrongRequireStrongFailed(this);
+        }
+    }
     inline void decStrong(__attribute__((unused)) const void* id) const {
         if (mCount.fetch_sub(1, std::memory_order_release) == 1) {
             std::atomic_thread_fence(std::memory_order_acquire);
@@ -59,7 +66,6 @@ private:
     mutable std::atomic<int32_t> mCount;
 };
 
-
 // This is a wrapper around LightRefBase that simply enforces a virtual
 // destructor to eliminate the template requirement of LightRefBase
 class VirtualLightRefBase : public LightRefBase<VirtualLightRefBase> {