Are my subclasses causing memory leaks?

Question

I'm subclassing UINavigationController and UITableview and for some reason, my views are leaking memory, although I've implemented all of the proper methods and release calls. When I use the native class instead of a subclass, everything works just fine, without leaking.

EDIT:

Here's my superclass header:

//
//  MBAbstractViewController.h
//  GabbaiHD
//
//  Created by Moshe Berman on 11/24/10.
//  Copyright 2010 MosheBerman.com. All rights reserved.
//

#import <UIKit/UIKit.h>


@interface MBAbstractViewController : UIViewController {
    IBOutlet UIImageView *backgroundImageView;
    NSString *announcementText;
}

@property (nonatomic, retain) NSString *type;
@property (nonatomic, retain) NSDictionary *options;
@property (nonatomic, retain) NSString *announcementText;

-(void) setAnnouncementText:(NSString *)text;

@end

Superclass implementation:

//
//  MBAbstractViewController.m
//  GabbaiHD
//
//  Created by Moshe Berman on 11/24/10.
//  Copyright 2010 MosheBerman.com. All rights reserved.
//

#import "MBAbstractViewController.h"


@implementation MBAbstractViewController

@synthesize type, options, announcementText;

 // The designated initializer.  Override if you create the controller programmatically and want to perform customization that is not appropriate for viewDidLoad.
/*
- (id)initWithNibName:(NSString *)nibNameOrNil bundle:(NSBundle *)nibBundleOrNil {
    self = [super initWithNibName:nibNameOrNil bundle:nibBundleOrNil];
    if (self) {
        // Custom initialization.
    }
    return self;
}
*/

/*
// Implement loadView to create a view hierarchy programmatically, without using a nib.
- (void)loadView {
}
*/

// Implement viewDidLoad to do additional setup after loading the view, typically from a nib.
- (void)viewDidLoad {
    [super viewDidLoad];
    UIColor *clearColor = [[UIColor alloc] colorWithAlphaComponent:0.0];
    [self.view setBackgroundColor: clearColor];
    [clearColor release];
}


- (BOOL)shouldAutorotateToInterfaceOrientation:(UIInterfaceOrientation)interfaceOrientation {
    if(interfaceOrientation == UIInterfaceOrientationLandscapeLeft ||  interfaceOrientation == UIInterfaceOrientationLandscapeRight){
        return YES;
    }else{
        return NO;
    }
}

- (void)didReceiveMemoryWarning {
    // Releases the view if it doesn't have a superview.
    [super didReceiveMemoryWarning];

    // Release any cached data, images, etc. that aren't in use.
}


- (void)viewDidUnload {
    [super viewDidUnload];
    // Release any retained subviews of the main view.
    // e.g. self.myOutlet = nil;

}


- (void)dealloc {
    [announcementText release];
    [options release];
    [type release];
    [super dealloc];
}


@end

Here's my subclass header:

//
//  MBAnnouncementViewController.h
//  GabbaiHD
//
//  Created by Moshe Berman on 11/24/10.
//  Copyright 2010 MosheBerman.com. All rights reserved.
//

#import <UIKit/UIKit.h>
#import "MBAbstractViewController.h"

@interface MBAnnouncementViewController : MBAbstractViewController {
    IBOutlet UILabel *announcement;
}

- (void) setAnnouncementText:(NSString *)text withSize:(CGFloat)size;

@end

and the subclass implementation:

        //
//  MBAnnouncementViewController.m
//  GabbaiHD
//
//  Created by Moshe Berman on 11/24/10.
//  Copyright 2010 MosheBerman.com. All rights reserved.
//

#import "MBAnnouncementViewController.h"
#import "Constants.h"[

@implementation MBAnnouncementViewController

 // The designated initializer.  Override if you create the controller programmatically and want to perform customization that is not appropriate for viewDidLoad.
/*
- (id)initWithNibName:(NSString *)nibNameOrNil bundle:(NSBundle *)nibBundleOrNil {
    self = [super initWithNibName:nibNameOrNil bundle:nibBundleOrNil];
    if (self) {
        // Custom initialization.
    }
    return self;
}
*/


// Implement viewDidLoad to do additional setup after loading the view, typically from a nib.
- (void)viewDidLoad {
    [super viewDidLoad];

    [announcement setText:announcementText]; 

    UIImage *slideImage = [[UIImage alloc] initWithContentsOfFile:[[NSBundle mainBundle] pathForResource:[[NSString stringWithFormat:@"%@_slide", kTheme]description] ofType:@"png"]];  
    [backgroundImageView setImage:slideImage];
    [slideImage release];   


}



- (BOOL)shouldAutorotateToInterfaceOrientation:(UIInterfaceOrientation)interfaceOrientation {
    // Overriden to allow any orientation.
    return YES;
}


- (void)didReceiveMemoryWarning {
    // Releases the view if it doesn't have a superview.
    [super didReceiveMemoryWarning];

    // Release any cached data, images, etc. that aren't in use.
}

- (void) setAnnouncementText:(NSString *)text withSize:(CGFloat)size{
    UIFont *font = [[UIFont alloc] fontWithSize:size];
    [announcement setFont:font];
    [font release];
    [announcement setText:text];

}

- (void)viewDidUnload {
    [super viewDidUnload];
    // Release any retained subviews of the main view.
    // e.g. self.myOutlet = nil;
}


- (void)dealloc {
    [announcementText release];
    [super dealloc];
}


@end

What could be causing memory leaks in a subclass? Am I missing something? (There is more relevant code here.)

Answer 1

You should not be alloc'ing UIColor in this way:

[[UIColor alloc] colorWithAlphaComponent:0.0];

You've alloc'd the instance without initializing it, then you've called a factory method and lost the reference to the instance you alloc'd.

You'll want to call one of the initializers before calling colorWithAlphaComponent:. I'm not sure what the behaviour will be if you don't do so. Why not just [UIColor clearColor];?

You're making the same mistake with UIFont too:

UIFont *font = [[UIFont alloc] fontWithSize:size];

You should never do this. The only methods you chain onto +alloc should be initializers (which always return whatever was alloc'd). You're leaking here, and also getting some weird unintended behaviour I dare say.

Also, as David Liu says, you appear to be over-releasing announcementText and under-releasing announcement, which will cause both a crash and a leak, depending on how lucky you get.

EDIT | Based on your update that shows your header files, there are further issues (not specifically related to leaks).

@property (nonatomic, retain) NSString *announcementText;

You should never retain NSString*; instead you should copy it.

@property (nonatomic, copy) NSString *announcementText;

This is because NSString* might actually be a mutable string and be changed later. You may use retain if you're specifically wanting this however.

You also do not need to expressly declare -setAnnouncementText: when you have the @property announcementText.

In your subclass setAnnouncementText:withSize: actually does nothing to the announcementText ivar, which is confusing I guess. You'll want to release the announcement ivar and fix the issue with UIColor and UIFont being used incorrectly (which are leaking).

Answer 2

According to the UIViewController documentation

... if your view controller contains outlets (properties or raw variables that contain the IBOutlet keyword), you should use the viewDidUnload method to relinquish ownership of those outlets or any other view-related data that you no longer need.

Try releasing announcement in the viewDidUnload since it's an IBOutlet, Interface Builder retains it.

- (void)viewDidUnload {
    [super viewDidUnload];
    [announcement release];
    announcement = nil;
}

Answer 3

In your subclass, I notice that you have an "announcement" variable that doesn't seem to show up in your superclass.

I'm assuming that your superclass has the following variables:

type
options
announcementText

While your subclass has an additional variable:

announcement

In your dealloc of the subclass, you release announcementText (which is also released in the superclass), not announcement, which is probably the cause of your leakage.

Answer 4

What does 'announcement' represent in your second class? Is it an IBOutlet(I do not see a synthesize for it)? If it is a property or IBOutlet you will want to make sure to release it in your dealloc.