Skip to content

Commit

Permalink
Avoid dangling weak references to deallocating objects.
Browse files Browse the repository at this point in the history
The previous checks for a deallocating object with negative reference count did not work because:
    1. the sign bit has to be recreated as was happening in objc_delete_weak_refs()
    2. there was no distinction between a saturated count and a negative reference count

refcount_max now indicates when the refcount has saturated and should no longer be mutated.
An underflow to -1 still maps to refcount_mask, allowing us to detect when an object is supposed to be deallocated.
Neither objc_release_fast_no_destroy_np() nor objc_retain_fast_np() mutate the refcount when it is one of those values, so the comment in objc_delete_weak_refs() was adjusted.
  • Loading branch information
Graham--M committed Mar 14, 2021
1 parent c399119 commit cc0d42a
Showing 1 changed file with 13 additions and 17 deletions.
30 changes: 13 additions & 17 deletions arc.mm
Original file line number Diff line number Diff line change
Expand Up @@ -238,12 +238,14 @@ static TLS_CALLBACK(cleanupPools)(struct arc_tls* tls)
* All of the bits other than the top bit are the real reference count.
*/
static const size_t refcount_mask = ~weak_mask;
static const size_t refcount_max = refcount_mask - 1;

extern "C" OBJC_PUBLIC size_t object_getRetainCount_np(id obj)
{
uintptr_t *refCount = ((uintptr_t*)obj) - 1;
uintptr_t refCountVal = __sync_fetch_and_add(refCount, 0);
return (((size_t)refCountVal) & refcount_mask) + 1;
size_t realCount = refCountVal & refcount_mask;
return realCount == refcount_mask ? 0 : realCount + 1;
}

extern "C" OBJC_PUBLIC id objc_retain_fast_np(id obj)
Expand All @@ -268,12 +270,12 @@ static TLS_CALLBACK(cleanupPools)(struct arc_tls* tls)
// If the serialisation happens the other way, then the locked
// check of the reference count will happen after we've referenced
// this and we don't zero the references or deallocate.
if (realCount < 0)
if (realCount == refcount_mask)
{
return nil;
}
// If the reference count is saturated, don't increment it.
if (realCount == refcount_mask)
if (realCount == refcount_max)
{
return obj;
}
Expand Down Expand Up @@ -329,8 +331,8 @@ static inline id retain(id obj)
do {
refCountVal = newVal;
size_t realCount = refCountVal & refcount_mask;
// If the reference count is saturated, don't decrement it.
if (realCount == refcount_mask)
// If the reference count is saturated or deallocating, don't decrement it.
if (realCount >= refcount_max)
{
return NO;
}
Expand All @@ -341,8 +343,7 @@ static inline id retain(id obj)
uintptr_t updated = (uintptr_t)realCount;
newVal = __sync_val_compare_and_swap(refCount, refCountVal, updated);
} while (newVal != refCountVal);
// We allow refcounts to run into the negative, but should only
// deallocate once.

if (shouldFree)
{
if (isWeak)
Expand Down Expand Up @@ -761,7 +762,7 @@ static BOOL setObjectHasWeakRefs(id obj)
size_t realCount = refCountVal & refcount_mask;
// If this object has already been deallocated (or is in the
// process of being deallocated) then don't bother storing it.
if (realCount < 0)
if (realCount == refcount_mask)
{
obj = nil;
cls = Nil;
Expand Down Expand Up @@ -848,16 +849,11 @@ static BOOL setObjectHasWeakRefs(id obj)
LOCK_FOR_SCOPE(&weakRefLock);
if (objc_test_class_flag(classForObject(obj), objc_class_flag_fast_arc))
{
// If another thread has done a load of a weak reference, then it will
// have incremented the reference count with the lock held. It may
// have done so in between this thread's decrementing the reference
// count and its acquiring the lock. In this case, report failure.
// Don't proceed if the object isn't deallocating.
uintptr_t *refCount = ((uintptr_t*)obj) - 1;
// Reconstruct the sign bit. We don't need to do this on any other
// operations, because even on increment the overflow will be correct
// after truncation.
uintptr_t refCountVal = (__sync_fetch_and_add(refCount, 0) & refcount_mask) << refcount_shift;
if ((((intptr_t)refCountVal) >> refcount_shift) >= 0)
uintptr_t refCountVal = __sync_fetch_and_add(refCount, 0);
size_t realCount = refCountVal & refcount_mask;
if (realCount != refcount_mask)
{
return NO;
}
Expand Down

0 comments on commit cc0d42a

Please sign in to comment.