Multithreaded iPhone app crashes with [NSAutoreleasePool release]

Question

Hi,

I have a memory management-related question in a multithreaded iPhone application. Let's say we have this method, that is called in a separate thread than the main-UI-thread:

- (BOOL)fetchAtIndex:(NSUInteger)index
{
    NSURL *theURL = [NSURL URLWithString:[queryURLs objectAtIndex:index]];
    // Pay attention to this line:
    NSData *theData = [[NetworkHelper fetchFromNetwork:theURL] retain];

    // Some code here...

    // Now what should I do before returning result?
    //[theData release]; ??
    //[theData autorelease]; ??
    return YES;
}

As you can see, I'm retaining the NSData I got back from my network operation. The question is: why shouldn't I release (or autorelease) it at the end of my method? The only way I made it work is to use retain at first, and nothing then. If I use any other combination (nothing at all; retain then release or autorelease), my program crashes with EXC_BAD_ACCESS when I release the thread's NSAutoreleasePool. What am I missing?

FYI, here is the main code for the thread:

- (void)threadedDataFetching;
{
    // Create an autorelease pool for this thread
    NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];

    // Reload data in separate thread
    [self fetchAtIndex:0];

    // Signal the main thread that fetching is finished
    [self performSelectorOnMainThread:@selector(finishedFetchingAll) withObject:nil waitUntilDone:NO];

    // Release all objects in the autorelease pool
    [pool release]; // This line causes EXC_BAD_ACCESS
}

Thanks for your help!

Answer 1

You must not release things you have not retained yourself (using either retain or implictly by methods with: init, new, or copy in their name).

If you retain result from fetchFromNetwork, then you must release it. Both release and autorelease should work (don't touch object after release, it's safest to set fields/variables to nil after release).

If you're not keeping the data, then you don't even need to retain it. [NetworkHelper fetchFromNetwork] should return autoreleased object. Body of fetchFromNetwork could look like this:

NSData *data = [[NSData alloc] init];
// stuff happens
return [data autorelease];

or

NSData *data = [otherObject dataFromOtherObject];
// stuff happens
return data; // don't (auto)release, since you haven't retained

If in doubt, err on the leaky side and run application via "Leaks" Instruments or LLVM checker.

Answer 2

The fact that you're on a separate thread is likely not relevant here. Memory management is the same, and you should be balancing your retain/release of NSData just as if this were the main thread. The fact that you're crashing when draining the autorelease pool suggests that you've probably done something with theData that you haven't showed us here (which seems obvious since the code as shown isn't very useful). What are you doing with theData that is requiring someone else to retain it?

Answer 3

As others have noted, it's not actually relevant that it's multithreaded in this instance. It's worth reading up on the Objective C Memory Management Guidelines that Apple produces.

Roughly speaking, any objects that you explicitly [[Foo alloc] init] yourself, you should own the responsibility for cleaning up. If you're returning this value from a method, you need to autorelease it as well.

Any object you get from a caller that doesn't begin with initWith (such as [Foo emptyFoo]) it's the responsibility of the caller to own the resource (i.e. autorelease it).

If you get passed an object and want to keep it outside of the method call (i.e. save it to an instance field) you must retain it. When you're done, you release it, usually in the destructor. If you're using Objective C properties, defining it as @retain will kick in this behaviour for you automatically; in the destructor, you can just set the property to nil.