From ec122eb46b6ce8f6e8bb3e08c34e575de666cd3e Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Wed, 16 Feb 2011 20:23:43 -0800 Subject: Fix some issues with RefBase debugging. First slipt sp<> out of RefBase into StrongPointer.h so it can be reused more easily and to make it clear that it doesn't require RefBase. Note: the rest of the change only affects the system when DEBUG_REFS is enabled. The main problem we fix here is that the owner id associated with each reference could get out of date when a sp<> or wp<> was moved, for instance when they're used in a Vector< >. We fix this issue by calling into RefBase::moveReferences from a template specialization for sp and wp of the type helpers. RefBase::moveReferences() has then a chance to update the owner ids. There is a little bit of trickery to implement this generically in RefBase, where we need to use a templatized functor that can turn a sp* casted to a void* into a RefBase*. Introduced a new debug option DEBUG_REFS_FATAL_SANITY_CHECKS currently set to 0 by default as there seem to be an issue with sp which trips the sanity checks. Change-Id: I4825b21c8ec47d4a0ef35d760760ae0c9cdfbd7f --- include/utils/RefBase.h | 277 +++++++++++++++--------------------------- include/utils/StrongPointer.h | 224 ++++++++++++++++++++++++++++++++++ include/utils/TypeHelpers.h | 13 -- 3 files changed, 325 insertions(+), 189 deletions(-) create mode 100644 include/utils/StrongPointer.h (limited to 'include/utils') diff --git a/include/utils/RefBase.h b/include/utils/RefBase.h index f8d96cf..9b0e7d8 100644 --- a/include/utils/RefBase.h +++ b/include/utils/RefBase.h @@ -22,16 +22,16 @@ #include #include #include +#include + +#include // --------------------------------------------------------------------------- namespace android { class TextOutput; -TextOutput& printStrongPointer(TextOutput& to, const void* val); TextOutput& printWeakPointer(TextOutput& to, const void* val); -template class wp; - // --------------------------------------------------------------------------- #define COMPARE_WEAK(_op_) \ @@ -50,15 +50,15 @@ inline bool operator _op_ (const U* o) const { \ return m_ptr _op_ o; \ } -#define COMPARE(_op_) \ -COMPARE_WEAK(_op_) \ -inline bool operator _op_ (const wp& o) const { \ - return m_ptr _op_ o.m_ptr; \ -} \ -template \ -inline bool operator _op_ (const wp& o) const { \ - return m_ptr _op_ o.m_ptr; \ -} +// --------------------------------------------------------------------------- + +class ReferenceMover; +class ReferenceConverterBase { +public: + virtual size_t getReferenceTypeSize() const = 0; + virtual void* getReferenceBase(void const*) const = 0; + inline virtual ~ReferenceConverterBase() { } +}; // --------------------------------------------------------------------------- @@ -115,6 +115,8 @@ public: getWeakRefs()->trackMe(enable, retain); } + typedef RefBase basetype; + protected: RefBase(); virtual ~RefBase(); @@ -138,6 +140,11 @@ protected: virtual void onLastWeakRef(const void* id); private: + friend class ReferenceMover; + static void moveReferences(void* d, void const* s, size_t n, + const ReferenceConverterBase& caster); + +private: friend class weakref_type; class weakref_impl; @@ -166,74 +173,21 @@ public: inline int32_t getStrongCount() const { return mCount; } - -protected: - inline ~LightRefBase() { } - -private: - mutable volatile int32_t mCount; -}; - -// --------------------------------------------------------------------------- - -template -class sp -{ -public: - typedef typename RefBase::weakref_type weakref_type; - - inline sp() : m_ptr(0) { } - - sp(T* other); - sp(const sp& other); - template sp(U* other); - template sp(const sp& other); - - ~sp(); - - // Assignment - sp& operator = (T* other); - sp& operator = (const sp& other); - - template sp& operator = (const sp& other); - template sp& operator = (U* other); - - //! Special optimization for use by ProcessState (and nobody else). - void force_set(T* other); - - // Reset - - void clear(); - - // Accessors + typedef LightRefBase basetype; - inline T& operator* () const { return *m_ptr; } - inline T* operator-> () const { return m_ptr; } - inline T* get() const { return m_ptr; } +protected: + inline ~LightRefBase() { } - // Operators - - COMPARE(==) - COMPARE(!=) - COMPARE(>) - COMPARE(<) - COMPARE(<=) - COMPARE(>=) - -private: - template friend class sp; - template friend class wp; +private: + friend class ReferenceMover; + inline static void moveReferences(void* d, void const* s, size_t n, + const ReferenceConverterBase& caster) { } - // Optimization for wp::promote(). - sp(T* p, weakref_type* refs); - - T* m_ptr; +private: + mutable volatile int32_t mCount; }; -template -TextOutput& operator<<(TextOutput& to, const sp& val); - // --------------------------------------------------------------------------- template @@ -329,113 +283,12 @@ private: template TextOutput& operator<<(TextOutput& to, const wp& val); -#undef COMPARE #undef COMPARE_WEAK // --------------------------------------------------------------------------- // No user serviceable parts below here. template -sp::sp(T* other) - : m_ptr(other) -{ - if (other) other->incStrong(this); -} - -template -sp::sp(const sp& other) - : m_ptr(other.m_ptr) -{ - if (m_ptr) m_ptr->incStrong(this); -} - -template template -sp::sp(U* other) : m_ptr(other) -{ - if (other) other->incStrong(this); -} - -template template -sp::sp(const sp& other) - : m_ptr(other.m_ptr) -{ - if (m_ptr) m_ptr->incStrong(this); -} - -template -sp::~sp() -{ - if (m_ptr) m_ptr->decStrong(this); -} - -template -sp& sp::operator = (const sp& other) { - T* otherPtr(other.m_ptr); - if (otherPtr) otherPtr->incStrong(this); - if (m_ptr) m_ptr->decStrong(this); - m_ptr = otherPtr; - return *this; -} - -template -sp& sp::operator = (T* other) -{ - if (other) other->incStrong(this); - if (m_ptr) m_ptr->decStrong(this); - m_ptr = other; - return *this; -} - -template template -sp& sp::operator = (const sp& other) -{ - U* otherPtr(other.m_ptr); - if (otherPtr) otherPtr->incStrong(this); - if (m_ptr) m_ptr->decStrong(this); - m_ptr = otherPtr; - return *this; -} - -template template -sp& sp::operator = (U* other) -{ - if (other) other->incStrong(this); - if (m_ptr) m_ptr->decStrong(this); - m_ptr = other; - return *this; -} - -template -void sp::force_set(T* other) -{ - other->forceIncStrong(this); - m_ptr = other; -} - -template -void sp::clear() -{ - if (m_ptr) { - m_ptr->decStrong(this); - m_ptr = 0; - } -} - -template -sp::sp(T* p, weakref_type* refs) - : m_ptr((p && refs->attemptIncStrong(this)) ? p : 0) -{ -} - -template -inline TextOutput& operator<<(TextOutput& to, const sp& val) -{ - return printStrongPointer(to, val.get()); -} - -// --------------------------------------------------------------------------- - -template wp::wp(T* other) : m_ptr(other) { @@ -572,7 +425,8 @@ void wp::set_object_and_refs(T* other, weakref_type* refs) template sp wp::promote() const { - return sp(m_ptr, m_refs); + T* p = (m_ptr && m_refs->attemptIncStrong(this)) ? m_ptr : 0; + return sp(p, true); } template @@ -590,6 +444,77 @@ inline TextOutput& operator<<(TextOutput& to, const wp& val) return printWeakPointer(to, val.unsafe_get()); } +// --------------------------------------------------------------------------- + +// this class just serves as a namespace so TYPE::moveReferences can stay +// private. + +class ReferenceMover { + // StrongReferenceCast and WeakReferenceCast do the impedance matching + // between the generic (void*) implementation in Refbase and the strongly typed + // template specializations below. + + template + struct StrongReferenceCast : public ReferenceConverterBase { + virtual size_t getReferenceTypeSize() const { return sizeof( sp ); } + virtual void* getReferenceBase(void const* p) const { + sp const* sptr(reinterpret_cast const*>(p)); + return static_cast(sptr->get()); + } + }; + + template + struct WeakReferenceCast : public ReferenceConverterBase { + virtual size_t getReferenceTypeSize() const { return sizeof( wp ); } + virtual void* getReferenceBase(void const* p) const { + wp const* sptr(reinterpret_cast const*>(p)); + return static_cast(sptr->unsafe_get()); + } + }; + +public: + template static inline + void move_references(sp* d, sp const* s, size_t n) { + memmove(d, s, n*sizeof(sp)); + StrongReferenceCast caster; + TYPE::moveReferences(d, s, n, caster); + } + template static inline + void move_references(wp* d, wp const* s, size_t n) { + memmove(d, s, n*sizeof(wp)); + WeakReferenceCast caster; + TYPE::moveReferences(d, s, n, caster); + } +}; + +// specialization for moving sp<> and wp<> types. +// these are used by the [Sorted|Keyed]Vector<> implementations +// sp<> and wp<> need to be handled specially, because they do not +// have trivial copy operation in the general case (see RefBase.cpp +// when DEBUG ops are enabled), but can be implemented very +// efficiently in most cases. + +template inline +void move_forward_type(sp* d, sp const* s, size_t n) { + ReferenceMover::move_references(d, s, n); +} + +template inline +void move_backward_type(sp* d, sp const* s, size_t n) { + ReferenceMover::move_references(d, s, n); +} + +template inline +void move_forward_type(wp* d, wp const* s, size_t n) { + ReferenceMover::move_references(d, s, n); +} + +template inline +void move_backward_type(wp* d, wp const* s, size_t n) { + ReferenceMover::move_references(d, s, n); +} + + }; // namespace android // --------------------------------------------------------------------------- diff --git a/include/utils/StrongPointer.h b/include/utils/StrongPointer.h new file mode 100644 index 0000000..5daccf4 --- /dev/null +++ b/include/utils/StrongPointer.h @@ -0,0 +1,224 @@ +/* + * Copyright (C) 2005 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. + */ + +#ifndef ANDROID_STRONG_POINTER_H +#define ANDROID_STRONG_POINTER_H + +#include + +#include +#include +#include + +// --------------------------------------------------------------------------- +namespace android { + +class TextOutput; +TextOutput& printStrongPointer(TextOutput& to, const void* val); + +template class wp; + +// --------------------------------------------------------------------------- + +#define COMPARE(_op_) \ +inline bool operator _op_ (const sp& o) const { \ + return m_ptr _op_ o.m_ptr; \ +} \ +inline bool operator _op_ (const T* o) const { \ + return m_ptr _op_ o; \ +} \ +template \ +inline bool operator _op_ (const sp& o) const { \ + return m_ptr _op_ o.m_ptr; \ +} \ +template \ +inline bool operator _op_ (const U* o) const { \ + return m_ptr _op_ o; \ +} \ +inline bool operator _op_ (const wp& o) const { \ + return m_ptr _op_ o.m_ptr; \ +} \ +template \ +inline bool operator _op_ (const wp& o) const { \ + return m_ptr _op_ o.m_ptr; \ +} + +// --------------------------------------------------------------------------- + +template +class sp +{ +public: + inline sp() : m_ptr(0) { } + + sp(T* other); + sp(const sp& other); + template sp(U* other); + template sp(const sp& other); + + ~sp(); + + // Assignment + + sp& operator = (T* other); + sp& operator = (const sp& other); + + template sp& operator = (const sp& other); + template sp& operator = (U* other); + + //! Special optimization for use by ProcessState (and nobody else). + void force_set(T* other); + + // Reset + + void clear(); + + // Accessors + + inline T& operator* () const { return *m_ptr; } + inline T* operator-> () const { return m_ptr; } + inline T* get() const { return m_ptr; } + + // Operators + + COMPARE(==) + COMPARE(!=) + COMPARE(>) + COMPARE(<) + COMPARE(<=) + COMPARE(>=) + +private: + template friend class sp; + template friend class wp; + + // Optimization for wp::promote(). + sp(T* p, bool); + + T* m_ptr; +}; + +#undef COMPARE + +template +TextOutput& operator<<(TextOutput& to, const sp& val); + +// --------------------------------------------------------------------------- +// No user serviceable parts below here. + +template +sp::sp(T* other) +: m_ptr(other) + { + if (other) other->incStrong(this); + } + +template +sp::sp(const sp& other) +: m_ptr(other.m_ptr) + { + if (m_ptr) m_ptr->incStrong(this); + } + +template template +sp::sp(U* other) : m_ptr(other) +{ + if (other) other->incStrong(this); +} + +template template +sp::sp(const sp& other) +: m_ptr(other.m_ptr) + { + if (m_ptr) m_ptr->incStrong(this); + } + +template +sp::~sp() +{ + if (m_ptr) m_ptr->decStrong(this); +} + +template +sp& sp::operator = (const sp& other) { + T* otherPtr(other.m_ptr); + if (otherPtr) otherPtr->incStrong(this); + if (m_ptr) m_ptr->decStrong(this); + m_ptr = otherPtr; + return *this; +} + +template +sp& sp::operator = (T* other) +{ + if (other) other->incStrong(this); + if (m_ptr) m_ptr->decStrong(this); + m_ptr = other; + return *this; +} + +template template +sp& sp::operator = (const sp& other) +{ + U* otherPtr(other.m_ptr); + if (otherPtr) otherPtr->incStrong(this); + if (m_ptr) m_ptr->decStrong(this); + m_ptr = otherPtr; + return *this; +} + +template template +sp& sp::operator = (U* other) +{ + if (other) other->incStrong(this); + if (m_ptr) m_ptr->decStrong(this); + m_ptr = other; + return *this; +} + +template +void sp::force_set(T* other) +{ + other->forceIncStrong(this); + m_ptr = other; +} + +template +void sp::clear() +{ + if (m_ptr) { + m_ptr->decStrong(this); + m_ptr = 0; + } +} + +template +sp::sp(T* p, bool) +: m_ptr(p) + { + } + +template +inline TextOutput& operator<<(TextOutput& to, const sp& val) +{ + return printStrongPointer(to, val.get()); +} + +}; // namespace android + +// --------------------------------------------------------------------------- + +#endif // ANDROID_STRONG_POINTER_H diff --git a/include/utils/TypeHelpers.h b/include/utils/TypeHelpers.h index 2ff2749..a1663f3 100644 --- a/include/utils/TypeHelpers.h +++ b/include/utils/TypeHelpers.h @@ -37,18 +37,6 @@ template struct trait_trivial_move { enum { value = false }; }; template struct trait_pointer { enum { value = false }; }; template struct trait_pointer { enum { value = true }; }; -// sp<> can be trivially moved -template class sp; -template struct trait_trivial_move< sp >{ - enum { value = true }; -}; - -// wp<> can be trivially moved -template class wp; -template struct trait_trivial_move< wp >{ - enum { value = true }; -}; - template struct traits { enum { @@ -217,7 +205,6 @@ void move_backward_type(TYPE* d, const TYPE* s, size_t n = 1) { } } - // --------------------------------------------------------------------------- /* -- cgit v1.1