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:

https://i.stack.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

The instrument isn't telling you that your init and newWithName are problems. It's telling you that that is where the leaked object was created. You can expand the responsible frame to see the whole stack at the time the object was created which may help you figure out why it's not being released properly later. — imaginaryboy, Sep 12 '10 at 22:58
Thank you to everyone who commented. I took the suggestions made by DarkDust and bbum, and renamed my factory methods to start with planet. I understand that this is a obj-c style issue, but I am glad to know what is proper use. Also I changed the Moons creation to call self.Moons = [NSMutableArray array]; — NYCChris, Sep 13 '10 at 13:47

bbum · Answer 1 · 2010-09-12T18:19:02.000

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.

Could you explain why you think the leak is `+[Planet newWithName]` ? It does exactly what it's supposed to do, return an instance with retain count 1. Or did I miss something ? Still, you're absolutely right in that `+[Planet planetWithName:]` with autorelease is better style. — DarkDust, Sep 12 '10 at 17:43
Hard to say conclusively without more code, but I would imagine that the caller does something like `self.myPlanet = [Planet newWithName: @"Bob"]` where `myPlanet` is an `@property` declared to `retain`. — bbum, Sep 12 '10 at 18:18

score 0 · Answer 2 · edited May 23 '17 at 10:32

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];

I am not calling performSelectorInBackground: directly. Does the framework do that automatically at some point? I am targetting ios 4 as my target. — NYCChris, Sep 13 '10 at 13:55
No, threads are only created explicitly by you (internally, some classes like NSURLConnection do create their own threads but you'll never notice). — DarkDust, Sep 13 '10 at 18:03

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

2 Answers2