Strong retain calls should always return the instance pointer.

This fixes a regression in 73132a6 (#200) where nil was returned
from a retain call after the object begins deallocating. Normal
retains of a deallocating object are still expected to return an
instance pointer inside its dealloc method and code compiled with ARC
will generate calls to objc_storeStrong() when a block captures the
self pointer inside the dealloc method.
main
Graham--M 5 years ago committed by David Chisnall
parent 38f44a8752
commit 921e3c3386

@ -2,6 +2,20 @@
void direct_saturation_test(); void direct_saturation_test();
@interface TestWithDelloc : Test
@end
@implementation TestWithDelloc
- (void)dealloc
{
id obj = nil;
objc_storeStrong(&obj, self);
assert(obj == self);
assert(object_getRetainCount_np(obj) == 0);
[super dealloc];
}
@end
int main() int main()
{ {
id obj = [Test new]; id obj = [Test new];
@ -27,7 +41,7 @@ int main()
// Final release should prevent further retains and releases. // Final release should prevent further retains and releases.
assert(objc_release_fast_no_destroy_np(obj) == YES); assert(objc_release_fast_no_destroy_np(obj) == YES);
assert(object_getRetainCount_np(obj) == 0); assert(object_getRetainCount_np(obj) == 0);
assert(objc_retain_fast_np(obj) == nil); assert(objc_retain_fast_np(obj) == obj);
assert(object_getRetainCount_np(obj) == 0); assert(object_getRetainCount_np(obj) == 0);
assert(objc_release_fast_no_destroy_np(obj) == NO); assert(objc_release_fast_no_destroy_np(obj) == NO);
assert(object_getRetainCount_np(obj) == 0); assert(object_getRetainCount_np(obj) == 0);
@ -54,7 +68,11 @@ int main()
assert(objc_delete_weak_refs(obj) == YES); assert(objc_delete_weak_refs(obj) == YES);
} }
obj = object_dispose(obj); object_dispose(obj);
// Check we can use strong references inside a dealloc method.
obj = [TestWithDelloc new];
[obj release];
obj = nil;
direct_saturation_test(); direct_saturation_test();
return 0; return 0;

@ -244,7 +244,7 @@ extern "C" OBJC_PUBLIC size_t object_getRetainCount_np(id obj)
return realCount == refcount_mask ? 0 : realCount + 1; return realCount == refcount_mask ? 0 : realCount + 1;
} }
extern "C" OBJC_PUBLIC id objc_retain_fast_np(id obj) static id retain_fast(id obj, BOOL isWeak)
{ {
uintptr_t *refCount = ((uintptr_t*)obj) - 1; uintptr_t *refCount = ((uintptr_t*)obj) - 1;
uintptr_t refCountVal = __sync_fetch_and_add(refCount, 0); uintptr_t refCountVal = __sync_fetch_and_add(refCount, 0);
@ -268,7 +268,7 @@ extern "C" OBJC_PUBLIC id objc_retain_fast_np(id obj)
// this and we don't zero the references or deallocate. // this and we don't zero the references or deallocate.
if (realCount == refcount_mask) if (realCount == refcount_mask)
{ {
return nil; return isWeak ? nil : obj;
} }
// If the reference count is saturated, don't increment it. // If the reference count is saturated, don't increment it.
if (realCount == refcount_max) if (realCount == refcount_max)
@ -283,6 +283,11 @@ extern "C" OBJC_PUBLIC id objc_retain_fast_np(id obj)
return obj; return obj;
} }
extern "C" OBJC_PUBLIC id objc_retain_fast_np(id obj)
{
return retain_fast(obj, NO);
}
__attribute__((always_inline)) __attribute__((always_inline))
static inline BOOL isPersistentObject(id obj) static inline BOOL isPersistentObject(id obj)
{ {
@ -301,7 +306,7 @@ static inline BOOL isPersistentObject(id obj)
return objc_test_class_flag(obj->isa, objc_class_flag_permanent_instances); return objc_test_class_flag(obj->isa, objc_class_flag_permanent_instances);
} }
static inline id retain(id obj) static inline id retain(id obj, BOOL isWeak)
{ {
if (isPersistentObject(obj)) { return obj; } if (isPersistentObject(obj)) { return obj; }
Class cls = obj->isa; Class cls = obj->isa;
@ -312,7 +317,7 @@ static inline id retain(id obj)
} }
if (objc_test_class_flag(cls, objc_class_flag_fast_arc)) if (objc_test_class_flag(cls, objc_class_flag_fast_arc))
{ {
return objc_retain_fast_np(obj); return retain_fast(obj, isWeak);
} }
return [obj retain]; return [obj retain];
} }
@ -593,7 +598,7 @@ extern "C" OBJC_PUBLIC id objc_retainAutoreleasedReturnValue(id obj)
extern "C" OBJC_PUBLIC id objc_retain(id obj) extern "C" OBJC_PUBLIC id objc_retain(id obj)
{ {
if (nil == obj) { return nil; } if (nil == obj) { return nil; }
return retain(obj); return retain(obj, NO);
} }
extern "C" OBJC_PUBLIC id objc_retainAutorelease(id obj) extern "C" OBJC_PUBLIC id objc_retainAutorelease(id obj)
@ -604,7 +609,7 @@ extern "C" OBJC_PUBLIC id objc_retainAutorelease(id obj)
extern "C" OBJC_PUBLIC id objc_retainAutoreleaseReturnValue(id obj) extern "C" OBJC_PUBLIC id objc_retainAutoreleaseReturnValue(id obj)
{ {
if (nil == obj) { return obj; } if (nil == obj) { return obj; }
return objc_autoreleaseReturnValue(retain(obj)); return objc_autoreleaseReturnValue(retain(obj, NO));
} }
@ -912,7 +917,9 @@ extern "C" OBJC_PUBLIC id objc_loadWeakRetained(id* addr)
{ {
obj = _objc_weak_load(obj); obj = _objc_weak_load(obj);
} }
return objc_retain(obj); // block_load_weak() or _objc_weak_load() can return nil
if (obj == nil) { return nil; }
return retain(obj, YES);
} }
extern "C" OBJC_PUBLIC id objc_loadWeak(id* object) extern "C" OBJC_PUBLIC id objc_loadWeak(id* object)

Loading…
Cancel
Save