Memory leak for NSDictionary loaded by plist file

Question

I have a memory leak problem that just can not understand! Watch this initialization method:

- (id)initWithNomeCompositore:(NSString *)nomeCompositore nomeOpera:(NSString *)nomeOpera {

if (self = [super init]) {

    NSString *pathOpere = [[NSBundle mainBundle] pathForResource:kNomeFilePlistOpere ofType:kTipoFilePlist];
    NSDictionary *dicOpera = [NSDictionary dictionaryWithDictionary:
                                [[[NSDictionary dictionaryWithContentsOfFile:pathOpere]
                                objectForKey:nomeCompositore]
                                objectForKey:nomeOpera]];

    self.nomeCompleto = [[NSString alloc] initWithString:nomeOpera];
    self.compositore = [[NSString alloc] initWithString:nomeCompositore];
    self.tipologia = [[NSString alloc] initWithString:[dicOpera objectForKey:kKeyTipologia]];
}

return self;}

Then this little variation (note self.tipologia):

- (id)initWithNomeCompositore:(NSString *)nomeCompositore nomeOpera:(NSString *)nomeOpera {

if (self = [super init]) {

    NSString *pathOpere = [[NSBundle mainBundle] pathForResource:kNomeFilePlistOpere ofType:kTipoFilePlist];
    NSDictionary *dicOpera = [NSDictionary dictionaryWithDictionary:
                                [[[NSDictionary dictionaryWithContentsOfFile:pathOpere]
                                objectForKey:nomeCompositore]
                                objectForKey:nomeOpera]];

    self.nomeCompleto = [[NSString alloc] initWithString:nomeOpera];
    self.compositore = [[NSString alloc] initWithString:nomeCompositore];
    self.tipologia = [[NSString alloc] initWithString:@"Test"];
}

return self;}

In the first variant is generated a memory leak, the second is not! And I just can not understand why! The memory leak is evidenced by Instruments, highlighted the line:

[NSDictionary dictionaryWithContentsOfFile:pathOpere]

This is the dealloc method:

- (void)dealloc {
[tipologia release];
[compositore release];
[nomeCompleto release];
[super dealloc];}

Answer 1

Try

nomeCompleto = [[NSString alloc] initWithString:nomeOpera];
compositore = [[NSString alloc] initWithString:nomeCompositore];
tipologia = [[NSString alloc] initWithString:[dicOpera objectForKey:kKeyTipologia]];

or

self.nomeCompleto = nomeOpera;
self.compositore = nomeCompositore;
self.tipologia = [dicOpera objectForKey:kKeyTipologia];

instead of self.xxx = [[yyy alloc] init...].

---

In the original code, the RHS of the assignment returns an object of retain count +1, and if you make the @property having (retain) or (copy), the final retain count would be +2. Therefore, even if you release these in -dealloc, the net retain count is +1, causing a memory leak.

---

BTW, there's no point calling +dictionaryWithDictionary:. Just use

NSDictionary* dicOpera = [[[NSDictionary dictionaryWithContentsOfFile:pathOpere]
                            objectForKey:nomeCompositore]
                            objectForKey:nomeOpera];

Answer 2

Remember that alloc returns an object that you own.

If you declared your three string properties as retain, assigning those objects to your properties means you now own each one twice—once because you allocked it, and again because you assigned it to your property. The objects remain alive because nothing releases their second ownerships.

If you declared the properties as copy (which is the correct way to declare an NSString property), assigning the object there stores a copy as the value of the property. You do nothing further with the original objects, which remain alive because nothing releases them.

Either way, that is your leak.

The property should be declared as copy; if it already is, don't try to fix the leak by changing that.

You should not use property access here. Remember that assigning to a property is a set<PropertyName>: message, and that your object is not fully initialized yet. Sending a message to an incompletely-initialized or incompletely-deallocated object is asking for trouble, particularly when subclasses are involved, since they may override the accessor methods in ways the superclass doesn't expect.

So, in init only, assign directly to the instance variables. In dealloc only, send release messages directly to the objects in the instance variables. Everywhere else, use property accesses.

You also should not use alloc and initWithString: here. It'll work, but the convention is to send copy messages to the objects you already have, the same as the properties would do. Send copy messages to your input string objects, then assign the copies to your instance variables.

When you do use property accesses, use the convenience constructors (stringWith…:, for example), as these return objects that you do not own. When you assign these objects to your copy-declared properties, you will actually be storing copies that you do own.

The other way would be to use alloc and initWithWhatever:, then immediately autorelease that object before assigning it to the property; this way creates an object that you own, then immediately gives up ownership before assigning it to the property.