views:

1816

answers:

5

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];
}
+2  A: 

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.

Chris Hanson
+1  A: 

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

Graham Lee
+3  A: 

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?).

mmalc
A: 

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.

titaniumdecoy
This is confusing.1. In the code as presented there is no way to reference theData outside the method.2. The autorelease pool does not decrement the retain count *after the method returns*, so if you could reference theData outside the method it would only crash if the pool had been drained.
mmalc
Your analysis is incorrect. 1. theData is passed to the decoder object which retains it. 2. Yes, the autorelease pool does decrement the reference count after the method returns. Learn something about Cocoa before you give people incorrect advice.
titaniumdecoy
It is not clear why (1) is relevant to whether or not theData is accessible outside of the method.
mmalc
(2) The issue is not whether the autorelease pool decrements the reference count, it's *when* -- it does not do so *after the method returns*, but (typically) at the end of the event loop. So if you were able to access theData outside of the method, you potentially could do without crashing.
mmalc
Think about what you are saying! If an autoreleased object could be deallocated at any point during the execution of the method in which it is declared, it would be unusable because you would never know whether it had already been deallocated. Think about it!
titaniumdecoy
I'm not saying that an autoreleased object can be "deallocated at any point during the execution of the method in which it is declared"; I'm stating that your answer is misleading in stating that the object is released after end of the method. It's not.
mmalc
theData is released when the autorelease pool with which it's associated is released. Per my earlier comment, therefore, if there were a reference to theData outside of the method, then it would be possible to access without necessarily causing a crash.
mmalc
The fundamental point is that your answer is more confusing than helpful.
mmalc
theData cannot be referenced outside the method because it is declared inside the method.
titaniumdecoy
By reading your responses to other questions related to Cocoa memory management, it seems you have a firm grasp of the subject. It puzzles me why you have such difficulty understanding such a simple idea.
titaniumdecoy
Finally, let me ask you something: If theData is not released after the end of the method (it is), then you are suggesting that it may be released during the execution of the method. This is not the case. See my second comment.
titaniumdecoy
*You* raise the issue of referencing theData outside of the method at the end of your answer:"If you release theData at the end of this method [...] your app will crash when you try to use it.
mmalc
Re when theData is released; your answer implies that it is released *when the method exits*; per my previous comments, this is not the case. It is released when the autorelease pool with which it's associated is released. So if you could access theData outside the method, doing so might not crash.
mmalc
To emphasise: I am not suggesting, and have not suggested, that theData "may be released during the execution of the method".
mmalc
Yes, it is released when the autorelease pool with which it is associated is released, which is some time after the method returns.
titaniumdecoy
Which is not the same as implied in the answer...You haven't, though, explained why in your answer you refer to accessing theData outside of the method in the first place -- as originally noted, it's simply confusing.
mmalc
It's only confusing to you. I give up on trying to explain it to you.
titaniumdecoy
Again, the simple fact of the matter is that your answer is at best misleading. Please explain why it is helpful to suggest accessing theData outside of the method. Doing so introduces unnecessary ambiguity.
mmalc
A: 

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.

mmalc