Merge "Fix memory order and race bugs in Refbase.h & RefBase.cpp"
This commit is contained in:
commit
62212954ef
|
@ -17,7 +17,7 @@
|
|||
#ifndef ANDROID_REF_BASE_H
|
||||
#define ANDROID_REF_BASE_H
|
||||
|
||||
#include <cutils/atomic.h>
|
||||
#include <atomic>
|
||||
|
||||
#include <stdint.h>
|
||||
#include <sys/types.h>
|
||||
|
@ -176,16 +176,17 @@ class LightRefBase
|
|||
public:
|
||||
inline LightRefBase() : mCount(0) { }
|
||||
inline void incStrong(__attribute__((unused)) const void* id) const {
|
||||
android_atomic_inc(&mCount);
|
||||
mCount.fetch_add(1, std::memory_order_relaxed);
|
||||
}
|
||||
inline void decStrong(__attribute__((unused)) const void* id) const {
|
||||
if (android_atomic_dec(&mCount) == 1) {
|
||||
if (mCount.fetch_sub(1, std::memory_order_release) == 1) {
|
||||
std::atomic_thread_fence(std::memory_order_acquire);
|
||||
delete static_cast<const T*>(this);
|
||||
}
|
||||
}
|
||||
//! DEBUGGING ONLY: Get current strong ref count.
|
||||
inline int32_t getStrongCount() const {
|
||||
return mCount;
|
||||
return mCount.load(std::memory_order_relaxed);
|
||||
}
|
||||
|
||||
typedef LightRefBase<T> basetype;
|
||||
|
@ -200,7 +201,7 @@ private:
|
|||
const void* old_id, const void* new_id) { }
|
||||
|
||||
private:
|
||||
mutable volatile int32_t mCount;
|
||||
mutable std::atomic<int32_t> mCount;
|
||||
};
|
||||
|
||||
// This is a wrapper around LightRefBase that simply enforces a virtual
|
||||
|
|
|
@ -27,7 +27,6 @@
|
|||
|
||||
#include <utils/RefBase.h>
|
||||
|
||||
#include <utils/Atomic.h>
|
||||
#include <utils/CallStack.h>
|
||||
#include <utils/Log.h>
|
||||
#include <utils/threads.h>
|
||||
|
@ -57,6 +56,68 @@
|
|||
|
||||
namespace android {
|
||||
|
||||
// Usage, invariants, etc:
|
||||
|
||||
// It is normally OK just to keep weak pointers to an object. The object will
|
||||
// be deallocated by decWeak when the last weak reference disappears.
|
||||
// Once a a strong reference has been created, the object will disappear once
|
||||
// the last strong reference does (decStrong).
|
||||
// AttemptIncStrong will succeed if the object has a strong reference, or if it
|
||||
// has a weak reference and has never had a strong reference.
|
||||
// AttemptIncWeak really does succeed only if there is already a WEAK
|
||||
// reference, and thus may fail when attemptIncStrong would succeed.
|
||||
// OBJECT_LIFETIME_WEAK changes this behavior to retain the object
|
||||
// unconditionally until the last reference of either kind disappears. The
|
||||
// client ensures that the extendObjectLifetime call happens before the dec
|
||||
// call that would otherwise have deallocated the object, or before an
|
||||
// attemptIncStrong call that might rely on it. We do not worry about
|
||||
// concurrent changes to the object lifetime.
|
||||
// mStrong is the strong reference count. mWeak is the weak reference count.
|
||||
// Between calls, and ignoring memory ordering effects, mWeak includes strong
|
||||
// references, and is thus >= mStrong.
|
||||
//
|
||||
// A weakref_impl is allocated as the value of mRefs in a RefBase object on
|
||||
// construction.
|
||||
// In the OBJECT_LIFETIME_STRONG case, it is deallocated in the RefBase
|
||||
// destructor iff the strong reference count was never incremented. The
|
||||
// destructor can be invoked either from decStrong, or from decWeak if there
|
||||
// was never a strong reference. If the reference count had been incremented,
|
||||
// it is deallocated directly in decWeak, and hence still lives as long as
|
||||
// the last weak reference.
|
||||
// In the OBJECT_LIFETIME_WEAK case, it is always deallocated from the RefBase
|
||||
// destructor, which is always invoked by decWeak. DecStrong explicitly avoids
|
||||
// the deletion in this case.
|
||||
//
|
||||
// Memory ordering:
|
||||
// The client must ensure that every inc() call, together with all other
|
||||
// accesses to the object, happens before the corresponding dec() call.
|
||||
//
|
||||
// We try to keep memory ordering constraints on atomics as weak as possible,
|
||||
// since memory fences or ordered memory accesses are likely to be a major
|
||||
// performance cost for this code. All accesses to mStrong, mWeak, and mFlags
|
||||
// explicitly relax memory ordering in some way.
|
||||
//
|
||||
// The only operations that are not memory_order_relaxed are reference count
|
||||
// decrements. All reference count decrements are release operations. In
|
||||
// addition, the final decrement leading the deallocation is followed by an
|
||||
// acquire fence, which we can view informally as also turning it into an
|
||||
// acquire operation. (See 29.8p4 [atomics.fences] for details. We could
|
||||
// alternatively use acq_rel operations for all decrements. This is probably
|
||||
// slower on most current (2016) hardware, especially on ARMv7, but that may
|
||||
// not be true indefinitely.)
|
||||
//
|
||||
// This convention ensures that the second-to-last decrement synchronizes with
|
||||
// (in the language of 1.10 in the C++ standard) the final decrement of a
|
||||
// reference count. Since reference counts are only updated using atomic
|
||||
// read-modify-write operations, this also extends to any earlier decrements.
|
||||
// (See "release sequence" in 1.10.)
|
||||
//
|
||||
// Since all operations on an object happen before the corresponding reference
|
||||
// count decrement, and all reference count decrements happen before the final
|
||||
// one, we are guaranteed that all other object accesses happen before the
|
||||
// object is destroyed.
|
||||
|
||||
|
||||
#define INITIAL_STRONG_VALUE (1<<28)
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
@ -64,10 +125,10 @@ namespace android {
|
|||
class RefBase::weakref_impl : public RefBase::weakref_type
|
||||
{
|
||||
public:
|
||||
volatile int32_t mStrong;
|
||||
volatile int32_t mWeak;
|
||||
RefBase* const mBase;
|
||||
volatile int32_t mFlags;
|
||||
std::atomic<int32_t> mStrong;
|
||||
std::atomic<int32_t> mWeak;
|
||||
RefBase* const mBase;
|
||||
std::atomic<int32_t> mFlags;
|
||||
|
||||
#if !DEBUG_REFS
|
||||
|
||||
|
@ -141,7 +202,7 @@ public:
|
|||
void addStrongRef(const void* id) {
|
||||
//ALOGD_IF(mTrackEnabled,
|
||||
// "addStrongRef: RefBase=%p, id=%p", mBase, id);
|
||||
addRef(&mStrongRefs, id, mStrong);
|
||||
addRef(&mStrongRefs, id, mStrong.load(std::memory_order_relaxed));
|
||||
}
|
||||
|
||||
void removeStrongRef(const void* id) {
|
||||
|
@ -150,7 +211,7 @@ public:
|
|||
if (!mRetain) {
|
||||
removeRef(&mStrongRefs, id);
|
||||
} else {
|
||||
addRef(&mStrongRefs, id, -mStrong);
|
||||
addRef(&mStrongRefs, id, -mStrong.load(std::memory_order_relaxed));
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -162,14 +223,14 @@ public:
|
|||
}
|
||||
|
||||
void addWeakRef(const void* id) {
|
||||
addRef(&mWeakRefs, id, mWeak);
|
||||
addRef(&mWeakRefs, id, mWeak.load(std::memory_order_relaxed));
|
||||
}
|
||||
|
||||
void removeWeakRef(const void* id) {
|
||||
if (!mRetain) {
|
||||
removeRef(&mWeakRefs, id);
|
||||
} else {
|
||||
addRef(&mWeakRefs, id, -mWeak);
|
||||
addRef(&mWeakRefs, id, -mWeak.load(std::memory_order_relaxed));
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -330,7 +391,7 @@ void RefBase::incStrong(const void* id) const
|
|||
refs->incWeak(id);
|
||||
|
||||
refs->addStrongRef(id);
|
||||
const int32_t c = android_atomic_inc(&refs->mStrong);
|
||||
const int32_t c = refs->mStrong.fetch_add(1, std::memory_order_relaxed);
|
||||
ALOG_ASSERT(c > 0, "incStrong() called on %p after last strong ref", refs);
|
||||
#if PRINT_REFS
|
||||
ALOGD("incStrong of %p from %p: cnt=%d\n", this, id, c);
|
||||
|
@ -339,7 +400,10 @@ void RefBase::incStrong(const void* id) const
|
|||
return;
|
||||
}
|
||||
|
||||
android_atomic_add(-INITIAL_STRONG_VALUE, &refs->mStrong);
|
||||
int32_t old = refs->mStrong.fetch_sub(INITIAL_STRONG_VALUE,
|
||||
std::memory_order_relaxed);
|
||||
// A decStrong() must still happen after us.
|
||||
ALOG_ASSERT(old > INITIAL_STRONG_VALUE, "0x%x too small", old);
|
||||
refs->mBase->onFirstRef();
|
||||
}
|
||||
|
||||
|
@ -347,27 +411,39 @@ void RefBase::decStrong(const void* id) const
|
|||
{
|
||||
weakref_impl* const refs = mRefs;
|
||||
refs->removeStrongRef(id);
|
||||
const int32_t c = android_atomic_dec(&refs->mStrong);
|
||||
const int32_t c = refs->mStrong.fetch_sub(1, std::memory_order_release);
|
||||
#if PRINT_REFS
|
||||
ALOGD("decStrong of %p from %p: cnt=%d\n", this, id, c);
|
||||
#endif
|
||||
ALOG_ASSERT(c >= 1, "decStrong() called on %p too many times", refs);
|
||||
if (c == 1) {
|
||||
std::atomic_thread_fence(std::memory_order_acquire);
|
||||
refs->mBase->onLastStrongRef(id);
|
||||
if ((refs->mFlags&OBJECT_LIFETIME_MASK) == OBJECT_LIFETIME_STRONG) {
|
||||
int32_t flags = refs->mFlags.load(std::memory_order_relaxed);
|
||||
if ((flags&OBJECT_LIFETIME_MASK) == OBJECT_LIFETIME_STRONG) {
|
||||
delete this;
|
||||
// Since mStrong had been incremented, the destructor did not
|
||||
// delete refs.
|
||||
}
|
||||
}
|
||||
// Note that even with only strong reference operations, the thread
|
||||
// deallocating this may not be the same as the thread deallocating refs.
|
||||
// That's OK: all accesses to this happen before its deletion here,
|
||||
// and all accesses to refs happen before its deletion in the final decWeak.
|
||||
// The destructor can safely access mRefs because either it's deleting
|
||||
// mRefs itself, or it's running entirely before the final mWeak decrement.
|
||||
refs->decWeak(id);
|
||||
}
|
||||
|
||||
void RefBase::forceIncStrong(const void* id) const
|
||||
{
|
||||
// Allows initial mStrong of 0 in addition to INITIAL_STRONG_VALUE.
|
||||
// TODO: Better document assumptions.
|
||||
weakref_impl* const refs = mRefs;
|
||||
refs->incWeak(id);
|
||||
|
||||
refs->addStrongRef(id);
|
||||
const int32_t c = android_atomic_inc(&refs->mStrong);
|
||||
const int32_t c = refs->mStrong.fetch_add(1, std::memory_order_relaxed);
|
||||
ALOG_ASSERT(c >= 0, "forceIncStrong called on %p after ref count underflow",
|
||||
refs);
|
||||
#if PRINT_REFS
|
||||
|
@ -376,7 +452,8 @@ void RefBase::forceIncStrong(const void* id) const
|
|||
|
||||
switch (c) {
|
||||
case INITIAL_STRONG_VALUE:
|
||||
android_atomic_add(-INITIAL_STRONG_VALUE, &refs->mStrong);
|
||||
refs->mStrong.fetch_sub(INITIAL_STRONG_VALUE,
|
||||
std::memory_order_relaxed);
|
||||
// fall through...
|
||||
case 0:
|
||||
refs->mBase->onFirstRef();
|
||||
|
@ -385,7 +462,8 @@ void RefBase::forceIncStrong(const void* id) const
|
|||
|
||||
int32_t RefBase::getStrongCount() const
|
||||
{
|
||||
return mRefs->mStrong;
|
||||
// Debugging only; No memory ordering guarantees.
|
||||
return mRefs->mStrong.load(std::memory_order_relaxed);
|
||||
}
|
||||
|
||||
RefBase* RefBase::weakref_type::refBase() const
|
||||
|
@ -397,7 +475,8 @@ void RefBase::weakref_type::incWeak(const void* id)
|
|||
{
|
||||
weakref_impl* const impl = static_cast<weakref_impl*>(this);
|
||||
impl->addWeakRef(id);
|
||||
const int32_t c __unused = android_atomic_inc(&impl->mWeak);
|
||||
const int32_t c __unused = impl->mWeak.fetch_add(1,
|
||||
std::memory_order_relaxed);
|
||||
ALOG_ASSERT(c >= 0, "incWeak called on %p after last weak ref", this);
|
||||
}
|
||||
|
||||
|
@ -406,16 +485,19 @@ void RefBase::weakref_type::decWeak(const void* id)
|
|||
{
|
||||
weakref_impl* const impl = static_cast<weakref_impl*>(this);
|
||||
impl->removeWeakRef(id);
|
||||
const int32_t c = android_atomic_dec(&impl->mWeak);
|
||||
const int32_t c = impl->mWeak.fetch_sub(1, std::memory_order_release);
|
||||
ALOG_ASSERT(c >= 1, "decWeak called on %p too many times", this);
|
||||
if (c != 1) return;
|
||||
atomic_thread_fence(std::memory_order_acquire);
|
||||
|
||||
if ((impl->mFlags&OBJECT_LIFETIME_WEAK) == OBJECT_LIFETIME_STRONG) {
|
||||
int32_t flags = impl->mFlags.load(std::memory_order_relaxed);
|
||||
if ((flags&OBJECT_LIFETIME_MASK) == OBJECT_LIFETIME_STRONG) {
|
||||
// This is the regular lifetime case. The object is destroyed
|
||||
// when the last strong reference goes away. Since weakref_impl
|
||||
// outlive the object, it is not destroyed in the dtor, and
|
||||
// we'll have to do it here.
|
||||
if (impl->mStrong == INITIAL_STRONG_VALUE) {
|
||||
if (impl->mStrong.load(std::memory_order_relaxed)
|
||||
== INITIAL_STRONG_VALUE) {
|
||||
// Special case: we never had a strong reference, so we need to
|
||||
// destroy the object now.
|
||||
delete impl->mBase;
|
||||
|
@ -424,13 +506,10 @@ void RefBase::weakref_type::decWeak(const void* id)
|
|||
delete impl;
|
||||
}
|
||||
} else {
|
||||
// less common case: lifetime is OBJECT_LIFETIME_{WEAK|FOREVER}
|
||||
// This is the OBJECT_LIFETIME_WEAK case. The last weak-reference
|
||||
// is gone, we can destroy the object.
|
||||
impl->mBase->onLastWeakRef(id);
|
||||
if ((impl->mFlags&OBJECT_LIFETIME_MASK) == OBJECT_LIFETIME_WEAK) {
|
||||
// this is the OBJECT_LIFETIME_WEAK case. The last weak-reference
|
||||
// is gone, we can destroy the object.
|
||||
delete impl->mBase;
|
||||
}
|
||||
delete impl->mBase;
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -439,7 +518,7 @@ bool RefBase::weakref_type::attemptIncStrong(const void* id)
|
|||
incWeak(id);
|
||||
|
||||
weakref_impl* const impl = static_cast<weakref_impl*>(this);
|
||||
int32_t curCount = impl->mStrong;
|
||||
int32_t curCount = impl->mStrong.load(std::memory_order_relaxed);
|
||||
|
||||
ALOG_ASSERT(curCount >= 0,
|
||||
"attemptIncStrong called on %p after underflow", this);
|
||||
|
@ -447,19 +526,20 @@ bool RefBase::weakref_type::attemptIncStrong(const void* id)
|
|||
while (curCount > 0 && curCount != INITIAL_STRONG_VALUE) {
|
||||
// we're in the easy/common case of promoting a weak-reference
|
||||
// from an existing strong reference.
|
||||
if (android_atomic_cmpxchg(curCount, curCount+1, &impl->mStrong) == 0) {
|
||||
if (impl->mStrong.compare_exchange_weak(curCount, curCount+1,
|
||||
std::memory_order_relaxed)) {
|
||||
break;
|
||||
}
|
||||
// the strong count has changed on us, we need to re-assert our
|
||||
// situation.
|
||||
curCount = impl->mStrong;
|
||||
// situation. curCount was updated by compare_exchange_weak.
|
||||
}
|
||||
|
||||
if (curCount <= 0 || curCount == INITIAL_STRONG_VALUE) {
|
||||
// we're now in the harder case of either:
|
||||
// - there never was a strong reference on us
|
||||
// - or, all strong references have been released
|
||||
if ((impl->mFlags&OBJECT_LIFETIME_WEAK) == OBJECT_LIFETIME_STRONG) {
|
||||
int32_t flags = impl->mFlags.load(std::memory_order_relaxed);
|
||||
if ((flags&OBJECT_LIFETIME_MASK) == OBJECT_LIFETIME_STRONG) {
|
||||
// this object has a "normal" life-time, i.e.: it gets destroyed
|
||||
// when the last strong reference goes away
|
||||
if (curCount <= 0) {
|
||||
|
@ -473,13 +553,13 @@ bool RefBase::weakref_type::attemptIncStrong(const void* id)
|
|||
// there never was a strong-reference, so we can try to
|
||||
// promote this object; we need to do that atomically.
|
||||
while (curCount > 0) {
|
||||
if (android_atomic_cmpxchg(curCount, curCount + 1,
|
||||
&impl->mStrong) == 0) {
|
||||
if (impl->mStrong.compare_exchange_weak(curCount, curCount+1,
|
||||
std::memory_order_relaxed)) {
|
||||
break;
|
||||
}
|
||||
// the strong count has changed on us, we need to re-assert our
|
||||
// situation (e.g.: another thread has inc/decStrong'ed us)
|
||||
curCount = impl->mStrong;
|
||||
// curCount has been updated.
|
||||
}
|
||||
|
||||
if (curCount <= 0) {
|
||||
|
@ -499,7 +579,7 @@ bool RefBase::weakref_type::attemptIncStrong(const void* id)
|
|||
}
|
||||
// grab a strong-reference, which is always safe due to the
|
||||
// extended life-time.
|
||||
curCount = android_atomic_inc(&impl->mStrong);
|
||||
curCount = impl->mStrong.fetch_add(1, std::memory_order_relaxed);
|
||||
}
|
||||
|
||||
// If the strong reference count has already been incremented by
|
||||
|
@ -518,21 +598,16 @@ bool RefBase::weakref_type::attemptIncStrong(const void* id)
|
|||
ALOGD("attemptIncStrong of %p from %p: cnt=%d\n", this, id, curCount);
|
||||
#endif
|
||||
|
||||
// now we need to fix-up the count if it was INITIAL_STRONG_VALUE
|
||||
// this must be done safely, i.e.: handle the case where several threads
|
||||
// curCount is the value of mStrong before we increment ed it.
|
||||
// Now we need to fix-up the count if it was INITIAL_STRONG_VALUE.
|
||||
// This must be done safely, i.e.: handle the case where several threads
|
||||
// were here in attemptIncStrong().
|
||||
curCount = impl->mStrong;
|
||||
while (curCount >= INITIAL_STRONG_VALUE) {
|
||||
ALOG_ASSERT(curCount > INITIAL_STRONG_VALUE,
|
||||
"attemptIncStrong in %p underflowed to INITIAL_STRONG_VALUE",
|
||||
this);
|
||||
if (android_atomic_cmpxchg(curCount, curCount-INITIAL_STRONG_VALUE,
|
||||
&impl->mStrong) == 0) {
|
||||
break;
|
||||
}
|
||||
// the strong-count changed on us, we need to re-assert the situation,
|
||||
// for e.g.: it's possible the fix-up happened in another thread.
|
||||
curCount = impl->mStrong;
|
||||
// curCount > INITIAL_STRONG_VALUE is OK, and can happen if we're doing
|
||||
// this in the middle of another incStrong. The subtraction is handled
|
||||
// by the thread that started with INITIAL_STRONG_VALUE.
|
||||
if (curCount == INITIAL_STRONG_VALUE) {
|
||||
impl->mStrong.fetch_sub(INITIAL_STRONG_VALUE,
|
||||
std::memory_order_relaxed);
|
||||
}
|
||||
|
||||
return true;
|
||||
|
@ -542,14 +617,15 @@ bool RefBase::weakref_type::attemptIncWeak(const void* id)
|
|||
{
|
||||
weakref_impl* const impl = static_cast<weakref_impl*>(this);
|
||||
|
||||
int32_t curCount = impl->mWeak;
|
||||
int32_t curCount = impl->mWeak.load(std::memory_order_relaxed);
|
||||
ALOG_ASSERT(curCount >= 0, "attemptIncWeak called on %p after underflow",
|
||||
this);
|
||||
while (curCount > 0) {
|
||||
if (android_atomic_cmpxchg(curCount, curCount+1, &impl->mWeak) == 0) {
|
||||
if (impl->mWeak.compare_exchange_weak(curCount, curCount+1,
|
||||
std::memory_order_relaxed)) {
|
||||
break;
|
||||
}
|
||||
curCount = impl->mWeak;
|
||||
// curCount has been updated.
|
||||
}
|
||||
|
||||
if (curCount > 0) {
|
||||
|
@ -561,7 +637,9 @@ bool RefBase::weakref_type::attemptIncWeak(const void* id)
|
|||
|
||||
int32_t RefBase::weakref_type::getWeakCount() const
|
||||
{
|
||||
return static_cast<const weakref_impl*>(this)->mWeak;
|
||||
// Debug only!
|
||||
return static_cast<const weakref_impl*>(this)->mWeak
|
||||
.load(std::memory_order_relaxed);
|
||||
}
|
||||
|
||||
void RefBase::weakref_type::printRefs() const
|
||||
|
@ -592,17 +670,19 @@ RefBase::RefBase()
|
|||
|
||||
RefBase::~RefBase()
|
||||
{
|
||||
if (mRefs->mStrong == INITIAL_STRONG_VALUE) {
|
||||
if (mRefs->mStrong.load(std::memory_order_relaxed)
|
||||
== INITIAL_STRONG_VALUE) {
|
||||
// we never acquired a strong (and/or weak) reference on this object.
|
||||
delete mRefs;
|
||||
} else {
|
||||
// life-time of this object is extended to WEAK or FOREVER, in
|
||||
// life-time of this object is extended to WEAK, in
|
||||
// which case weakref_impl doesn't out-live the object and we
|
||||
// can free it now.
|
||||
if ((mRefs->mFlags & OBJECT_LIFETIME_MASK) != OBJECT_LIFETIME_STRONG) {
|
||||
int32_t flags = mRefs->mFlags.load(std::memory_order_relaxed);
|
||||
if ((flags & OBJECT_LIFETIME_MASK) != OBJECT_LIFETIME_STRONG) {
|
||||
// It's possible that the weak count is not 0 if the object
|
||||
// re-acquired a weak reference in its destructor
|
||||
if (mRefs->mWeak == 0) {
|
||||
if (mRefs->mWeak.load(std::memory_order_relaxed) == 0) {
|
||||
delete mRefs;
|
||||
}
|
||||
}
|
||||
|
@ -613,7 +693,9 @@ RefBase::~RefBase()
|
|||
|
||||
void RefBase::extendObjectLifetime(int32_t mode)
|
||||
{
|
||||
android_atomic_or(mode, &mRefs->mFlags);
|
||||
// Must be happens-before ordered with respect to construction or any
|
||||
// operation that could destroy the object.
|
||||
mRefs->mFlags.fetch_or(mode, std::memory_order_relaxed);
|
||||
}
|
||||
|
||||
void RefBase::onFirstRef()
|
||||
|
|
Loading…
Reference in New Issue