Releasing a delegating object in its delegate callback method

Question

I'm trying to figure out what the recommended practice is for the following situation. Certain objects, such as CLLocationManager or MKReverseGeocoder, send their results asynchronously to a delegate callback method. Is it OK to release that CLLocationManager or MKReverseGeocoder instance (or whatever class it may be) in the callback method? The point is that you no longer need that object around, so you tell it to stop sending updates, set its delegate to nil, and release the object.

Pseudo code:

@interface SomeClass <CLLocationManagerDelegate>
...
@end

@implementation SomeClass

...

- (void)someMethod
{
    CLLocationManager* locManager = [[CLLocationManager alloc] init];
    locManager.delegate = self;
    [locManager startUpdatingLocation];
}

- (void)locationManager:(CLLocationManager *)manager didUpdateToLocation:(CLLocation *)newLocation fromLocation:(CLLocation *)oldLocation
{
    // Do something with the location
    // ...

    [manager stopUpdatingLocation];
    manager.delegate = nil;
    [manager release];
}

@end

I am wondering if this usage pattern is considered to be always OK, if it's considered to be never OK, or if it depends on the class?

There is an obvious case where releasing the delegating object would go wrong and that is if it needs to do stuff after it has notified the delegate. If the delegate releases the object, its memory may get overwritten and the application crashes. (That appears to be what happens in my app with CLLocationManager in a particular circumstance, both only on the simulator. I'm trying to figure out if it is a simulator bug or if what I am doing is fundamentally flawed.)

I have been searching and I cannot find a conclusive answer to this. Does anyone have an authoritative source that can answer this question?

Answer 1

This is a very good question, I waited few hours in hope someone would give a sufficient answer, but because no one even replied, I'll give it a try. First I'll comment on your approach, then I try to suggest how I would go around this.

It's definitely a very bad idea to release - thus deallocate an object from its delegate. Just think about how objects (like a CLLocationManager) do call their delegates - they just call them in the middle of some method. When call to delegate is finished, code's execution comes back to a method of an object that has already been deallocated. BAM!

Let's forget for a moment about the fact that this is a bad idea. I see two options how to fix that easily. First, autorelease instead of release gives an object a little longer time spam - it'd at least survive returning from delegate. That should be sufficient for most of the cases, at least if author of API did her job well and encapsulated logic behind the main API class (in case of CLLocationManager it might be waiting for GPS to turn off...). Second option would be to delay releasing (performSelector:withObject:afterDelay: comes to mind), but that's more of a workaround for badly implemented APIs.

So if releasing it is not a good idea, then what is ?

Well, what do you really gain by releasing a CLLocationManager ? Freeing those few bytes of memory is not going to save your app from terminating when system is out of memory. Anyway, is it really only once that you need the current user's location ?

I suggest that you encapsulate tasks related to CLLocationManager into a separate class, probably even a singleton - that class would become its delegate, and it would take care of communicating with CLLocationManager and informing your application about results (probably by sending NSNotification). CLLocationManager would be released from that class's dealloc, and never as a result of a delegate callback. stopUpdatingLocation should suffice, freeing few bytes of memory - well, you can do it when your app is entering background, but as long as your app runs, freeing those few bytes do not make any significant improvement in memory consumption.

** Addition **

It's natural, and correct, for a delegate to have ownership of an object for which it acts as delegate. But the delegate should not release the object as a result of a callback. There's one exception to this though, and it is the callback telling you processing is over. As an example for this is NSURLConnection's connectionDidFinishLoading: which states in documentation "The delegate will receive no further messages". You can have a class downloading bunch of files, each with a different NSURLConnection (having your class as delegate), allocating AND releasing them as download of files progress.

CLLocationManager's behavior is different. You should have only one instance of CLLocationManager in your program. That instance is managed by some code, probably a singleton - one that could be released when application goes into background, reinitialized when it wakes up. CLLocationManager's lifespan would be the same as of its managing class, which acts as delegate as well.

Answer 2

No. That pattern is always considered to be wrong. It breaks the Cocoa Memory Management Rules. The manager object was passed in as a parameter. You did not obtain it by new, alloc or copy, nor did you retain it. You therefore must not release it.

Answer 3

Just like Michal said there is absolutely no good reason to release the manager object for memeory savings. Also, just likeJeremyP said, it would be completely incorrect pattern wise and design wise to release an object inside another function that only receives that object. It goes against all the rules.

However, what would be correct is to simply stop the updates and set the managers delegate to nil as you are already doing. So the only thing you need to remove is the [manager release] line.

My guess is that you are creating a manager in a local scope and therefore are trying to work out how to make sure the the manager gets released. The correct thing to do here would be to create a manager as an instance variable of your class and then just release it in the class dealloc.

Answer 4

Another reason why your pattern is a bad idea, is that for something like a CLLocationManager you generally want to tell it to stop receiving updates if the screen goes to sleep - you can only do that if you maintain a reference somewhere that you can tell to start/stop. And if you are maintaining a reference, then you might as well fully manage it.