Problem invalidating NSTimer in dealloc

Question

Following is my code:

.h file:

#import "Foundation/Foundation.h"
@interface GObject:NSObject{
    NSTimer* m_Timer;
}
@property(nonatomic, retain) NSTimer* m_Timer;

- (void)Initialize;
- (void)TimerCallback:(NSTimer*)pTimer;
@end

.m file:

@implementation GObject

@synthesize m_Timer

- (void) Initialize{
    self.m_Timer = [NSTimer scheduledTimerWithTimeInterval:5.0 
                       target:self 
                       selector: @selector(TimerCallback:) 
                       userInfo: nil 
                       repeats: YES];

}

- (void)TimerCallback:(NSTimer*)pTimer {
    //Some Code
}
- (void)dealloc {
    [m_Timer invalidate]; //--Crashes Here
    [m_Timer release];
    m_Timer = nil;
    [super dealloc];
}
@end

Now when the dealloc gets called, the program crashes in the line invalidating the timer. The next two lines don't even get called. I get a "EXC_BAD_ACCESS" error. Can anyone tell me why that might be happening, and what is the proper way of stopping and releasing a NSTimer member variable in a class.

Answer 1

Try @property (nonatomic, assign) NSTimer *m_Timer; instead.

Answer 2

There are a few things you should address to clean this up.

Your class declaration is a little off. If you want to inherit from NSObject, the correct syntax is:

@interface GObject : NSObject

In your implementation, you should implement - (id)init rather than - (void)Initialize. There is no instance method - (void)Initialize... there is a static method + (void)initialize. Note the + and the difference in capitalization, which is significant): the initialize method is called once in your program before the class receives its first method.

In this case, your Initialize method is not being called at all (it is spelled wrong, and it is an instance method instead of a static method). Instead, you want to implement init, which is the designated initializer for NSObject instances:

- (id)init {
    if (self = [super init]) {
        self.m_Timer = [NSTimer scheduledTimerWithTimeInterval:5.0 
                   target:self 
                   selector: @selector(TimerCallback:) 
                   userInfo: nil 
                   repeats: YES];
    }
    return self;
}

Lastly, be sure to use the @ symbol before the property statement:

@property(nonatomic, retain) NSTimer* m_Timer;

And don't forget to synthesize it in your implementation:

@implementation GObject

@synthesize m_Timer;

Answer 3

I did some research and tests, and could figure out the answer to my own question. Ok, here it goes:

Whenever we assign self as target to the NSTimer, the timer holds a reference to our object. If the timer is repeating (or has a long time period), it would not get invalidated on its own (or will take too long to invalidate automatically, if not repeating). So, even if the object is released in the meantime, it wouldn't call the dealloc method, as the retain count would at least be 1. Now, I was forcibly trying to dellocate my object by calling repeated releases on it till the retain count became 0. That was my mistake.

But if you do not do that, your object will keep alive, and you will eventually have a memory leak as you lose the rest of the references to that object by various releases. The only one kept will be with the NSTimer. This sounds like a deadlock situation. My code crashed, because when the delloc tried to invalidate the NSTimer, it tried to release the reference that it was holding. But since I had been a smartass and reduced the retain count to 0 already, that would cause a memory exception.

To solve this, firstly I cleaned up my act and removed the code to forcibly dellocate the object. Then just before I wanted the object to be dellocated, I called the NSTimer's invalidate function. This released the target instance that the Timer had. After that, calling release on my object successfully dellocated it.

The bottom line is, if your object has NSTimers that repeat or do not invalidate (get over) automatically, never invalidate them in the delloc function. The delloc will not get called till the timer is holding the instance to your object. Instead have a cleanup function to invalidate the timers, before releasing the object. That is the solution that I came up with. If there is a better one out there, I most certainly want to know.