diff --git a/libutils/Android.bp b/libutils/Android.bp index df761a8dc..be8c86979 100644 --- a/libutils/Android.bp +++ b/libutils/Android.bp @@ -142,6 +142,7 @@ cc_library { "Errors.cpp", "FileMap.cpp", "JenkinsHash.cpp", + "LightRefBase.cpp", "NativeHandle.cpp", "Printer.cpp", "RefBase.cpp", diff --git a/libutils/LightRefBase.cpp b/libutils/LightRefBase.cpp new file mode 100644 index 000000000..e08ffecfc --- /dev/null +++ b/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 + +#include + +namespace android { + +void LightRefBase_reportIncStrongRequireStrongFailed(const void* thiz) { + LOG_ALWAYS_FATAL("incStrongRequireStrong() called on %p which isn't already owned", thiz); +} + +} // namespace android diff --git a/libutils/StrongPointer_test.cpp b/libutils/StrongPointer_test.cpp index 29f6bd4bb..f27c1f1cc 100644 --- a/libutils/StrongPointer_test.cpp +++ b/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 +class StrongPointer : public ::testing::Test {}; + +using RefBaseTypes = ::testing::Types; +TYPED_TEST_CASE(StrongPointer, RefBaseTypes); + +TYPED_TEST(StrongPointer, move) { bool isDeleted; - sp sp1 = sp::make(&isDeleted); - SPFoo* foo = sp1.get(); + sp sp1 = sp::make(&isDeleted); + TypeParam* foo = sp1.get(); ASSERT_EQ(1, foo->getStrongCount()); { - sp sp2 = std::move(sp1); + sp 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 sp2 = std::move(sp1); + sp sp2 = std::move(sp1); } ASSERT_TRUE(isDeleted) << "foo was leaked!"; } -TEST(StrongPointer, NullptrComparison) { - sp foo; +TYPED_TEST(StrongPointer, NullptrComparison) { + sp foo; ASSERT_EQ(foo, nullptr); ASSERT_EQ(nullptr, foo); } -TEST(StrongPointer, PointerComparison) { +TYPED_TEST(StrongPointer, PointerComparison) { bool isDeleted; - sp foo = sp::make(&isDeleted); + sp foo = sp::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 foo = sp::make(&isDeleted); - // can only get a valid sp<> object when you construct it as an sp<> object - EXPECT_DEATH(sp::fromExisting(foo), ""); + auto foo2 = sp::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::fromExisting(foo), ""); delete foo; } diff --git a/libutils/include/utils/LightRefBase.h b/libutils/include/utils/LightRefBase.h index b04e5c155..40edf6718 100644 --- a/libutils/include/utils/LightRefBase.h +++ b/libutils/include/utils/LightRefBase.h @@ -28,6 +28,8 @@ namespace android { class ReferenceRenamer; +void LightRefBase_reportIncStrongRequireStrongFailed(const void* thiz); + template 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 mCount; }; - // This is a wrapper around LightRefBase that simply enforces a virtual // destructor to eliminate the template requirement of LightRefBase class VirtualLightRefBase : public LightRefBase {