Newbie IPhone developer using Instruments panel: Where is my leak?

Question

Update

---

I have update the planets.m/h files to reflect the suggestions made by the commentors (thank you bbum and DarkDust. my factory methods now all call autorelease and look like this:

- (Planet *) initWithName: (NSString *)name;
+ (Planet *) planetWithNameAndMoons:(NSString *)name moons:(id)firstObj, ... NS_REQUIRES_NIL_TERMINATION;
+ (Planet *) planetWithName:(NSString *)name;
+ (Planet *) planetWithResultSet:(FMResultSet *)resultSet;

and now my init now looks like this

- (id)init {
    self = [super init];
    self.Id = 0;
    self.SupportsLife = NO;
    self.Moons =  [NSMutableArray array];   
    return self;
}

I am calling planetWithName in my 2nd controller off the root. This is how I am using it

- (Planet *)newPlanet
{
    int i = 0;
    Planet *planet = nil;
    NSMutableDictionary *criteria = [NSMutableDictionary new];

    do {
        i += 1;
        [criteria setValue: [NSString stringWithFormat:@"Planet %d", i] forKey: @"Name"];
        planet = [[PlanetRepo SharedRepo] Find: criteria];
    } while (planet != nil);    // if a planet is found with that name, keep looking...

    planet = [Planet planetWithName: (NSString *)[criteria valueForKey:@"Name"]];
    [planet retain];

    [criteria release];
    criteria = nil;

    return planet;
}

And here is my PlanetRepo class (note, it is wrapping around FMDB) PlanetRepo.h

#import <Foundation/Foundation.h>
#import "Planet.h"

@interface PlanetRepo : NSObject { 
    FMDatabase * _repoSource;
}

+ (PlanetRepo *) SharedRepo;

- (Planet *)Find:(NSDictionary *)criteria;
- (NSMutableArray *)GetAll;
- (Planet *)GetById:(id ) Id;
- (BOOL) Save:(Planet *) planet;
- (BOOL) Delete: (Planet *) planet;

- (BOOL) Open:(NSString *)repoSource;
- (void) Close;

@end

@interface PlanetRepo (Private)
- (NSError *) CreateDatabaseIfNeeded:(NSString *)databasePath;
@end

Planet.m

#import "FMDatabase.h"
#import "PlanetRepo.h"
#import "Planet.h"

@implementation PlanetRepo

+ (PlanetRepo *)SharedRepo 
{   
    static PlanetRepo *_sharedRepo;
    @synchronized(self) 
    {
        if (!_sharedRepo) 
            _sharedRepo = [[super alloc] init]; 
    }
    return _sharedRepo;
}

+ (id) alloc { 
    return [self SharedRepo]; 
}

- (BOOL) Open: (NSString *)repoSource 
{   
    NSArray *paths = NSSearchPathForDirectoriesInDomains( NSDocumentDirectory, NSUserDomainMask, YES);
    NSString *documentsDirectory = [paths objectAtIndex:0];
    NSString *databasePath = [documentsDirectory stringByAppendingPathComponent: repoSource];
    NSError *error = [self CreateDatabaseIfNeeded: databasePath];

    BOOL result = (error == nil);
    if(result) 
    {
        _repoSource = (FMDatabase *)[FMDatabase databaseWithPath:databasePath];
        [_repoSource retain];
        result = [_repoSource open];
        if (result) 
            NSLog(@"Planet Repo open successful!");
        else 
        {
            NSLog(@"Could not open Planet Repo.");
            [self Close];
        }
    }       
    else 
        NSLog(@"Could not copy db.", [error localizedDescription]);
    return result;
}

- (void) Close 
{
    [_repoSource release];
    _repoSource = nil;
}

- (void) dealloc 
{
    [self Close];
    [super dealloc];
}

- (Planet *)Find:(NSDictionary *)criteria
{
    if (criteria == nil) return nil;

    const NSUInteger criteriaCount = [criteria count];
    if (criteriaCount == 0) return nil;

    NSMutableString *temp = [NSMutableString stringWithString: @"SELECT Id, Name, SupportsLife FROM planet WHERE"];
    NSMutableArray *values = [[NSMutableArray alloc] initWithCapacity:criteriaCount];
    int i = 0;
    for (id key in criteria) 
    {
        [values addObject: [criteria objectForKey:key]];
        NSLog(@"key: %@, value: %@", key, [values objectAtIndex:i] );

        [temp appendString: [NSString stringWithFormat:@" %@=?", key ] ];
        if ( i < (criteriaCount - 1) ) 
            [temp appendString: @","];
        i++;
    }

    NSString *sql = [NSString stringWithString:temp];
    NSLog(@"sql: [%@]", sql);
    FMResultSet *resultSet = [_repoSource executeQuery:sql withArgumentsInArray: values ];

    Planet *planet = nil;
    if ( [resultSet hasAnotherRow] )
        planet = [Planet planetWithResultSet: resultSet];

    [values release];
    [resultSet close];

    return planet;
}

-(NSMutableArray *)GetAll
{
    NSMutableArray* resultsArray = [[NSMutableArray new] autorelease];
    FMResultSet *resultSet = [_repoSource executeQuery:@"SELECT Id, Name, SupportsLife FROM planet ORDER BY Name ASC"];
    while ([resultSet next]) 
        [resultsArray addObject:  [Planet planetWithResultSet:resultSet] ];
    [resultSet close];

    return resultsArray;
}

- (Planet *) GetById:(id) Id
{
    if( ![_repoSource open]) return NULL;

    Planet *planet = NULL;
    FMResultSet *resultSet = [_repoSource executeQuery:@"SELECT Id, Name, SupportsLife FROM planet WHERE Id=?" withArgumentsInArray:Id];
    if ([resultSet next])
        planet = [Planet planetWithResultSet:resultSet];
    [resultSet close];

    return planet;
}

- (BOOL) Save:(Planet *) planet
{
    if( ![_repoSource open]) return NO;

    [_repoSource beginTransaction];
    BOOL result = NO;
    if (planet.Id == 0) 
    {
        result = [_repoSource executeUpdate: @"INSERT INTO \"planet\" (Name,SupportsLife) values (?,?);", planet.Name, planet.SupportsLife];
        planet.Id = [_repoSource lastInsertRowId];
    }
    else 
        result = [_repoSource executeUpdate: @"UPDATE \"planet\" SET Name=?, SupportsLife=? WHERE Id=?", 
                  planet.Name, 
                  [NSNumber numberWithBool:planet.SupportsLife], 
                  [NSNumber numberWithInt:planet.Id]];


    if (result == YES)
        [_repoSource commit];
    else {
        [_repoSource rollback];
        NSLog( @"%@", [_repoSource lastErrorMessage] );
    }
    return result;  
}

- (BOOL) Delete: (Planet *) planet
{
    if( ![_repoSource open]) return NO;

    [_repoSource beginTransaction];
    BOOL result = [_repoSource executeUpdate: @"DELETE FROM \"planet\" WHERE Id=?", [NSNumber numberWithInt:planet.Id]];

    if (result == YES)
        [_repoSource commit];
    else {
        [_repoSource rollback];
        NSLog( @"%@", [_repoSource lastErrorMessage] );
    }
    return result;  
}

- (NSError *) CreateDatabaseIfNeeded:(NSString *)databasePath 
{   
    NSFileManager *fileManager = [NSFileManager defaultManager];
    BOOL doesDBExist = [fileManager fileExistsAtPath: databasePath];

    if (doesDBExist == NO )
    {
        NSString* dbFileName = [databasePath lastPathComponent];

        //the database does not exist, so we will copy it to the users document directory...
        NSString *sourceDbPath = [[[NSBundle mainBundle] resourcePath] stringByAppendingPathComponent:dbFileName];

        NSError *error;
        if (![fileManager copyItemAtPath:sourceDbPath toPath:databasePath error:&error]) 
        {
            NSLog(@"Could not copy db.", [error localizedDescription]);
            return [error autorelease];
        }
    }
    return nil;
}

@end

Original

---

I have been working on picking up objective c lately. Its a nice language but as a .Net guy this memory management stuff is killing me! :)

I have a sample class, called Planet, and when I run through instrument panel I get the following:

http://i.imgur.com/QYdxB.jpg (image)

Leaked Object |  Address  |   Size   | Responsible Library | Responsible Frame
--------------+-----------+----------+---------------------+-------------------
   Planet     | 0x7136380 | 32 Bytes | NavTest             | +[Planet newWithName:]
   Planet     | 0x712da20 | 32 Bytes | NavTest             | -[Planet init]
   Planet     | 0x5917a20 | 32 Bytes | Foundation          | -[NSPlaceholderString

I think this means I have a problem when I am creating an array in the [Planet init]?

I am listing my class file here as well. I would very much appreciate any help on this.

Planet.h

#import <Foundation/Foundation.h>
#import "FMDatabase.h"

@class Planet;

@interface Planet : NSObject { }

@property (nonatomic, assign) int Id;
@property (nonatomic, copy) NSString *Name;
@property (nonatomic, retain) NSMutableArray *Moons;
@property (nonatomic, assign) BOOL SupportsLife;

- (Planet *)initWithName: (NSString *)name;

+ (Planet *) newWithNameAndMoons:(NSString *)name moons:(id)firstObj, ... NS_REQUIRES_NIL_TERMINATION;
+ (Planet *) newWithName:(NSString *)name;
+ (Planet *) readPlanetFrom:(FMResultSet *)resultSet;

@end

Planet.m

#import "Planet.h"
#import "FMDatabase.h"

@implementation Planet

@synthesize Id;
@synthesize Name; 
@synthesize Moons; 
@synthesize SupportsLife; 

- (NSString *) description {
 return [NSString stringWithFormat:@"ID: %d\nName: %@", Id ,Name];
}

- (void) dealloc {
 [Moons release];
 [Name release];

 Moons = nil;
 Name = nil;
 [super dealloc];
}

- (id)initWithName:(NSString *)aName {
 self = [self init];
 if (self) self.Name = aName;
 return self;
}

- (id)init {
 self = [super init];
 self.Id = 0;
 self.SupportsLife = NO;
 NSMutableArray *tmp = [NSMutableArray new]; 
 self.Moons = tmp;
 [tmp release];
 tmp = nil;
 return self;
}

+ (Planet *) newWithName:(NSString *)name {
 return [[self alloc] initWithName: name];
}

+ (Planet *)newWithNameAndMoons:(NSString *)name moons:(id)firstObj, ...   {
 Planet *planet = [self newWithName: name];

 va_list args;
    va_start(args, firstObj);
    for (id arg = firstObj; arg != nil; arg = va_arg(args, id))
 {
        [planet.Moons addObject: arg];
  [arg release];
 }
    va_end(args);
 return planet;
}

+ (Planet *) readPlanetFrom:(FMResultSet *)resultSet
{
 Planet *planet = [[self alloc] init]; 
 planet.Id = [resultSet intForColumn:@"Id"];
 planet.Name = [resultSet stringForColumn:@"Name"];
 planet.SupportsLife = [resultSet boolForColumn: @"SupportsLife"];
 return [planet autorelease];
}
@end

Answer 1

Some issues:

• instance variables and properties should start with lower case letters (to disambiguate from class names)

• there is no need -- but no harm -- in assigning tmp = nil; in your init method (which needs the if(self) {...} test, btw).

• you could avoid the use of a tmp variable by saying self.moons = [NSMutableArray array];

This is a likely source of leaks is the following method:

+ (Planet *) newWithName:(NSString *)name {
 return [[self alloc] initWithName: name];
}

That really should be something like:

+ (id) planetWithName: (NSString *) aName {
    return [[[self alloc] initWithName: name] autorelease];
}

Also, this:

+ (Planet *) readPlanetFrom:(FMResultSet *)resultSet

Should probably be this:

+ (id) planetWithResultSet: (FMResultSet *) aResultSet

---

Without seeing more code, it is hard to say exactly where the leak is coming from. Run the code under the Allocations instrument with retain/release tracking turned on. Pick one of the leaking Planets and look at all the retain/release events. There will be an extra retain.

Answer 2

Try as I might, I can't spot a problem here. I only have a very wild guess: do you have a separate thread where the planet is allocated ? E. g. through performSelectorInBackground: ? If so, you first need to set up a NSAutoreleasePool and drain it at the end of the method. If that really is the problem you would see messages like *** _NSAutoreleaseNoPool(): Object 0x1234560 of class Foo autoreleased with no pool in place - just leaking

Still, I have a comment: Your use of new is quite uncommon. new is a synonym for [[SomeClass alloc] init]. However, its use is very uncommon in Objective-C, almost deprecated.

So instead of

tmp = [NSMutableArray new];
self.Moons = tmp;
[tmp release];

It's more common to write:

self.Moons = [[[NSMutableArray alloc] init] autorelease];

or better yet:

self.Moons = [NSMutableArray array];