0

Looks like I did not understand memory management in Objective C... sigh.

I have the following code (note that in my case, placemark.thoroughfare and placemark.subThoroughfare are both filled with valid data, thus both if-conditions will be TRUE

item is tied to a ManagedObjectContext. The managed variables in item such as place have setters/getters created with @dynamic. Thus, the declaration is

@property (nonatomic, retain) NSString *place;
@dynamic place;

Later in the code, in the ReverseGeocoderDelegate, I access it:

- (void)reverseGeocoder:(MKReverseGeocoder *)geocoder didFindPlacemark:(MKPlacemark *)placemark {

if (placemark.thoroughfare) {
    [item.place release];
    item.place = [NSString stringWithFormat:@"%@ ", placemark.thoroughfare];        
} else {
    [item.place release];
    item.place = @"Unknown Place";
}
if (placemark.thoroughfare && placemark.subThoroughfare) {
// *** problem is here ***
    [item.place release];
    item.place = [NSString stringWithFormat:@"%@ %@", placemark.thoroughfare , placemark.subThoroughfare];
}

If I do not release item.place at the marked location in the code, Instruments finds a memory leak there. If I do, the program crashes as soon as I try to access item.place outside the offending method.

Any ideas?

Axel
  • 1,716
  • 1
  • 16
  • 28
  • It’d be useful to paste the declaration of the `place` property, as well as the definition of its setter method (if any). –  Jan 16 '11 at 11:41
  • Do you mean you generated your getters/setters with `@synthesize`? – Jesse Rusak Jan 16 '11 at 13:46
  • I used @dynamic, as I understood this is the right thing for properties tied to a ManagedObjectContext as in my case – Axel Jan 16 '11 at 16:08

2 Answers2

2

First of all, I would change the logic like this:

NSString *newPlace = nil;

if (placemark.thoroughfare && placemark.subThoroughfare) {
    newPlace = [NSString stringWithFormat:@"%@ %@", placemark.thoroughfare , placemark.subThoroughfare];
}
else {
    if (placemark.thoroughfare) {
        newPlace = [NSString stringWithFormat:@"%@ ", placemark.thoroughfare];
    }
    else {
        newPlace = @"Unknown Place";
    }
}

item.place = newPlace;    // Either nil of valid string can be assigned to

Using release for simply re-initializing pointer is not so recommended by many. You don't have to do this if it was to save some memory.

Your previous logic does releasing twice. What release does initially is simply decrementing retainCount. If it's 0, then the object is deallocated. Object will not be deallocated until its retainCount is 0.

Assuming your item has the property retain and since stringWithFormat: returns autoreleased string, so at the 2nd release, you were trying to release what was going to be autoreleased anyway.

Best way to clearing an object multiple times is simply assigning nil to it.

petershine
  • 3,190
  • 1
  • 25
  • 49
  • I changed it according to your suggestion, but still, instruments shows leaks in strange places. Because the whole project is too big, I have the impression that I have to fix it on my own. Thanks for your help and yes: I went to assigning nil to my retained properties instead of releasing them. Seems much safer and should have the same effect. – Axel Jan 18 '11 at 07:24
  • I am glad that my suggestion was helpful. Hope you fix all the leaks! – petershine Jan 20 '11 at 15:32
1

A starting point would be to re-read about properties because you don't need to do `[item.place release]' anywhere. So you can remove them. The dynamic code created by the runtime to enable that property automatically handles releasing anything previously assigned to it.

Also, the [NSString stringWithFormat:... creates an autorelease object (not sure if you knew that :-) which means that if you where manually managing the memory for a variable (not a property) then you would have to retain/release it. But because you are using properties, you don't.

I cannot see why instruments is finding a memory leak. Perhaps some code higher up is to do with it. For example if you went item.place = [NSString alloc] initWith...]; then I think you would need it.

The crash I would suspect to be because of the releases causing retain counts to be zero and triggering exec bad access errors.

Hope that helps.

drekka
  • 20,957
  • 14
  • 79
  • 135
  • Yes, I should have been a bit more verbose in my problem description. Added more information. – Axel Jan 16 '11 at 11:55
  • Good. But I think my comments still apply. It would appear that you are "over managing". Start by removing all the releases. The managed object context handles the code for the properties and will manage it correctly. – drekka Jan 16 '11 at 11:57
  • ‘But because you are using properties, you don't.’ → well, not always. If it’s an assign property (and assign is a default property attribute), then he’d still need to manage memory, even though it’s not a good idea. Also, it could be the case that he has a custom setter. –  Jan 16 '11 at 11:58
  • @Bavarious - true. I forgot to mention I was assuming that the properties are set with the retain attribute. Just checked one of my projects and the classes as generated by core data are set with retain on the properties. So hopefully my comments are correct. :-) – drekka Jan 16 '11 at 12:02
  • I don't have custom setters. I use @dynamic to generate. But if all the comments are correct, where is the leak? Instruments points me at the place above. – Axel Jan 16 '11 at 12:07
  • Ahhh... Ok. Core data. A useful clue. – bbum Jan 16 '11 at 17:32
  • So - I removed that release-stuff and fixed various other potential problems in other classes, but still, I have that memory leak. Is it possible that instruments assigns leaks to incorrect places? – Axel Jan 17 '11 at 14:43