commit da75cef983fb1dc44c6221dbd35f670a885046e5
author Steven Moreland <smoreland@google.com> Wed Mar 31 16:05:04 2021 +0000
committer Steven Moreland <smoreland@google.com> Mon Apr 05 17:45:18 2021 +0000
tree 6612bdfa77f6a8c1c30d922fb8e372a7476cd0a3
parent 767f264fa4a8f019f6acfce8290e10f18113a524

ANDROID_UTILS_REF_BASE_DISABLE_IMPLICIT_CONSTRUCTION

In form, inspired by ANDROID_BASE_UNIQUE_FD_DISABLE_IMPLICIT_CONVERSION.

We get occasional bugs about sp double-ownership. When this flag is
enabled, we have:
- you must construct RefBase objects using sp<>::make
- you must construct wp<> objects by converting them to sp<>
- if you want to convert a raw pointer to an sp<> object (this is
  possible since the refcount is used internally, and is used commonly
  on this*), then you must use 'assertStrongRefExists' semantics which
  aborts if there is no strong ref held. That is, if a client uses
  std::make_shared and then calls a function which internally used to
  call `sp<T>(this)`, you would now call
  `sp<T>::assertStrongRefExists(this)`, and the double ownership
  problem would become a runtime error.

Bug: 184190315
Test: libutils_test
Change-Id: Ie18d3146420df1808e3733027070ec234dda4e9d

libutils/RefBase.cpp

diff --git a/libutils/RefBase.cpp b/libutils/RefBase.cpp
index 8e45226..b57e287 100644
--- a/libutils/RefBase.cpp
+++ b/libutils/RefBase.cpp
@@ -443,6 +443,20 @@
     refs->mBase->onFirstRef();
 }
 
+void RefBase::incStrongRequireStrong(const void* id) const {
+    weakref_impl* const refs = mRefs;
+    refs->incWeak(id);
+
+    refs->addStrongRef(id);
+    const int32_t c = refs->mStrong.fetch_add(1, std::memory_order_relaxed);
+
+    LOG_ALWAYS_FATAL_IF(c <= 0 || c == INITIAL_STRONG_VALUE,
+                        "incStrongRequireStrong() called on %p which isn't already owned", refs);
+#if PRINT_REFS
+    ALOGD("incStrong (requiring strong) of %p from %p: cnt=%d\n", this, id, c);
+#endif
+}
+
 void RefBase::decStrong(const void* id) const
 {
     weakref_impl* const refs = mRefs;
@@ -521,6 +535,14 @@
     ALOG_ASSERT(c >= 0, "incWeak called on %p after last weak ref", this);
 }
 
+void RefBase::weakref_type::incWeakRequireWeak(const void* id)
+{
+    weakref_impl* const impl = static_cast<weakref_impl*>(this);
+    impl->addWeakRef(id);
+    const int32_t c __unused = impl->mWeak.fetch_add(1,
+            std::memory_order_relaxed);
+    LOG_ALWAYS_FATAL_IF(c <= 0, "incWeakRequireWeak called on %p which has no weak refs", this);
+}
 
 void RefBase::weakref_type::decWeak(const void* id)
 {