iPhone: Is this a leak or not

Question

Recently someone on Stack Overflow told me that the code below does not leak, that the property handles retention itself:

self.locationManager = [[CLLocationManager alloc] init];

in dealloc :

   self.locationManager = nil;

where in the .h file:

@property (nonatomic, retain) CLLocationManager *locationManager;

I thought that was an obvious leak and believed this should fix the leak:

self.locationManager = [[[CLLocationManager alloc] init] autorelease];

However he claimed that would not work, because in his words : "you don't autorelease properties of a class. The autogenerated accessor of a property defined to retain will handle retention automatically"

And he made me wonder if he is wrong or have I not understood memory management at all?

EDIT 1: Is the code

self.myName=[NSSting stringWithFormat:@"%@ is correct.", @"TechZen"];

any different than

 self.locationManager = [[[CLLocationManager alloc] init] autorelease];

memory management-wise?

The guy says the first one is correct and refuses the second one. Why would the second one be SO WRONG? As far as I can see both assign autoreleased instances to some properties, but somehow there's still a stubborn argument that second one is wrong. I cannot see it, any help would be so welcome.

Answer 1

Add this line underneath... It obviously gives your the retain count for locationManager. This will tell you if you need to release it manually or not.

NSLog(@"retainCount:%d", [locationManager retainCount]);

edit:

[locationManager release];

Answer 2

@jnic: you are amazingly wrong. as far as I understand it, the previous value of the property is sent the release message, not the object you want to assign to the property. so, yes the proposed code leaks indeed and you have to send an autorelease message just like you thought, ahmet emrah

Answer 3

Counting retains and releases helps in this case. It is most definitely a leak. Your locationManager object will be retained 2 times: once by the alloc/init calls, and once by the property. Setting the property to nil will only release the locationManager once.

For the examples given in Edit 1, they are indeed the same. It sounds like the other developer has an aversion to immediately autoreleasing or doesn't quite understand what autorelease does.

Answer 4

The semantics of the retain property option are:

• the old value (if any) gets a release message
• the new value gets a retain message

Therefore, your CLLocationManager instance will have a retain-count of 2 after the setter. One from alloc and one from the retaining setter. You should send a release message right after the setter:

CLLocationMamnager *aLocationManager = [[CLLocationManager alloc] init];
self.locationManager = aLocationManager;
[aLocationManager release];

Alternatively, add it to the autorelease pool so that it will at least be free'd eventually. Like you wrote yourself:

self.locationManager = [[[CLLocationManager alloc] init] autorelease];

Even better, don't use the retain property option. Leave it as assign (the default) and you are set to go since you are using a retained object anyway.

Answer 5

Okay, here's the deal.

When you define a property like so...

@property (nonAtomic, retain) NSString myName;

...because of the defaults of the property command its actually like defining it as:

@property (nonAtomic, readwrite, retain, getter=getMyName,setter=setMyName) NSString myName;

When you use @synthesize myName; behind the scenes, the complier generates a getter method that looks something like this:

-(void) setMyName:(NSString *) aString{
    if (!(myString==aString) { //test if a string is the same **object** as the current myString
        if (aString != nil) { // if nil we don't want to send a retain
            [aString retain]; // increment the retain count by one
        }        
        [myString release]; //decrement the retain count of the currently assigned string by one.
        myString=nil; //set the pointer to nil to ensure we don't point to the old string object
        myString=aString; //assign the newly retained object to the myString symbol     
    }
}

As you can see, any string from any source, of any prior retain count, autoreleased or not, will be automatically retained by the method upon assignment and when a new value is assigned, it will be automatically released by the method. Multiple assignments will not stack the retain count. As long as you use the generated setter, the object assigned (in this case aString) will always have a retain count that will keep it alive in the class.

This is why you can do this...

self.myName=[NSSting stringWithFormat:@"%@ is correct.", @"TechZen"];

without having to do this:

self.myName=[[NSSting stringWithFormat:@"%@ is correct.", @"TechZen"] retain];

... and not have to worry if the string value will suddenly disappear when the autoreleasepool drains.

However if you ever call...

[self.myName release]; 

...anywhere outside the dealloc, then the object in the property my be nilled unless you track it relentlessly. By the same token, if you call..

[self.myName retain];

... anywhere, then the object in the property will leak (possibly even after the self object has been deallocated.)

This is why I say never to retain or autorelease any object assigned or copied in a property. It's not only pointless but counter productive. It follows that you only want to call release on a property when you are done with the self object because the efficient tracking of the retain count by the setter means that you can nil the property even though you may still needed.

Autorelease is never needed for a property because the property is always retained by the self object and any other object should handle retention internally if it uses the self object's property.

Once you understand what goes on inside the generated accessors, the rules are obvious. Never retain a property's object explicitly. Never release a property's object save in dealloc. Never autorelease a property's object.

The obvious corollary to these rules is always to use the self.propertyName references internally in the self object to ensure that the retention of the property's is automatically managed.

Answer 6

The following statement is retained twice, and as such must be released twice:

self.locationManager = [[CLLocationManager alloc] init];

This is probably best written as follows:

self.locationManager = [[[CLLocationManager alloc] init]autorelease];

Now, this variable has been retained just once, and you can release it in the dealloc function of your class.

Also, if you run the following line of code, a release is called:

locationManager = nil;

Since locationManager is synthesized, when you set it to nil, it gets released first.

Furthermore, if you did the following, locationManager would first be released, then reset behind the scenes:

self.locationManager = foo;

Finally, the following would crash with an exc_bad_access, because you are double releasing locationManager when you set it to foo:

self.locationManager = [[[CLLocationManager alloc] init]autorelease];
[locationManager release];
self.locationManager = foo;

Answer 7

Simple rule of thumb: If a property is marked as "retain", always give it an autoreleased variable (if you are creating it) unless of course you also NEED to retain it elsewhere.