Merge "ANDROID_UTILS_REF_BASE_DISABLE_IMPLICIT_CONSTRUCTION" am: 99037347d6 (d8c4c860) · Commits · e / os / android_system_core

libutils/RefBase.cpp

+22 −0

Original line number	Diff line number	Diff line
		@@ -443,6 +443,20 @@ void RefBase::incStrong(const void* id) const
		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 @@ void RefBase::weakref_type::incWeak(const void* id)
		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)
		{

libutils/RefBase_test.cpp

+24 −0

Original line number	Diff line number	Diff line
		@@ -241,6 +241,30 @@ TEST(RefBase, ReplacedComparison) {
		ASSERT_FALSE(wp1 != wp2);
		}

		TEST(RefBase, AssertWeakRefExistsSuccess) {
		// uses some other refcounting method, or non at all
		bool isDeleted;
		sp<Foo> foo = sp<Foo>::make(&isDeleted);
		wp<Foo> weakFoo = foo;

		EXPECT_EQ(weakFoo, wp<Foo>::fromExisting(foo.get()));

		EXPECT_FALSE(isDeleted);
		foo = nullptr;
		EXPECT_TRUE(isDeleted);
		}

		TEST(RefBase, AssertWeakRefExistsDeath) {
		// uses some other refcounting method, or non at all
		bool isDeleted;
		Foo* foo = new Foo(&isDeleted);

		// can only get a valid wp<> object when you construct it from an sp<>
		EXPECT_DEATH(wp<Foo>::fromExisting(foo), "");

		delete foo;
		}

		// Set up a situation in which we race with visit2AndRremove() to delete
		// 2 strong references. Bar destructor checks that there are no early
		// deletions and prior updates are visible to destructor.

libutils/StrongPointer_test.cpp

+13 −2

Original line number	Diff line number	Diff line
		@@ -21,7 +21,7 @@

		using namespace android;

		class SPFoo : public LightRefBase<SPFoo> {
		class SPFoo : virtual public RefBase {
		public:
		explicit SPFoo(bool* deleted_check) : mDeleted(deleted_check) {
		*mDeleted = false;
		@@ -69,3 +69,14 @@ TEST(StrongPointer, PointerComparison) {
		ASSERT_NE(nullptr, foo);
		ASSERT_NE(foo, nullptr);
		}

		TEST(StrongPointer, AssertStrongRefExists) {
		// uses some other refcounting method, or non at all
		bool isDeleted;
		SPFoo* foo = new SPFoo(&isDeleted);

		// can only get a valid sp<> object when you construct it as an sp<> object
		EXPECT_DEATH(sp<SPFoo>::fromExisting(foo), "");

		delete foo;
		}

libutils/include/utils/RefBase.h

+79 −1

Original line number	Diff line number	Diff line
		@@ -140,7 +140,9 @@
		// count, and accidentally passed to f(sp<T>), a strong pointer to the object
		// will be temporarily constructed and destroyed, prematurely deallocating the
		// object, and resulting in heap corruption. None of this would be easily
		// visible in the source.
		// visible in the source. See below on
		// ANDROID_UTILS_REF_BASE_DISABLE_IMPLICIT_CONSTRUCTION for a compile time
		// option which helps avoid this case.

		// Extra Features:

		@@ -167,6 +169,42 @@
		// to THE SAME sp<> or wp<>. In effect, their thread-safety properties are
		// exactly like those of T, NOT atomic<T>.

		// Safety option: ANDROID_UTILS_REF_BASE_DISABLE_IMPLICIT_CONSTRUCTION
		//
		// This flag makes the semantics for using a RefBase object with wp<> and sp<>
		// much stricter by disabling implicit conversion from raw pointers to these
		// objects. In order to use this, apply this flag in Android.bp like so:
		//
		// cflags: [
		// "-DANDROID_UTILS_REF_BASE_DISABLE_IMPLICIT_CONSTRUCTION",
		// ],
		//
		// REGARDLESS of whether this flag is on, best usage of sp<> is shown below. If
		// this flag is on, no other usage is possible (directly calling RefBase methods
		// is possible, but seeing code using 'incStrong' instead of 'sp<>', for
		// instance, should already set off big alarm bells. With carefully constructed
		// data structures, it should NEVER be necessary to directly use RefBase
		// methods). Proper RefBase usage:
		//
		// class Foo : virtual public RefBase { ... };
		//
		// // always construct an sp object with sp::make
		// sp<Foo> myFoo = sp<Foo>::make(/args/);
		//
		// // if you need a weak pointer, it must be constructed from a strong
		// // pointer
		// wp<Foo> weakFoo = myFoo; // NOT myFoo.get()
		//
		// // If you are inside of a method of Foo and need access to a strong
		// // explicitly call this function. This documents your intention to code
		// // readers, and it will give a runtime error for what otherwise would
		// // be potential double ownership
		// .... Foo::someMethod(...) {
		// // asserts if there is a memory issue
		// sp<Foo> thiz = sp<Foo>::fromExisting(this);
		// }
		//

		#ifndef ANDROID_REF_BASE_H
		#define ANDROID_REF_BASE_H

		@@ -244,6 +282,7 @@ class RefBase
		{
		public:
		void incStrong(const void* id) const;
		void incStrongRequireStrong(const void* id) const;
		void decStrong(const void* id) const;

		void forceIncStrong(const void* id) const;
		@@ -257,6 +296,7 @@ public:
		RefBase* refBase() const;

		void incWeak(const void* id);
		void incWeakRequireWeak(const void* id);
		void decWeak(const void* id);

		// acquires a strong reference if there is already one.
		@@ -365,10 +405,24 @@ public:

		inline wp() : m_ptr(nullptr), m_refs(nullptr) { }

		// if nullptr, returns nullptr
		//
		// if a weak pointer is already available, this will retrieve it,
		// otherwise, this will abort
		static inline wp<T> fromExisting(T* other);

		// for more information about this flag, see above
		#if defined(ANDROID_UTILS_REF_BASE_DISABLE_IMPLICIT_CONSTRUCTION)
		wp(std::nullptr_t) : wp() {}
		#else
		wp(T* other); // NOLINT(implicit)
		#endif
		wp(const wp<T>& other);
		explicit wp(const sp<T>& other);

		#if !defined(ANDROID_UTILS_REF_BASE_DISABLE_IMPLICIT_CONSTRUCTION)
		template<typename U> wp(U* other); // NOLINT(implicit)
		#endif
		template<typename U> wp(const sp<U>& other); // NOLINT(implicit)
		template<typename U> wp(const wp<U>& other); // NOLINT(implicit)

		@@ -376,11 +430,15 @@ public:

		// Assignment

		#if !defined(ANDROID_UTILS_REF_BASE_DISABLE_IMPLICIT_CONSTRUCTION)
		wp& operator = (T* other);
		#endif
		wp& operator = (const wp<T>& other);
		wp& operator = (const sp<T>& other);

		#if !defined(ANDROID_UTILS_REF_BASE_DISABLE_IMPLICIT_CONSTRUCTION)
		template<typename U> wp& operator = (U* other);
		#endif
		template<typename U> wp& operator = (const wp<U>& other);
		template<typename U> wp& operator = (const sp<U>& other);

		@@ -481,12 +539,26 @@ private:
		// Note that the above comparison operations go out of their way to provide an ordering consistent
		// with ordinary pointer comparison; otherwise they could ignore m_ptr, and just compare m_refs.

		template <typename T>
		wp<T> wp<T>::fromExisting(T* other) {
		if (!other) return nullptr;

		auto refs = other->getWeakRefs();
		refs->incWeakRequireWeak(other);

		wp<T> ret;
		ret.m_refs = refs;
		return ret;
		}

		#if !defined(ANDROID_UTILS_REF_BASE_DISABLE_IMPLICIT_CONSTRUCTION)
		template<typename T>
		wp<T>::wp(T* other)
		: m_ptr(other)
		{
		m_refs = other ? m_refs = other->createWeak(this) : nullptr;
		}
		#endif

		template<typename T>
		wp<T>::wp(const wp<T>& other)
		@@ -502,12 +574,14 @@ wp<T>::wp(const sp<T>& other)
		m_refs = m_ptr ? m_ptr->createWeak(this) : nullptr;
		}

		#if !defined(ANDROID_UTILS_REF_BASE_DISABLE_IMPLICIT_CONSTRUCTION)
		template<typename T> template<typename U>
		wp<T>::wp(U* other)
		: m_ptr(other)
		{
		m_refs = other ? other->createWeak(this) : nullptr;
		}
		#endif

		template<typename T> template<typename U>
		wp<T>::wp(const wp<U>& other)
		@@ -534,6 +608,7 @@ wp<T>::~wp()
		if (m_ptr) m_refs->decWeak(this);
		}

		#if !defined(ANDROID_UTILS_REF_BASE_DISABLE_IMPLICIT_CONSTRUCTION)
		template<typename T>
		wp<T>& wp<T>::operator = (T* other)
		{
		@@ -544,6 +619,7 @@ wp<T>& wp<T>::operator = (T* other)
		m_refs = newRefs;
		return *this;
		}
		#endif

		template<typename T>
		wp<T>& wp<T>::operator = (const wp<T>& other)
		@@ -569,6 +645,7 @@ wp<T>& wp<T>::operator = (const sp<T>& other)
		return *this;
		}

		#if !defined(ANDROID_UTILS_REF_BASE_DISABLE_IMPLICIT_CONSTRUCTION)
		template<typename T> template<typename U>
		wp<T>& wp<T>::operator = (U* other)
		{
		@@ -579,6 +656,7 @@ wp<T>& wp<T>::operator = (U* other)
		m_refs = newRefs;
		return *this;
		}
		#endif

		template<typename T> template<typename U>
		wp<T>& wp<T>::operator = (const wp<U>& other)

libutils/include/utils/StrongPointer.h

+39 −0

Original line number	Diff line number	Diff line
		@@ -50,10 +50,25 @@ public:
		template <typename... Args>
		static inline sp<T> make(Args&&... args);

		// if nullptr, returns nullptr
		//
		// if a strong pointer is already available, this will retrieve it,
		// otherwise, this will abort
		static inline sp<T> fromExisting(T* other);

		// for more information about this macro and correct RefBase usage, see
		// the comment at the top of utils/RefBase.h
		#if defined(ANDROID_UTILS_REF_BASE_DISABLE_IMPLICIT_CONSTRUCTION)
		sp(std::nullptr_t) : sp() {}
		#else
		sp(T* other); // NOLINT(implicit)
		#endif
		sp(const sp<T>& other);
		sp(sp<T>&& other) noexcept;

		#if !defined(ANDROID_UTILS_REF_BASE_DISABLE_IMPLICIT_CONSTRUCTION)
		template<typename U> sp(U* other); // NOLINT(implicit)
		#endif
		template<typename U> sp(const sp<U>& other); // NOLINT(implicit)
		template<typename U> sp(sp<U>&& other); // NOLINT(implicit)

		@@ -61,13 +76,17 @@ public:

		// Assignment

		#if !defined(ANDROID_UTILS_REF_BASE_DISABLE_IMPLICIT_CONSTRUCTION)
		sp& operator = (T* other);
		#endif
		sp& operator = (const sp<T>& other);
		sp& operator=(sp<T>&& other) noexcept;

		template<typename U> sp& operator = (const sp<U>& other);
		template<typename U> sp& operator = (sp<U>&& other);
		#if !defined(ANDROID_UTILS_REF_BASE_DISABLE_IMPLICIT_CONSTRUCTION)
		template<typename U> sp& operator = (U* other);
		#endif

		//! Special optimization for use by ProcessState (and nobody else).
		void force_set(T* other);
		@@ -201,6 +220,19 @@ sp<T> sp<T>::make(Args&&... args) {
		return result;
		}

		template <typename T>
		sp<T> sp<T>::fromExisting(T* other) {
		if (other) {
		check_not_on_stack(other);
		other->incStrongRequireStrong(other);
		sp<T> result;
		result.m_ptr = other;
		return result;
		}
		return nullptr;
		}

		#if !defined(ANDROID_UTILS_REF_BASE_DISABLE_IMPLICIT_CONSTRUCTION)
		template<typename T>
		sp<T>::sp(T* other)
		: m_ptr(other) {
		@@ -209,6 +241,7 @@ sp<T>::sp(T* other)
		other->incStrong(this);
		}
		}
		#endif

		template<typename T>
		sp<T>::sp(const sp<T>& other)
		@@ -222,6 +255,7 @@ sp<T>::sp(sp<T>&& other) noexcept : m_ptr(other.m_ptr) {
		other.m_ptr = nullptr;
		}

		#if !defined(ANDROID_UTILS_REF_BASE_DISABLE_IMPLICIT_CONSTRUCTION)
		template<typename T> template<typename U>
		sp<T>::sp(U* other)
		: m_ptr(other) {
		@@ -230,6 +264,7 @@ sp<T>::sp(U* other)
		(static_cast<T*>(other))->incStrong(this);
		}
		}
		#endif

		template<typename T> template<typename U>
		sp<T>::sp(const sp<U>& other)
		@@ -272,6 +307,7 @@ sp<T>& sp<T>::operator=(sp<T>&& other) noexcept {
		return *this;
		}

		#if !defined(ANDROID_UTILS_REF_BASE_DISABLE_IMPLICIT_CONSTRUCTION)
		template<typename T>
		sp<T>& sp<T>::operator =(T* other) {
		T* oldPtr(const_cast<T volatile*>(&m_ptr));
		@@ -284,6 +320,7 @@ sp<T>& sp<T>::operator =(T* other) {
		m_ptr = other;
		return *this;
		}
		#endif

		template<typename T> template<typename U>
		sp<T>& sp<T>::operator =(const sp<U>& other) {
		@@ -306,6 +343,7 @@ sp<T>& sp<T>::operator =(sp<U>&& other) {
		return *this;
		}

		#if !defined(ANDROID_UTILS_REF_BASE_DISABLE_IMPLICIT_CONSTRUCTION)
		template<typename T> template<typename U>
		sp<T>& sp<T>::operator =(U* other) {
		T* oldPtr(const_cast<T volatile*>(&m_ptr));
		@@ -315,6 +353,7 @@ sp<T>& sp<T>::operator =(U* other) {
		m_ptr = other;
		return *this;
		}
		#endif

		template<typename T>
		void sp<T>::force_set(T* other) {