views:

129

answers:

2

According to the rules of memory management in a non garbage collected world, one is not supposed to retain a the calling object in a delegate. Scenario goes like this:

I have a class that inherits from UITableViewController and contains a search bar. I run expensive search operations in a secondary thread. This is all done with an NSOperationQueue and subclasses NSOperation instances. I pass the controller as a delegate that adheres to a callback protocol into the NSOperation.

There are edge cases when the application crashes because once an item is selected from the UITableViewController, I dismiss it and thus its retain count goes to 0 and dealloc gets invoked on it. The delegate didn't get to send its message in time as the results are being passed at about the same time the dealloc happens.

Should I design this differently? Should I call retain on my controller from the delegate to ensure it exists until the NSOperation itself is dealloc'd? Will this cause a memory leak? Right now if I put a retain on the controller, the crashes goes away. I don't want to leak memory though and need to understand if there are cases where retaining the delegate makes sense.

Just to recap.

UITableViewController creates an NSOperationQueue and NSOperation that gets embedded into the queue. The UITableViewController passes itself as a delegate to NSOperation. NSOperation calls a method on UITableViewController when it's ready. If I retain the UITableViewController, I guarantee it's there, but I'm not sure if I'm leaking memory. If I only use an assign property, edge cases occur where the UITableViewController gets dealloc'd and objc_msgSend() gets called on an object that doesn't exist in memory and a crash is imminent.

+2  A: 

In the general case, a delegate owns the objects it has set itself as the delegate to, or at least retains references to them directly or indirectly. In the dealloc method, a delegate should either release all objects that it is the delegate of in such a way that prevents future callbacks, like NSTimer invalidate, or clear the delegate member of those objects that may persist.

While it is only convention that prevents retaining a delegate, it is convention based on good design. In your case, wouldn't the results be discarded anyway since the delegate is being disposed?

You can make the delegate property of your NSOperation atomic by not setting the nonatomic flag and synthesizing the getter and setter. Or you can use performSelectorOnMainThread before using the delegate member.

To recap, there is usually a better solution than retaining the delegate.

drawnonward
What benefit does setting the property to atomic in this case?
randombits
Atomicity is relevant for delegates to objects running in other threads. I do not know off hand if NSOperations are run in a separate thread, so just covering all the bases.
drawnonward
drawnonward: They are. That's mainly what they're for, in fact.
Peter Hosey
A: 

You could retain the UITableViewController at the beginning of the NSOperation and release it at its end.
Alternatively, you could set it to nil after it is released so the dangling call from the NSOperation won't crash your program.
Depending on how the NSOperation is executed you could also autorelease the UITableViewController instead of releasing it, so the NSOperation can still use it.

However, all these things really only cure the symptoms, not the illness. The correct way to do it is outlined by drawnonward.

BastiBechtold