1

I'm having problems with a leak in the init method of a class I have created. To keep it simple, I have the following (simplified) problem:

  • ViewController initialises an instance of
  • ClipData class which initialises an instance of
  • AnimationData class which initialise a string

ViewController:

myClipData = [[ClipData alloc] init];

ClipData:

- (id)init
{
    self = [super init];
    if (self) {
        animData = [[AnimationData alloc] init];  //LEAK HERE
    }

    return self;
}

AnimationData:

- (id)init
{
    self = [super init];
    if (self) {
        name = [NSString string];
    }

    return self;
}

All the objects in the classes are declared as (nonatomic, retain). I'm aware that doing this bumps up the retain count, but how do I initialise the AnimationData without leaking the animData???

Probably a very stupid question, so any help much appreciated.

Thanks,
Duncs

theDuncs
  • 4,649
  • 4
  • 39
  • 63

2 Answers2

2

You are never releasing the animData. You need to add dealloc to your class:

- (void)dealloc {
  [animData release];

  [super dealloc];
}

Similarly, you need to add a similar dealloc to AnimationData.

On a related note, you need to retain and later release the string created in -[AnimationData init], what you are doing right now is essentially a noop, except that it leaves behind a garbled pointer.

Williham Totland
  • 28,471
  • 6
  • 52
  • 68
  • Actually, thanks to ann implementation detail of Cocoa, he is lucky with the `[NSString string]` method since it returns a reference to a constant (empty) string that never goes away. – JeremyP Apr 27 '12 at 10:13
  • @JeremyP: Be that as it may, it's not behavior you should come to rely on. – Williham Totland Apr 27 '12 at 10:18
  • No it's not and I did not mean to imply that you should rely on it. I was just adding the point to explain why theDuncs won't actually see a garbage string as a result of this, using the current implementation of Cocoa. – JeremyP Apr 27 '12 at 10:20
  • Huge school-boy error on my part. I think I still have some unresolved questions in my head about memory allocation, but now I get this part. Thanks WT. You da man! – theDuncs Apr 27 '12 at 11:11
0

When you have an alloc you must also have a release.

You should also reference the properties through self so you access the properties rather than the underlying members.

So you should really do :

ClipData *clip = [[ClipData alloc] init];
self.myClipData = clip;
[clip release];

And

 if (self) {
        AnimationData *data = [[AnimationData alloc] init]; 
        self.animData = data;  
        [data release];
    }

Make sure you also release the properties in the dealloc of the class by setting them to nil.

self.myClipData = nil;
self.animData = nil;
Mongus Pong
  • 11,337
  • 9
  • 44
  • 72
  • Actually, http://stackoverflow.com/questions/192721/why-shouldnt-i-use-objective-c-2-0-accessors-in-init-dealloc – Williham Totland Apr 27 '12 at 10:28
  • Mongus - thanks so much. I'm not sure I understand the benefit of assigning the new ClipData to an object, then releasing the object. Surely I can just call self.myClipData = [[ClipData alloc] init]; ??? I managed to resolve the leak by using the dealloc method, which I'd forgotten to add, but the points go to Williham for getting there first. Thanks so much! – theDuncs Apr 27 '12 at 11:13
  • The alloc will give the object a reference count of 1. Assigning it to the property - which retains the object - will increase the count to 2. So you need a release to set the count back to 1. In your question you are not assigning via the property, so you are avoiding the extra retain. – Mongus Pong Apr 27 '12 at 11:42
  • 1
    I like to see allocs and releases close together to avoid confusion. – Mongus Pong Apr 27 '12 at 11:51