views:

83

answers:

3

I am allocating myMDD in main which contains an NSMutableArray instance variable (alloc/init-ed in init). When I add items to the NSMutableArray (frameList) I release after adding. The array and the objects it now contains are released at the bottom of main.

int main (int argc, const char * argv[]) {
    NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];

    MDD *myMDD = [[MDD alloc] init];
    Frame *myFrame = [[Frame alloc] init];

    [myMDD addFrame:myFrame];

    [myMDD release];
    [pool drain];
    return 0;
}

// METHOD_ mdd addFrame:
-(void)addFrame:(Frame*) inFrame {
    [frameList addObject:inFrame];
    [inFrame release];
}

// METHOD_ mdd dealloc
-(void)dealloc {
    NSLog(@"_deal...: %@", self);
    [frameList release];
    [super dealloc];
}

My question is that the "static analyser" reports a potential memory leak, prefering to have the release for frame added main. (i.e)

 int main (int argc, const char * argv[]) {

    ...

 [myFrame release]; // Added
    [myMDD release];
    [pool drain];
    return 0;
}

// METHOD_ mdd addFrame:
-(void)addFrame:(Frame*) inFrame {
    [frameList addObject:inFrame];
    // [inFrame release];
}

I can see why this is, if I alloc myMDD and never call addFrame then I need to release it. Maybe its just a case of adding a autorelease to myMDD, but would that work in the situation where I call addFrame and the NSMutableArray is releasing the object?

EDIT_001

Changed to ...

int main (int argc, const char * argv[]) {
    ...
    [myMDD addFrame:myFrame];
    [myFrame release];
    myFrame = nil;

    [myMDD release];
    [pool drain];
    return 0;
}

// METHOD_ mdd addFrame:
-(void)addFrame:(Frame*) inFrame {
    [frameList addObject:inFrame];
}

gary

+1  A: 

As per convention, an add method should just retain the object if needed, not release it. And as a general rule, you should not release object that you did not retain, in your example the scope where you retained (created) the frame is not the same as in the addFrame method.

By scope I mean logic scope, not language scope.

In that particular example, you must call release just after addFrame. But the release should not be in the addFrame method.

Thanks Nicolas, that makes sense, It sort of felt more tidy with the release in the method, but I had a feeling I was not quite doing things the right way. Thanks again.
fuzzygoat
+2  A: 

The reason you got that warning is because an NSMutableDArray retains any object put into it; likewise, when an NSMutableArray is released, it also releases any object contained within it. So let's look at your code.

This line:

Frame *myFrame = [[Frame alloc] init];

creates a new instance of Frame called myFrame. myFrame has a retain count of 1, because you used alloc/init to create it.

You then pass this to addFrame::

[myMDD addFrame:myFrame];

Which in turn puts it into an instance of an NSMutableArray:

[frameList addObject:inFrame];

At this point, inFrame and myFrame point to the same object. When added to the array, this object's retain count is incremented, so now it is 2.

Later on, back in main, you release myMDD, which releases frameList. Assuming frameList now has a retain count of 0, it is deallocated -- and, as an NSMutableArray, it releases any object it contains, which includes the object pointed to my myFrame.

So now myFrame's retain count is 1...so it doesn't get released, and you have a memory leak.

One Cocoa-y way to solve the problem is by autorelease myFrame:

Frame *myFrame = [[[Frame alloc] init] autorelease];

Which means it won't leak. Then, use the -[MDD dealloc] method in your second example (Edit_001). You're right that you shouldn't release inFrame in your addFrame method, since you're not retaining it.

mipadi
Thanks mipadi, for now I am going to use EDIT_001 and remove the release from addFrame, I can see why thats not so good an idea now. As for myFrame I am going to release it in main() again as per EDIT_001, I do appreciate that I could also use autorelease as you suggested.
fuzzygoat
+1  A: 

In most cases, Cocoa provides class methods that initialize and return an autoreleased version of an object. i.e. [NSMutableDictionary dictionary] vs [[NSMutableDictionary alloc] init].

I advise to always use the class methods where possible if you're creating an object that you won't need to keep around or if you going to store it in a collection (NSArray, NSDictionary, NSSet, etc).

The general rule then is to only alloc objects your class owns directly (i.e. an instace or class variable, not inside a collection) and to use the class methods for all other cases.

Adrian