Self deallocs after cancelPreviousPerformRequestsWithTarget

Question

Using ARC and iOS 6.1, I have a simple class here to demonstrate my problem:

#import <GHUnitIOS/GHUnit.h>

@interface MyClass : NSObject
@property BOOL cancel;
@property BOOL dead;
-(void)doSomething;
-(void)reset;
-(void)logMe;
@end

@implementation MyClass

-(id)init {
    self = [super init];
    if(self) {
        [[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(reset) name:@"dude" object:nil];
        NSLog(@"I'm alive");
    }
    return self;
}

-(void)dealloc {
    _dead = YES;
    [[NSNotificationCenter defaultCenter] removeObserver:self];
    [MyClass cancelPreviousPerformRequestsWithTarget:self];
    NSLog(@"I'm dead");
}

-(void)doSomething {
    NSLog(@"dude:%d", _dead);
    if(!_cancel) {
        [self performSelector:@selector(doSomething) withObject:nil afterDelay:0.2];
        NSLog(@"scheduled");
    }
    [self logMe];
}

-(void)reset {
    NSLog(@"reset");
    [MyClass cancelPreviousPerformRequestsWithTarget:self];
    _cancel = YES;
    [self doSomething];
}

-(void)logMe {
    NSLog(@"logme");
}
@end

@interface ATest : GHTestCase
@end

@implementation ATest

-(BOOL)shouldRunOnMainThread {return YES;}
-(void)setUpClass {}
-(void)tearDownClass {}
-(void)setUp {}
-(void)tearDown {}

-(void)testBlah {
    MyClass* blah = [[MyClass alloc] init];
    [blah doSomething];
    dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(1.0 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^(void){
        [[NSNotificationCenter defaultCenter] postNotificationName:@"dude" object:nil];
    });
    blah = nil;
}

@end

In the test, MyClass gets instantiated and I kick off doSomething, which performs some work (i.e., logging) and then calls itself after 0.25s if _cancel is false. Meanwhile, I schedule a notification to fire (which eventually sets _cancel to true) after 1.0s. Then I nil out blah.

So my expectation is the timer that gets created by the performSelector:withObject:withDelay is owning a reference to MyClass.

However, when I run this test with zombies enabled, I get this output:

2013-02-28 15:30:55.518 Tests[11946:c07] ATest/testBlah
2013-02-28 15:30:56.789 Tests[11946:c07] Re-running: ATest/testBlah
2013-02-28 15:30:56.790 Tests[11946:c07] I'm alive
2013-02-28 15:30:56.790 Tests[11946:c07] dude:0
2013-02-28 15:30:56.791 Tests[11946:c07] scheduled
2013-02-28 15:30:56.791 Tests[11946:c07] logme
2013-02-28 15:30:56.792 Tests[11946:c07] ATest/testBlah ✔ 0.00s
2013-02-28 15:30:56.991 Tests[11946:c07] dude:0
2013-02-28 15:30:56.992 Tests[11946:c07] scheduled
2013-02-28 15:30:56.992 Tests[11946:c07] logme
2013-02-28 15:30:57.193 Tests[11946:c07] dude:0
2013-02-28 15:30:57.194 Tests[11946:c07] scheduled
2013-02-28 15:30:57.194 Tests[11946:c07] logme
2013-02-28 15:30:57.395 Tests[11946:c07] dude:0
2013-02-28 15:30:57.395 Tests[11946:c07] scheduled
2013-02-28 15:30:57.396 Tests[11946:c07] logme
2013-02-28 15:30:57.596 Tests[11946:c07] dude:0
2013-02-28 15:30:57.597 Tests[11946:c07] scheduled
2013-02-28 15:30:57.597 Tests[11946:c07] logme
2013-02-28 15:30:57.792 Tests[11946:c07] reset
2013-02-28 15:30:57.793 Tests[11946:c07] I'm dead
2013-02-28 15:30:57.793 Tests[11946:c07] * -[MyClass doSomething]: message sent to deallocated instance 0xb584880

Why is self being deallocated after I call cancelPreviousPerformRequestsWithTarget: in the reset method?

Is this issue an ARC problem or a coding error?

Jesse Rusak · Accepted Answer · 2013-02-28T22:56:01.667

Neat question. I would call this a bug in NSNotificationCenter. Here's a simplified version of your code with the same behaviour. All we're doing is setting ourselves up to listen to a notification, and keeping ourselves alive with a single strong (static) reference. When the notification goes off, we clear that reference. (In your case, the last strong reference to your object was in the performSelector: machinery; the target of a performSelector: is retained, and when you canceled it, it released its reference to you.)

@interface MyClass : NSObject
@end

static MyClass *instance;

@implementation MyClass

-(id)init {
    self = [super init];
    if(self) {
        [[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(clearReference) name:@"dude" object:nil];
        NSLog(@"I'm alive");
        instance = self;
    }
    return self;
}

- (void)clearReference {
    instance = nil;
    [self logMe];
}

-(void)dealloc {
    [[NSNotificationCenter defaultCenter] removeObserver:self];
    NSLog(@"I'm dead");
}

-(void)logMe {
    NSLog(@"logme");
}

@end

// Test case
[[MyClass alloc] init];
[[NSNotificationCenter defaultCenter] postNotificationName:@"dude" object:nil];

This causes a zombie-message at [self logMe]. The reason is that in clearReference, when we do instance = nil; that is the last strong reference to us, so we are deallocated before we call [self logMe];. But why, you might ask, isn't ARC holding on to us?

Well, ARC never retains self because it's usually safe to assume the caller of the method has a strong reference to self, and if every method had to retain/release self, it would add up to a lot of overhead. (For code compiled under ARC, this assumption is practically always true, since to call a method on an object you first need a reference to it.) Unfortunately, NSNotificationCenter isn't retaining your object before calling your method. I would call this a bug: in non-ARC code, it's usually polite to ensure you have at least a temporary strong reference to an object before calling some unknown callback on it:

id objectToCall = ...;
[objectToCall retain];
[objectToCall performSelector:...]; // the actual callback
[objectToCall release];

Code like this would ensure the crash you're seeing doesn't happen. NSNotificationCenter, clearly, is not doing this. You could verify this by looking at the retain history of your object in the Zombies instrument.

Since you can't change NSNotificationCenter, one admittedly-ugly workaround I've used before when you could be deallocated and your caller might not be holding a strong reference to you is something like this:

- (void)clearReference {
    CFRetain((__bridge CFTypeRef)(self));
    instance = nil;
    [self logMe];
    CFRelease((__bridge CFTypeRef)(self));
}

That way, at least, you are sure that you won't be deallocated until the end of your method.

I disagree that this is a problem with NSNotificationCenter. And doing `[objectToCall retain]; [objectToCall performSelector:...]; [objectToCall release];` is bad style and makes no sense. Something should be retained only if we need to keep it around to be used. In `[objectToCall performSelector:...];`, the caller gives `objectToCall` to the message passing mechanism, and never uses it again, so this function does not have to retain it. Instead, it's the `clearReference` method that is using `self` after causing it to be deallocated. That is the function that needs to retain it. — newacct, Mar 01 '13 at 00:06
When manually retaining or releasing, how does one accomplish this via ARC? Are the core foundation classes used above allowed in ARC? — Tim Reddy, Mar 01 '13 at 00:56
@TReddy Yes, you can call CFRetain/CFRelease in ARC. That's why they're not [self retain] and [self release] which are forbidden. Again, it is a bit ugly. — Jesse Rusak, Mar 01 '13 at 00:57
@newacct Since `objectToCall` is a non-strong reference, if NSNotificationCenter was compiled with ARC, it would do exactly the same retain/performSelector:/release dance I suggest. (Since the compiler would store the object in a temporary strong reference before the performSelector:, which would make ARC insert a retain & release.) `clearReference` can't be the source of the problem because it has no way of knowing that `instance` is the last reference to self. — Jesse Rusak, Mar 01 '13 at 00:59
Your comments around NSNotificaitonCenter helped me find this link which is pretty much identical to my situation: http://stackoverflow.com/questions/13744970/nsnotificationcenter-and-safe-multithreading — Tim Reddy, Mar 01 '13 at 01:57
@TReddy Yes, that's a good point. In your case, it's probably even better just to use the blocks-based notification API to avoid these issues altogether. — Jesse Rusak, Mar 01 '13 at 02:13
Yes, `clearReference` has no way of knowing that `instance` is the last reference to `self`. Just like if you remove an item from a collection, you have no way of knowing that it might be the same object as another unrelated local variable that you got earlier, and you may have removed its last reference. In both cases, you must be conservative and retain any variables that might be affected. That's the reason why ARC retains local variables and parameters by default -- any call could potentially cause them to be deallocated. But ARC has a special rule that does not retain the parameter `self` — newacct, Mar 01 '13 at 04:09
So, in short, you're saying it's the *callee's* responsibility to keep `self` alive, and I'm saying it's the *caller's* responsibility. I don't think it's documented either way, so we might just have to disagree. :) — Jesse Rusak, Mar 01 '13 at 13:05

OpenThread · Answer 2 · 2016-01-07T04:07:53.367

__weak typeof (self) (weakSelf) = self;Rather than the retain-release dance:

CFRetain((__bridge CFTypeRef)(self));
[NSObject cancelPreviousPerformRequestsWithTarget:self selector:@selector(foo) object:nil];
[self bar];
CFRelease((__bridge CFTypeRef)(self));

I prefer the ARC way like this:

__weak typeof (self) (weakSelf) = self;
[NSObject cancelPreviousPerformRequestsWithTarget:self selector:@selector(foo) object:nil];
[weakSelf bar];

If line 2 make self dealloced, then weakSelf of line 3 will be nil instead of a zombie pointer, and [nil bar] is safe.

And there are 2 advantages in the weakSelf solution:

1. In the retain-release dance solution, [self bar] cost CPU time, and doesn't make any sense.
2. The weak point looks beautiful than convert an object into CFType and manually call retain and release.

p.s.
Like any other xxxxx dance, I'd like to name the weakSelf solution as 'philosophy dance' : )

Self deallocs after cancelPreviousPerformRequestsWithTarget

2 Answers2