CODEWITHSUNDEEP
objective-ccocoa-touchiosmemory-managementmemory-leaks
Dessie
Dessie

Reputation: 131

Can I use autorelease to fix this memory leak?

I have a memory leak in my iPhone app. I added Google AdMob to my app using sample code downloaded from Google. However, I was having a hard time getting it into testing mode so I added an extra variable as follows:

GADRequest *r = [[GADRequest alloc] init];
r.testing = YES;
[bannerView_ loadRequest:r];

I found the memory leak using Instruments. Instruments does not lead me to this line of code, it just dumps me in the main.m file. However, when I comment out the code relating to AdMob the leak goes away and I know enough to see that I haven't taken care of releasing this new variable. I just don't know exactly how I should set about releasing it. The variable r is not addressed in the header file so this is all the code that deals with it.

I tried adding:

- (void)dealloc {
[r release];
....
}

but that caused a build error saying "'r' undeclared". That's weird because it looks to me like I'm declaring r in the first quoted line above but I guess that's wrong. Any help would be much appreciated. I really have tried to educate myself about memory leaks but I still find them very confusing.

Upvotes: 0

Views: 406

Answers (3)

Dessie
Dessie

Reputation: 131

OP here. Thanks for all the detailed and thoughtful responses. This has definitely helped get a better handle on memory management. I did exactly as recommended, adding [r release]; right below the posted code. However, I still have a leak. I have isolated it to a single line of code. The following leaks:

GADRequest *r = [[GADRequest alloc] init];
r.testing = YES;
[bannerView_ loadRequest:r];
[r release];

The following does not leak:

GADRequest *r = [[GADRequest alloc] init];
r.testing = YES;
// [bannerView_ loadRequest:r];
[r release];

I figure I'm changing the retain count on bannerView with the loadRequest but I don't know how to fix it. I tried a [bannerView_ release] immediately after the [r release]; line (i.e. releasing locally) but that didn't work. I didn't expect it to because bannerView_ is declared elsewhere. I tried [bannerView_ release]; in the dealloc method but that didn't work. I also tried [bannerView_ autorelease]; locally. The wise heads at Google put [bannerView_ release]; in the ViewDidUnload method.

It's possible that Instruments is just messing with my head. The leak appears after about 10 seconds but the app performs well and the amount of memory leaked doesn't seem to be spirally upwards as the app continues to run. Is there such a thing as a benign memory leak?

Thanks again for your help,

Dessie.

Upvotes: 0

Regexident
Regexident

Reputation: 29552

If your r is locally declared (as it seems, judging from your snippet), then it cannot be accessed from outside its scope (here: the method it was declared in). You either need to make it accessible within your class instance by declaring it an ivar.

Declaring it an ivar would look like this:

@interface YourClass : SuperClass {
    GADRequest *request;
}

//...

@end

You then change your code to this:

request = [[GADRequest alloc] init];
request.testing = YES;
[bannerView_ loadRequest:request];

Also don't forget to release it in dealloc:

- (void)dealloc {
    [request release];
    //...
}

However this is not what you want in this situation (I've just included it to clarify why you get the warning about r not being declared).

You (most likely) won't need request any second time after your snippet has run, thus storing it in an ivar will only needlessly occupy RAM and add unwantd complexity to your class. Stuff you only need at the immediate time after its creation should be taken care of (released) accordingly, that is within the very same scope.

What you'll actually want to do is simply (auto)release it, properly taking care of it.

Keep in mind though that your loadRequest: will need to take care of retaining r for as long as it needs it. Apple's implementation does this, of course. But you might want to write a similar method on your own one day, so keep this in mind then.

GADRequest *r = [[GADRequest alloc] init];
r.testing = YES;
[bannerView_ loadRequest:r];
[r release]; //or: [r autorelease];

Upvotes: 4

Daniel G. Wilson
Daniel G. Wilson

Reputation: 15055

Just add [r release]; right below the code:

GADRequest *r = [[GADRequest alloc] init];
r.testing = YES;
[bannerView_ loadRequest:r];
[r release];

The variable r is declared only in that section of your code, so that's where it should be released. The point of releasing is to get rid of it as soon as you no longer need it, so the above should work perfectly for you.

Upvotes: 4

Related Questions