CODEWITHSUNDEEP
iphoneobjective-ccocoa-touch
fuzzygoat
fuzzygoat

Reputation: 26223

Retaining repeating NSTimer for later access?

I am creating an NSTimer in the createTimer method that I want to refer back to in the later cancelTimer method. To facilitate this I am taking ownership of the NSTimer via a retained property so that I can refer back to it later. The issue that is confusing me is, if I start the timer, cancel it and start it again the code crashes.

@property(nonatomic, retain) NSTimer *walkTimer;

.

-(void)createTimer {
    NSTimer *tempTimer = [NSTimer scheduledTimerWithTimeInterval:1 target:self selector:@selector(updateTimerDisplay) userInfo:nil repeats:YES];
    [self setWalkTimer:tempTimer];
}

-(void)cancelTimer {
    [walkTimer release];
    [[self walkTimer] invalidate];
}

Now I seem to have fixed this by changing cancelTimer to:

-(void)cancelTimer {
    [self setWalkTimer:nil];
    [[self walkTimer] invalidate];
}

I am just curious why release was not working, my understanding was that:

  1. NSTimer (Autorelease object, not owned by me)
  2. setWalkTimer (takes ownership for me, retainCount+1)
  3. release (relinquishes my ownership, retainCount-1)
  4. invalidate (lets system dispose of timer)

EDIT:

// this fails ...
-(void)cancelTimer {
    [[self walkTimer] invalidate];
    [walkTimer release];
}

// this works fine ...
-(void)cancelTimer {
    [[self walkTimer] invalidate];
    [self setWalkTimer: nil];
}

EDIT: 002

Initially I think I was mixing up 

@property(nonatomic, retain) NSTimer *walkTimer;
// &
[self setWalkTimer];

and thinking that I needed a release to balance the property, I don't I either overwrite it with a new set (either to another object or nil) and finally release the property in dealloc.

Is the property(retain) the same as retain, I would say no, which is where I was going wrong I think.

EDIT: 003 With regards to this question I think I personally confused things by wrongly using [walkTimer release] As a result the topic drifted to essentially a new question which I have written up as this

Upvotes: 0

Views: 1507

Answers (3)

danyowdee
danyowdee

Reputation: 4698

Don't retain a scheduled NSTimer if you've set its target to self Don't set self as the target of a repeating timer unless you are absolutely sure to know all the consequences

(...otherwise the runtime drowns a kitten in leaked timers, targets and userInfos — or so goes the saying.)

Please read and re-read "Overview" in the NSTimer Class Reference and pay special attention to the last paragraph.

In a nutshell:

  1. If you schedule an NSTimer, it becomes associated to the current run-loop which retains it.
  2. Furthermore, the timer retains its target.
  3. NSTimer instances are not reusable: "Once invalidated, timer objects cannot be reused".

So there is no point in retaining a scheduled timer in the first place.

If you need to hang on to it (e.g. in order to cancel it) use a non-owning (aka weak) reference to it.

Update:
For a thorough explanation, see my answer to your other question (it now has graphs — albeit as links only — and stuff).

Please consider the rest of this post (as well as many of my comments) as obsolete.

Your property becomes

@property (nonatomic, assign) NSTimer *walkTimer;

BTW:

-(void)cancelTimer {
    [self setWalkTimer:nil]; // great, now [self walkTimer] returns nil so
    [[self walkTimer] invalidate]; // here, you are calling [nil invalidate]
}

And since messaging nil is absolutely fine in Objective C, your crash miraculously vanishes...while your timer will happily continue to fire.

Edit

I've forgot to mention:
A timer wants a selector that takes one argument, which will be the timer that fired... Or is that just a typo?

Upvotes: 0

Daniel Dickison
Daniel Dickison

Reputation: 21882

You release before you call invalidate. That means by the time you call invalidate, you've already relinquished ownership of the timer. In practice, you end up calling invalidate on a deallocated timer instance.

What you should do is call invalidate before you call release. Since you are using a retained property, you can just set the property to nil:

// Schedule the timer.
self.walkTimer = [NSTimer scheduledTimerWith...];

// Cancel the timer.
[self.walkTimer invalidate];
self.walkTimer = nil;

Update to clear up any confusion regarding memory management

It's important to keep in mind the Memory Management Rules of Objective-C — you own an object if you call alloc, copy or retain on it, and if you own an object, you have to eventually call release. In this case, setWalkTimer: retains the timer because the property is declared as retain — that means you own the timer and must call release on it down the road. The invalidate method does not count as relinquishing ownership of the timer.

When you schedule a timer, the run loop retains it, and when the timer fires or is invalidated, the run loop releases it. But really, you don't need to know that — it's an implementation detail. The call to release by invalidate is only to balance the retain when the timer was scheduled on the run loop.

Upvotes: 4

Rob Napier
Rob Napier

Reputation: 299345

You need to invalidate before you release. After the timer has fire, you are the only one holding a retain on the timer. So when you call release, the timer deallocates. You then call invalidate on invalid memory and you crash.

Upvotes: 0

Related Questions