Memory leaking with [NSKeyedUnarchiver decodeObjectForKey]

Question

Every time I call this method my NSMutableData is leaking and I cannot figure out how to plug it. theData's retain count is upped by one after the decoder is allocated and initialized and I have no idea why. I am stuck with a retain count of 2 at the end of the method and attempting to release it causes an app crash.

- (void)readVenueArchiveFile:(NSString *)inFile key:(NSString *)inKey
{
    NSMutableData *theData;
    NSKeyedUnarchiver *decoder;


    theData = [NSData dataWithContentsOfFile:inFile];

    decoder = [[NSKeyedUnarchiver alloc] initForReadingWithData:theData];

    venueIOList = [[decoder decodeObjectForKey:inKey] mutableCopy];

    [decoder finishDecoding];

    [decoder release];
}

Answer 1

Don't worry about retain counts, worry about balance within a method. What you're doing in this method looks correct, assuming venueIOList is an instance variable.

To expand on my answer a little bit: The unarchiver might be retaining your data during the unarchive operation, and then sending the data -autorelease when it's done instead of -release. Since that's not something you did, it's not something you have to care about.

Answer 2

The ultimate source for refcount-related memory management enlightenment is still, IMO, "Hold Me, Use Me, Free Me" from Stepwise.

Answer 3

I would suggest replacing this line:

venueIOList = [[decoder decodeObjectForKey:inKey] mutableCopy];

with:

ListClassName *decodedList = [decoder decodeObjectForKey:inKey];
self.venueIOList = decodedList;

This makes the memory management of decodedList clear. It is considered best practice to assign instance variables using an accessor method (except in init methods). In your current implementation, if you ever invoke readVenueArchiveFile: a second time on the same object, you will leak (as you will if decodedList already has a value). Moreover, you can put the copy logic in your accessor method and forget about it rather than having to remember mutableCopy every time you assign a new value (assuming there's a good reason to make a mutable copy anyway?).

Answer 4

Your code is correct; there is no memory leak.

theData = [NSData dataWithContentsOfFile:inFile];

is equivalent to

theData = [[[NSData alloc] initWithContentsOfFile:inFile] autorelease];

At this point theData has a reference count of 1 (if less, it would be deallocated). The reference count will be automatically decremented at some point in the future by the autorelease pool.

decoder = [[NSKeyedUnarchiver alloc] initForReadingWithData:theData];

The decoder object keeps a reference to theData which increments its reference count to 2.

After the method returns, the autorelease pool decrements this value to 1. If you release theData at the end of this method, the reference count will become 0, the object will be deallocated, and your app will crash when you try to use it.

Answer 5

Reducing peak memory footprint

In general, it is considered best practice to avoid generating autoreleased objects.

[Most of this paragraph amended from this question.] Since you typically(1) don't have direct control over their lifetime, autoreleased objects can persist for a comparatively long time and unnecessarily increase the memory footprint of your application. Whilst on the desktop this may be of little consequence, on more constrained platforms this can be a significant issue. On all platforms, therefore, and especially on more constrained platforms, where possible you are strongly discouraged from using methods that would lead to autoreleased objects and instead encouraged to use the alloc/init pattern.

I would suggest replacing this:

theData = [NSData dataWithContentsOfFile:inFile];

with:

theData = [[NSData alloc] initWithContentsOfFile:inFile];

then at the end of the method add:

[theData release];

This means that theData will be deallocated before the method exits. You should end up with:

- (void)readVenueArchiveFile:(NSString *)inFile key:(NSString *)inKey
{
    NSMutableData *theData;
    NSKeyedUnarchiver *decoder;

    theData = [[NSData alloc] initWithContentsOfFile:inFile];
    decoder = [[NSKeyedUnarchiver alloc] initForReadingWithData:theData];
    ListClassName *decodedList = [decoder decodeObjectForKey:inKey];
    self.venueIOList = decodedList;
    [decoder finishDecoding];
    [decoder release];
    [theData release];

}

This makes the memory management semantics clear, and reclaims memory as quickly as possible.

(1) You can take control by using your own local autorelease pools. For more on this, see Apple's Memory Management Programming Guide.