tags:

views:

95

answers:

7

If I allocate an object in a method, 'getASprocket' and call it this way, will there be a leak here?

Sprocket *sprock = [Sprocket getASprocket];

// store this returned value as an ivar
ivarSprock = [sprock retain];

// release the originally acquired object
[sprock release];

The Sprocket object is allocated and returned this way:

– (Sprocket *)getASprocket {
    Sprocket *sprocket;

    sprocket = [[Sprocket alloc] init];

    return [sprocket retain];
}

Also, would changing from '[sprocket retain];' inside the 'aSprocket' method to 'return [sprocket autorelease];' make worse performance hit?

A: 

You should probably read the memory management guide. It's not long, and it's a very healthy reading.

Regarding your specific case, in getASprocket you should change:

return [sprocket retain];

to:

return [sprocket autorelease];

since the caller of the method should not be in charge of releasing the object (unless he retains it himself, obviously).

Also, it is not about performance at all - it's all memory management.

Aviad Ben Dov
A: 

All this is indeed causing leaks. So lets go over the reference count of your Sprocket, in getASprocket you are saying Sprocet alloc which increases the count and then retain, which increases it again you are at +2 here, then in the first chunk of code you are retaining again +3, then releasing +2, you have leaked the object with a +2 reference count. What you can do i s

-(Sprocket*)getASprocket)
{
    return [[[Sprocket alloc] init] autorelease]
}

this keeps the reference count at 0

Daniel
A: 

There's a leak in your getASprocket: method.

sprocket = [[Sprocket alloc] init];
return [sprocket retain];

You're retaining sprocket twice, once with alloc/init and once with retain, which is not something you want to do. Use

return [sprocket autorelease];

Also be sure to set iVarSprock to nil before you release sprocket or it'll be pointing to some random memory.

Terry Wilcox
A: 

While not technically incorrect, the getASprocket method is very unlike typical Cocoa and Cocoa Touch development. If you don't have additional logic in the method, you would be better served doing this:

ivarSprock = [[ Sprocket alloc ] init ];

// Other Code Here

[ ivarSprock release ];
Jeff Kelley
A: 

As an aside, if you are concerned about performance in an application, the thing that you should look at first and foremost is the algorithms you are using to implement your software. That, first and foremost, is what will improve the performance. Only after you have implemented strong algorithms should you (if you need to) go back and take a look at the more minute worries.

JasCav
A: 

Hi,

Please see this good explanation page Especially sub-page #7

each retain created another memory object. so lets see what we've got:
in getASprocket :

sprocket = [[Sprocket alloc] init];

+1

return [sprocket retain];

+1

and on your method:

ivarSprock = [sprock retain];

+1

[sprock release];

-1

What should we do? Well we should let the Sprock go by making it autorelease:

  return [[[Sprocket alloc] init] autorelease]

or in the words of the reffered link above:

In most cases, the setter for an instance variable should just autorelease the old object, and retain the new one. You then just make sure to release it in dealloc as well.

yn2
A: 

Given the way you wrote your method, there is in fact a leak in your code. You'd want to write it like this (otherwise "sprock" ends up with a retain count of 1, and therefore not dealloc'd):

+ (Sprocket *)getASprocket
{
    return [[[Sprocket alloc] init] autorelease];
}

Retain counts are really very simple once you get the hang of it. Just follow these rules:

  • Objects created by alloc or copy have a retain count of 1 (and must be released)
  • Assume all other objects have a retain count of 1, but are autoreleased (like the above method)
  • Retain objects you wish to keep around
  • Release objects when you are no longer interested in them (never call dealloc yourself)

Using autorelease pools does have a slight performance impact, but it just makes things so much simpler because you don't have to know the internal implementation details of each method.

Michael