summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMathias Agopian <mathias@google.com>2013-03-18 22:27:41 -0700
committerAlex Ray <aray@google.com>2013-07-30 13:57:01 -0700
commit6cd548c7154c1633a0ed318c31dd22c50a5f5d02 (patch)
treece9928ccfb45dac23ead4ddae0da0253b0cdd2fe
parent6d4419d9b130719dd2355861907dc8f87368a51c (diff)
downloadsystem_core-6cd548c7154c1633a0ed318c31dd22c50a5f5d02.zip
system_core-6cd548c7154c1633a0ed318c31dd22c50a5f5d02.tar.gz
system_core-6cd548c7154c1633a0ed318c31dd22c50a5f5d02.tar.bz2
Fix a crasher with RefBase debugging and vectors of wp<>
background: we have some code to fix-up the IDs of references when using RefBase's DEBUG_REFS when those refs are managed by arrays wp<> or sp<> (this is because wp<> / sp<> don't have a trivial ctor when DEBUG_REFS is enabled, and Vector treats them as trivial for obvious performance reasons) this is complicated by the fact that we don't want to have to recompile everything when enabling DEBUG_REFs (i.e.: the Vector code cannot know wheter it's enabled or not for its template stuff). problem: there was a bug in the fix-up code for wp<> which was trying to access the weakref_impl from the RefBase* however, this was moronic since RefBase could have been destroyed if there wasn't any more strong refs -- and this happned. Instead we need to get the weakref_impl directly from the wp<> Change-Id: Ie16e334204205fdbff142acb9faff8479a78450b
-rw-r--r--include/utils/RefBase.h95
-rw-r--r--libs/utils/RefBase.cpp24
2 files changed, 71 insertions, 48 deletions
diff --git a/include/utils/RefBase.h b/include/utils/RefBase.h
index 8e304e7..cbfe13a 100644
--- a/include/utils/RefBase.h
+++ b/include/utils/RefBase.h
@@ -52,12 +52,16 @@ inline bool operator _op_ (const U* o) const { \
}
// ---------------------------------------------------------------------------
-class ReferenceMover;
-class ReferenceConverterBase {
+
+class ReferenceRenamer {
+protected:
+ // destructor is purposedly not virtual so we avoid code overhead from
+ // subclasses; we have to make it protected to guarantee that it
+ // cannot be called from this base class (and to make strict compilers
+ // happy).
+ ~ReferenceRenamer() { }
public:
- virtual size_t getReferenceTypeSize() const = 0;
- virtual void* getReferenceBase(void const*) const = 0;
- inline virtual ~ReferenceConverterBase() { }
+ virtual void operator()(size_t i) const = 0;
};
// ---------------------------------------------------------------------------
@@ -144,17 +148,23 @@ 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;
RefBase(const RefBase& o);
RefBase& operator=(const RefBase& o);
+private:
+ friend class ReferenceMover;
+
+ static void renameRefs(size_t n, const ReferenceRenamer& renamer);
+
+ static void renameRefId(weakref_type* ref,
+ const void* old_id, const void* new_id);
+
+ static void renameRefId(RefBase* ref,
+ const void* old_id, const void* new_id);
+
weakref_impl* const mRefs;
};
@@ -185,8 +195,9 @@ protected:
private:
friend class ReferenceMover;
- inline static void moveReferences(void* d, void const* s, size_t n,
- const ReferenceConverterBase& caster) { }
+ inline static void renameRefs(size_t n, const ReferenceRenamer& renamer) { }
+ inline static void renameRefId(T* ref,
+ const void* old_id, const void* new_id) { }
private:
mutable volatile int32_t mCount;
@@ -455,42 +466,48 @@ inline TextOutput& operator<<(TextOutput& to, const wp<T>& val)
// 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 <typename TYPE>
- struct StrongReferenceCast : public ReferenceConverterBase {
- virtual size_t getReferenceTypeSize() const { return sizeof( sp<TYPE> ); }
- virtual void* getReferenceBase(void const* p) const {
- sp<TYPE> const* sptr(reinterpret_cast<sp<TYPE> const*>(p));
- return static_cast<typename TYPE::basetype *>(sptr->get());
- }
- };
-
- template <typename TYPE>
- struct WeakReferenceCast : public ReferenceConverterBase {
- virtual size_t getReferenceTypeSize() const { return sizeof( wp<TYPE> ); }
- virtual void* getReferenceBase(void const* p) const {
- wp<TYPE> const* sptr(reinterpret_cast<wp<TYPE> const*>(p));
- return static_cast<typename TYPE::basetype *>(sptr->unsafe_get());
- }
- };
-
public:
+ // it would be nice if we could make sure no extra code is generated
+ // for sp<TYPE> or wp<TYPE> when TYPE is a descendant of RefBase:
+ // Using a sp<RefBase> override doesn't work; it's a bit like we wanted
+ // a template<typename TYPE inherits RefBase> template...
+
template<typename TYPE> static inline
void move_references(sp<TYPE>* d, sp<TYPE> const* s, size_t n) {
+
+ class Renamer : public ReferenceRenamer {
+ sp<TYPE>* d;
+ sp<TYPE> const* s;
+ virtual void operator()(size_t i) const {
+ // The id are known to be the sp<>'s this pointer
+ TYPE::renameRefId(d[i].get(), &s[i], &d[i]);
+ }
+ public:
+ Renamer(sp<TYPE>* d, sp<TYPE> const* s) : s(s), d(d) { }
+ };
+
memmove(d, s, n*sizeof(sp<TYPE>));
- StrongReferenceCast<TYPE> caster;
- TYPE::moveReferences(d, s, n, caster);
+ TYPE::renameRefs(n, Renamer(d, s));
}
+
+
template<typename TYPE> static inline
void move_references(wp<TYPE>* d, wp<TYPE> const* s, size_t n) {
+
+ class Renamer : public ReferenceRenamer {
+ wp<TYPE>* d;
+ wp<TYPE> const* s;
+ virtual void operator()(size_t i) const {
+ // The id are known to be the wp<>'s this pointer
+ TYPE::renameRefId(d[i].get_refs(), &s[i], &d[i]);
+ }
+ public:
+ Renamer(wp<TYPE>* d, wp<TYPE> const* s) : s(s), d(d) { }
+ };
+
memmove(d, s, n*sizeof(wp<TYPE>));
- WeakReferenceCast<TYPE> caster;
- TYPE::moveReferences(d, s, n, caster);
+ TYPE::renameRefs(n, Renamer(d, s));
}
};
diff --git a/libs/utils/RefBase.cpp b/libs/utils/RefBase.cpp
index ef87131..abaf3c0 100644
--- a/libs/utils/RefBase.cpp
+++ b/libs/utils/RefBase.cpp
@@ -631,21 +631,27 @@ void RefBase::onLastWeakRef(const void* /*id*/)
// ---------------------------------------------------------------------------
-void RefBase::moveReferences(void* dst, void const* src, size_t n,
- const ReferenceConverterBase& caster)
-{
+void RefBase::renameRefs(size_t n, const ReferenceRenamer& renamer) {
#if DEBUG_REFS
- const size_t itemSize = caster.getReferenceTypeSize();
for (size_t i=0 ; i<n ; i++) {
- void* d = reinterpret_cast<void *>(intptr_t(dst) + i*itemSize);
- void const* s = reinterpret_cast<void const*>(intptr_t(src) + i*itemSize);
- RefBase* ref(reinterpret_cast<RefBase*>(caster.getReferenceBase(d)));
- ref->mRefs->renameStrongRefId(s, d);
- ref->mRefs->renameWeakRefId(s, d);
+ renamer(i);
}
#endif
}
+void RefBase::renameRefId(weakref_type* ref,
+ const void* old_id, const void* new_id) {
+ weakref_impl* const impl = static_cast<weakref_impl*>(ref);
+ impl->renameStrongRefId(old_id, new_id);
+ impl->renameWeakRefId(old_id, new_id);
+}
+
+void RefBase::renameRefId(RefBase* ref,
+ const void* old_id, const void* new_id) {
+ ref->mRefs->renameStrongRefId(old_id, new_id);
+ ref->mRefs->renameWeakRefId(old_id, new_id);
+}
+
// ---------------------------------------------------------------------------
TextOutput& printStrongPointer(TextOutput& to, const void* val)