4

This is something that has puzzled me for awhile.

I have a NSTimer, added to the currentRunLoop, and if I don't retain it, it crashes.

NSTimer *timer = [[NSTimer timerWithTimeInterval:60.0 target:self selector:@selector(tryUnion:) userInfo:nil repeats:NO] retain];
[[NSRunLoop currentRunLoop] addTimer:timer forMode:NSDefaultRunLoopMode];

I have read that I don't need to retain it, as addTimer to the NSRunLoop does that.

later I invalidate and release (if I don't retain above, I don't release below- that's the crashing case):

- (void) tryUnion:(NSTimer*)aTimer {
[aTimer invalidate];
[aTimer release];
}

My questions are 1) How should I write this, if its possible to do what i've done without the retain/release. 2) analyze flags this or a potential leak of the object in "timer". As written here, is there the possibility of leaks, or is it just that analyzer isn't smart enough to know that there is a release in the function called by the timer?

ransomweaver
  • 472
  • 6
  • 19
  • Looks like what I should really be doing is this: [NSTimer scheduledTimerWithTimeInterval:60.0 target:self selector:@selector(tryUnion:) userInfo:nil repeats:NO]; with no runloop scheduling, and no invalidate in the selector. Yes/No anyone? I get no errors or analyzer complaints. – ransomweaver May 18 '11 at 02:12

3 Answers3

4

The rule is, don't release it if you didn't retain it. Here is a similar question which talks about that. So in your example code I would suggest not retaining it, and not releasing it.

But that's not your issue. Your issue is that you've created a non-repeating timer. You don't need to invalidate those, they invalidate themselves after they fire. From the NSTimer docs:

repeats
If YES, the timer will repeatedly reschedule itself until invalidated. If NO, the timer will be invalidated after it fires.

So you should neither retain/release that timer, nor invalidate it. Fire and forget.

As an aside, if you use scheduledTimerWithTimeInterval:target:selector:userInfo:repeats:, the adding of the timer to the runloop is done for you as well. So your whole thing would be like:

NSTimer *timer = [NSTimer scheduledTimerWithTimeInterval:60.0 target:self selector:@selector(tryUnion:) userInfo:nil repeats:NO];

Your timer function doesn't need to do anything with aTimer, just do whatever thing your timer is supposed to do; the timer will be invalidated and released without further intervention.

Community
  • 1
  • 1
zpasternack
  • 17,838
  • 2
  • 63
  • 81
1

Calling invalidate on a timer releases it so you don't need to have another release statement. See the docs here

enamrik
  • 2,292
  • 2
  • 27
  • 42
  • Hm, is the problem that since my timer doesn't repeat, I don't need to invalidate? because if I don't put retain (AND don't put release) it crashes when it runs the selector (with invalidate) – ransomweaver May 18 '11 at 01:51
  • after testing I find that I get the same crash at release by the runloop without invalidate in the selector – ransomweaver May 18 '11 at 01:56
1

Well you're right in that the NSTimer shouldn't have to be retained. The class method +(NSTimer *)timerWithInterval should, by rule, autorelease it's reference.

But NSTimer's invalidate message removes itself from the NSRunLoop and that releases it's reference (the only one left at the time). Your second [aTimer release] call is the cause of the crash.

Nick Bedford
  • 4,365
  • 30
  • 36
  • No, as stated in my post, If I put the timer in *timer with retain, I am not putting release in the callback. without BOTH retain and release, it fails with an error traced to >_CFRunLoopTimerDeallocate>CFRelease. – ransomweaver May 18 '11 at 01:47