Memory Leaks in Cocoa

Question

I'm working on an iphone application and having some trouble with memory leaks. I've read some docs about garbage collection that it make it sound simple but I must be missing something. I've got a viewController that needs access to an array which may need to repopulated from time to time. Here is a simplified version of what I have:

//AppDelegate.m
- (NSMutableArray *)getMathFacts {
        //Some database stuff
    NSMutableArray * arr = [[NSMutableArray alloc] init];
    while (sqlite3_step(math_fact_statement) == SQLITE_ROW) {
     [arr addObject:[[NSNumber numberWithInt:sqlite3_column_int(math_fact_statement, 0)] autorelease]];
    }
    return arr;
}

//ViewController.h
@interface ReviewViewController : UIViewController  {
    NSMutableArray *reviewArr;
}
@property (retain, nonatomic) NSMutableArray *reviewArr;

//ViewController.m
- (void)viewDidLoad {
    [super viewDidLoad];
    [self loadMathFacts];
}
- (void)loadMathFacts {
    self.reviewArr = [appDelegate getMathFacts];
}
- (void)loadAllMathFacts {
    self.reviewArr = [appDelegate getAllMathFacts];
}
-(IBAction) getAll {
    [reviewArr release];
    [self loadAllMathFacts]
}

GetAllMathFacts is similar to getMathFacts, it just has a different SQL statement. When I run this checking for Leaks it is like a sieve. It seems like something simple, but I feel like I've tried everything and it just moves the leak around.

Any advice would be appreciated. Thanks

Answer 1

The iPhone OS actually doesn't have garbage collection. What you're doing with retain/release is called reference counting.

The solution to your problem is probably to make getMathFacts return an autoreleased object (change return arr; to return [arr autorelease];), because the definition of the property reviewArr is probably something like @property (retain) NSArray *reviewArr;, which means every time you call self.reviewArr = something;, something is retained, which means after you set reviewArr in loadMathFacts and loadAllMathFacts, reviewArr is retained one time too much.

Answer 2

In getMathFacts, you do a

NSMutableArray * arr = [[NSMutableArray alloc] init];

that array is owned by you. It has a retain count of 1. Later when

- (void)loadMathFacts {
    self.reviewArr = [appDelegate getMathFacts];
}

that same array is now retained by reviewArr and the retain count goes to 2.

When you then do a

-(IBAction) getAll {
    [reviewArr release];
    [self loadAllMathFacts]
}

in the first release statement, your array is now getting released once and the retain count goes to 1. In [self loadAllMathFacts]

- (void)loadAllMathFacts {
    self.reviewArr = [appDelegate getAllMathFacts];
}

self.reviewArr will release the the array, before retaining a new array. After this release the retain count is down to 0. I don't see a leak here. Maybe in -getAllMathFacts?

Now, one thing I would change to make things look a little better is this:

- (void)loadMathFacts {
    NSMUtableArray array = [appDelegate getMathFacts];
    self.reviewArr = array;
    [array release];
}
- (void)loadAllMathFacts {
    NSMUtableArray array = [appDelegate getAllMathFacts];
    self.reviewArr = array;
    [array release];

}
-(IBAction) getAll {  // you don't need to release in here
    [self loadAllMathFacts]
}

Plus you should rename your get method calls to use "new" instead of "get" since the convention is that if you return something that is owned by the caller, it should have new or copy in the name of the method.

As another answerer said, you could use autorelease, although I'd rather use autorelease in other situations. But if you were to do it with autorelease this is how I'd do it:

//AppDelegate.m
- (NSMutableArray *)getMathFacts {
        //Some database stuff
    NSMutableArray * arr = [[NSMutableArray alloc] init];
    while (sqlite3_step(math_fact_statement) == SQLITE_ROW) {
        [arr addObject:[[NSNumber numberWithInt:sqlite3_column_int(math_fact_statement, 0)] autorelease]];
    }
    return [arr autorelease];
}

do the same with -getAllMathFacts. You should still change the code to be more like above, except you don't have to release after doing the self.reviewArray, and you don't have to change the name of the methods. What you have to remember is that if even autoreleased objects can leak if you call retain on them and then forget about them. The nice thing about autorelease is that the object is kept around for the duration of a runloop, long enough to hand it to some other object and let them decide whether they want to retain it or let it expire.

I hope I haven't missed anything. Go through my logic, and feel free to poke holes in it or ask questions. I can easily have missed something.

By the way, self.reviewArr when you have:

@property (retain, nonatomic) NSMutableArray *reviewArr;

does the following:

- (void)setReviewArr:(NSMutableArray*)array
{
  NSMutableArray* oldReviewArr = reviewArr;
  reviewArr = [array retain];
  [oldReviewArr release];
}

Answer 3

As of Xcode 3.2 there is support for running the Clang analyzer. To do this choose Build->Build & Analyze. This will run the analyzer which is really a wonderful tool for finding reference counting issues.

For Xcode prior to 3.2, this might help: http://stackoverflow.com/questions/961844/using-clang-static-analyzer-from-within-xcode