views:

141

answers:

1

I am leaking memory on this:

my custom class:

+ (id)vectorWithX:(float)dimx Y:(float)dimy{
return [[[Vector alloc] initVectorWithX:dimx Y:dimy] autorelease]; }


- (Vector*)add:(Vector*)q {
return [[[Vector vectorWithX:x+q.x Y:y+q.y] retain] autorelease]; }

in app delegate I initiate it:

Vector *v1 = [[Vector alloc] initVector];
Vector *v2 = [[Vector alloc] initVector];       
Vector *vtotal = [[v1 add:v2] retain];

[v1 release];
[v2 release];
[vtotal release];

How this leaks? I release or autorelease them properly. The app crashes immediately if I don't retain these, because of an early release I guess. It also crashes if I add another release.

+2  A: 

Why do you think you're leaking memory? Start with that. And what object do you crash on accessing? That will tell you most likely what object your under-retain is related to. If I had to make a guess, I'd suspect initVector, just because it's a very strange name for a method. What does it do? Why is it not called just "init?"

Is this threaded code? You're doing a lot more retain/autorelease than is generally appropriate. You do not need to retain an object in order to hold onto it through the current event loop. Generally speaking you only retain ivars, because those are what you need through the next event loop. If you have a lot of calls to retain outside of accessors, then you're almost certainly mis-managing memory. The above should be:

+ (id)vectorWithX:(float)dimx y:(float)dimy
{
    return [[[Vector alloc] initVectorWithX:dimx y:dimy] autorelease];
}

- (Vector*)add:(Vector*)q
{
    return [Vector vectorWithX:(self.x + q.x) y:(self.y + q.y)];
}

...

Vector *v1 = [[Vector alloc] initVector];
Vector *v2 = [[Vector alloc] initVector];
Vector *vtotal = [v1 add:v2];
...
[v1 release];
[v2 release];

Personally, I would handle v1/v2 with autorelease because I think it makes the code more maintainable and understandable, but there are other schools of thought:

Vector *v1 = [[[Vector alloc] initVector] autorelease];
Vector *v2 = [[[Vector alloc] initVector] autorelease];
Vector *vtotal = [v1 add:v2];
Rob Napier
Hey @Rob, you've over-released your vectors in your clean-up example (autorelease after initialization then specifically release at the end).
Jason Coco
I got both of them backwards.... Thanks. Fixed.
Rob Napier
@gok, Looking at your code in the above comment, the method in question should be just "init" not "initVector." And you should make sure that you're using accessors rather than accessing your ivars directly. Not using accessors is the #1 cause of memory errors like what you're seeing. Yes, I know these are floats. Use accessors anyway. Likely your actual leak is elsewhere; probably in something else that retains vector. (But why do you think you have a leak, btw?)
Rob Napier
@Rob, thanks, I modified the code a lot and removed a lot of retains, I will post the updated code here once I have time to have a look at it again. I will try to use accessors for ivars. let's see.
gok