Fix race bug in attemptIncStrong

The compensating onLastStrongRef call could be made even when there
was no onIncStrongAttempted call to compensate for.  This
happened in the OBJECT_LIFETIME_STRONG case when e.g. curCount
was initially zero, but was concurrently incremented by another
thread.

I believe the old code was also incorrect in the
curCount = INITIAL_STRONG_VALUE + 1 case,
which seems to be possible under unlikely conditions.
In that case, I believe the compensating call IS needed.
Thus the condition was also changed.

Bug: 30503444
Change-Id: I44bcbcbb1264e4b52b6d3750dc39b041c4140381
This commit is contained in:
Hans Boehm 2016-07-29 14:39:10 -07:00
parent 02ccdc5db9
commit 7f27cbc3f4
1 changed files with 9 additions and 10 deletions

View File

@ -580,15 +580,14 @@ bool RefBase::weakref_type::attemptIncStrong(const void* id)
// grab a strong-reference, which is always safe due to the
// extended life-time.
curCount = impl->mStrong.fetch_add(1, std::memory_order_relaxed);
}
// If the strong reference count has already been incremented by
// someone else, the implementor of onIncStrongAttempted() is holding
// an unneeded reference. So call onLastStrongRef() here to remove it.
// (No, this is not pretty.) Note that we MUST NOT do this if we
// are in fact acquiring the first reference.
if (curCount > 0 && curCount < INITIAL_STRONG_VALUE) {
impl->mBase->onLastStrongRef(id);
// If the strong reference count has already been incremented by
// someone else, the implementor of onIncStrongAttempted() is holding
// an unneeded reference. So call onLastStrongRef() here to remove it.
// (No, this is not pretty.) Note that we MUST NOT do this if we
// are in fact acquiring the first reference.
if (curCount != 0 && curCount != INITIAL_STRONG_VALUE) {
impl->mBase->onLastStrongRef(id);
}
}
}
@ -598,7 +597,7 @@ bool RefBase::weakref_type::attemptIncStrong(const void* id)
ALOGD("attemptIncStrong of %p from %p: cnt=%d\n", this, id, curCount);
#endif
// curCount is the value of mStrong before we increment ed it.
// curCount is the value of mStrong before we incremented 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().