Repeating NSTimer, weak reference, owning reference or iVar?

Question

I thought I would put this out here as a separate question from my previous retaining-repeating-nstimer-for-later-access as the discussion has moved forward making a new question clearer than yet another EDIT:

The scenario is an object creates a repeating NSTimer, lets say in viewDidLoad, once created the NSTimer needs to stay around so it can be accessed by other methods.

NSTimer *ti = [NSTimer scheduledTimerWithTimeInterval:1 
                                               target:self 
                                             selector:@selector(updateDisplay:) 
                                             userInfo:nil 
                                              repeats:YES];

I understand that when created the runloop takes ownership of the NSTimer and ultimately stops, removes and releases the NSTimer when [ti invalidate]; is called.

By virtue of the fact that we need to access the NSTimer in more than one method we need some way to hold a reference for future use, the revised question is:

// (1) Should the NSTimer be held using an owning reference (i.e.)
@property(nonatomic, retain) NSTimer *walkTimer;
[self setWalkTimer: ti];
...
...
// Cancel method
[[self walkTimer] invalidate;
[self setWalkTimer:nil];
...
...
// dealloc method
[walkTimer release];
[super dealloc];

.

// (2) Should the NSTimer be held using a weak reference (i.e.)
@property(nonatomic, assign) NSTimer *walkTimer;
[self setWalkTimer: ti];
...
...
// Cancel method
[[self walkTimer] invalidate];
[self setWalkTimer:nil];
...
...
// dealloc method
[super dealloc];

.

// (3) Use an iVar and rely on the runLoop holding (i.e. retaining) the timer
NSTimer *walkTimer;
NSTimer *walkTimer = [NSTimer scheduledTimerWithTimeInterval:1 
                                                      target:self 
                                                    selector:@selector(updateDisplay:) 
                                                    userInfo:nil 
                                                     repeats:YES];
...
...
// Cancel method
[walkTimer invalidate];
walkTimer = nil;

.

// (4) Something not listed above ...

I am happy for just (1) (2) (3) or (4) as a lot of discussion regarding which is best has already been written on the Other thread. There does seem to be a lot of conflicting answers so I hope this more specific question will help focus on what might be best practice in this situation.

---

EDIT:

As a side note in the Apple NSTimer Class Reference 4 out of 5 of the sample code projects use NSTimers that are assigned** to a retained property. Here is an example of what the class reference examples show:

@property (nonatomic, retain) NSTimer *updateTimer;
updateTimer = [NSTimer scheduledTimerWithTimeInterval:.01 target:self selector:@selector(updateCurrentTime) userInfo:p repeats:YES];
...
...
// Cancel
[updateTimer invalidate];
updateTimer = nil;
...
...
// Dealloc method
[super dealloc];
[updateTimer release];

** It should be noted that in the examples Apple are assigning the iVar directly and not using the property setter.

Answer 1

After giving it all some more thought and finding an important flaw in my reasoning, I've come to a different conclusion:

It doesn't matter much, whether you hold an owning or a non-owning reference to a timer that you need to invalidate. It is completely a matter of taste.

The deal breaker is, what the target of the timer is:

If the object that creates a timer is its target, managing that object's lifetime becomes more fragile: it cannot simply be retain/release managed, instead you need to ensure that the client that holds the last reference to this object makes it invalidate the timer before it disposes of it.

Let me illustrate the situation with a couple of sort-of-object-graphs:

1. You start in a state from which you setup the timer and set yourself as the target. Setup of the Timer: yourObject is owned by someClientObject. In parallel exists the current run-loop with an array of scheduledTimers. the setupTimer method is called upon yourObject:

   

2. The result is the following initial state. In addition to the former state yourObject now has a reference (owned or not) to the workTimer, which in turn owns yourObject. Furthermore, workTimer is owned by the run-loops scheduledTimers array:

   

3. So now you'll use the object, but when you're done with it and simply release it, you'll end up with simple release leak: after someClientObject disposes of yourObject through a simple release, yourObject is disassociated from the object-graph but kept alive by workTimer. workTimer and yourObject are leaked!

Where you leak the object (and the timer) because the runloop keeps the timer alive, which — in turn — keeps an owning reference to your object.

This can be avoided if yourObject is only ever owned by one single instance at a time, when it is properly disposed of proper disposal through cancellation: before disposing of yourObject through release, someClientObject calls the cancelTimer method on yourObject. Within that method, yourObject invalidates workTimer and (if it owned workTimer) disposes of workTimer through release:

But now, how do you resolve the following situation?
Multiple Owners: Setup like in the initial state, but now with multiple independent clientObjects that hold references to yourObject

There is no easy answer, I am aware of! (Not that the latter has to say much, but...)

So my advice is...

1. Don't make your timer a property/don't provide accessors for it! Instead, keep it private (with the modern runtime I think you could go so far as to define the ivar in a class extension) and only deal with it from one single object. (You may retain it, if you feel more comfortable doing so, but there is absolutely no need for it.)

   • Caveat: If you absolutely need to access the timer from another object, make the property retain the timer (as that is the only way to avoid crashes caused by clients that directly invalidated the timer they accessed) and provide your own setter. Rescheduling a timer is — in my opinion — not a good reason to break encapsulation here: provide a mutator if you need to do that.

2. Set the timer up with a target other than self. (There are plenty of ways doing so. Maybe through writing a generic TimerTarget class or — if you can use it — through a MAZeroingWeakReference?)

I apologize for being a moron in the first discussion and want to thank Daniel Dickison and Rob Napier for their patience.

So here is the way I am going to handle timers from now on:

// NSTimer+D12WeakTimerTarget.h:
#import <Foundation/NSTimer.h>
@interface NSTimer (D12WeakTimerTarget)
+(NSTimer *)D12scheduledTimerWithTimeInterval:(NSTimeInterval)ti weakTarget:(id)target selector:(SEL)selector userInfo:(id)userInfo repeats:(BOOL)shouldRepeat logsDeallocation:(BOOL)shouldLogDealloc;
@end

// NSTimer+D12WeakTimerTarget.m:
#import "NSTimer+D12WeakTimerTarget.h"
@interface D12WeakTimerTarget : NSObject {
    __weak id weakTarget;
    SEL selector;
    // for logging purposes:
    BOOL logging;
    NSString *targetDescription;
}
-(id)initWithTarget:(id)target selector:(SEL)aSelector shouldLog:(BOOL)shouldLogDealloc;
-(void)passthroughFiredTimer:(NSTimer *)aTimer;
-(void)dumbCallbackTimer:(NSTimer *)aTimer;
@end

@implementation D12WeakTimerTarget
-(id)initWithTarget:(id)target selector:(SEL)aSelector shouldLog:(BOOL)shouldLogDealloc
{
    self = [super init];
    if ( !self )
        return nil;

    logging = shouldLogDealloc;

    if (logging)
        targetDescription = [[target description] copy];

    weakTarget = target;
    selector = aSelector;

    return self;
}

-(void)dealloc
{
    if (logging)
        NSLog(@"-[%@ dealloc]! (Target was %@)", self, targetDescription);

    [targetDescription release];
    [super dealloc];
}

-(void)passthroughFiredTimer:(NSTimer *)aTimer;
{
    [weakTarget performSelector:selector withObject:aTimer];
}

-(void)dumbCallbackTimer:(NSTimer *)aTimer;
{
    [weakTarget performSelector:selector];
}
@end

@implementation NSTimer (D12WeakTimerTarget)
+(NSTimer *)D12scheduledTimerWithTimeInterval:(NSTimeInterval)ti weakTarget:(id)target selector:(SEL)selector userInfo:(id)userInfo repeats:(BOOL)shouldRepeat logsDeallocation:(BOOL)shouldLogDealloc
{
    SEL actualSelector = @selector(dumbCallbackTimer:);
    if ( 2 != [[target methodSignatureForSelector:aSelector] numberOfArguments] )
        actualSelector = @selector(passthroughFiredTimer:);

    D12WeakTimerTarget *indirector = [[D12WeakTimerTarget alloc] initWithTarget:target selector:selector shouldLog:shouldLogDealloc];

    NSTimer *theTimer = [NSTimer scheduledTimerWithTimeInterval:ti target:indirector selector:actualSelector userInfo:userInfo repeats:shouldRepeat];
    [indirector release];

    return theTimer;
}
@end

---

Original (for full disclosure):

You know my opinion from your other post:

There is little reason for an owning reference of a scheduled timer (and bbum seems to agree).

That said, your options 2, and 3 are essentially the same. (There is additional messaging involved in [self setWalkTimer:nil] over walkTimer = nil but I'm not sure if the compiler won't optimize that away and access the ivar directly, but well...)

Answer 2

I generally manage the invalidate inside of the accessor so that you never get surprised by a timer accessing you after you think you got rid of it:

@property(nonatomic, retain) NSTimer *walkTimer;
[self setWalkTimer: ti];

- (void)setWalkTimer:(NSTimer *)aTimer
{
    if (aTimer != walkTimer_)
    {
        [aTimer retain];
        [walkTimer invalidate];
        [walkTimer release];
        walkTimer = aTimer;
    }
}
...
...
// Cancel method
[self setWalkTimer:nil];
...
...
// Make a new timer, automatically invalidating the old one
[self setWalkTimer:[... a new timer ...]]
...
...
// dealloc method
[walkTimer_ invalidate];
[walkTimer_ release];
[super dealloc];