0
votes

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

3
Re EDIT_001: You don't need to nil out a variable, unless you're using it to hold a cache (so that nil means “need to (re)create the cache”). You almost never need to nil out a local variable like myFrame.Peter Hosey
Hi Peter, myFrame is a pointer to an object, without the nil is it not unsafe as I have released what it was pointing at? I was under the impression it was better to nil pointers as messages can be sent to nil, but not to objects that don't exist?fuzzygoat
This is the dealloc method we're talking about. If you're sending a message to the pointer in any variable of an object that has deallocked (or is in the middle of deallocking), you are doing something wrong. Better to crash and find out about that bug immediately than for the message to go to nil and the bug to thus remain hidden.Peter Hosey

3 Answers

2
votes

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.

1
votes

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.

1
votes

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.