Somehow my singleton properties are getting released

Question

I have a singleton object called PoolManager that loads and saves some data in a plist. Throughout my program when something needs to know about my pool, it asks the [PoolManager sharedPoolManager] for it's properties. I have a single view that's responsible for setting these properties and all others just read from it. It was all working fine, and then for no reason I can tell, it started crashing. I set NSZombieEnabled = YES and can see that when I access one of the two NSString properties, they appear to have been released. The debugger message is: *** -[CFString respondsToSelector:]: message sent to deallocated instance 0x5a336d0

I tried going back to a previous snapshot where everything worked and it still does this. I even used TimeMachine to go back to the project from yesterday and it does it too. I'm baffled.

Here is the singleton object code... It's the surface and shape strings that are apparently zombies. Sorry for all the NSLogs

---

//  MyPoolSingleton.h

#import <Foundation/Foundation.h>

#define kFileName @"data.plist"

@interface PoolManager : NSObject {
    float   volume;
    float length;
    float width;
    float depth;    
    NSString *surface;
    NSString *shape;
    BOOL isMetric;
    int fcTarget;
    int cyaTarget;
    int taTarget;
    int chTarget;
    int saltTarget;
}

@property float volume;
@property float length;
@property float width;
@property float depth;
@property (nonatomic, retain) NSString *surface;
@property (nonatomic, retain) NSString *shape;
@property BOOL isMetric;
@property int fcTarget;
@property int cyaTarget;
@property int taTarget;
@property int chTarget;
@property int saltTarget;

+ (PoolManager*)sharedPoolManager;
- (void)retrieveState;
- (void)saveState;
- (NSString*)dataFilePath;

@end

---

//  MyPoolSingleton.m

#import "PoolManager.h"

@implementation PoolManager

@synthesize volume;
@synthesize length;
@synthesize width;
@synthesize depth;
@synthesize surface;
@synthesize shape;
@synthesize isMetric;
@synthesize fcTarget;
@synthesize cyaTarget;
@synthesize taTarget;
@synthesize chTarget;
@synthesize saltTarget;

static PoolManager* _sharedPoolManager = nil;

+ (PoolManager*)sharedPoolManager {
    @synchronized([PoolManager class])
    {
        if (!_sharedPoolManager)
            [[self alloc] init];
        return _sharedPoolManager;
    }
    return nil;
}

+ (id)alloc {
    @synchronized([PoolManager class])
    {
        NSAssert(_sharedPoolManager == nil, @"Attempted to allocate a second instance of a singleton.");
        _sharedPoolManager = [super alloc];
        return _sharedPoolManager;
    }
    return nil;
}

- (id)init {
    self = [super init];
    return self;
}

- (void)retrieveState {
    NSLog(@"--retrieveState");
    NSString *filePath = [self dataFilePath];
    if ([[NSFileManager defaultManager] fileExistsAtPath:filePath]) {
        NSLog(@"    fileExistsAtPath: reading array from plist");
        NSArray *array = [[NSArray alloc] initWithContentsOfFile:filePath];
        volume = [[array objectAtIndex:0] floatValue];
            NSLog(@"      reading array: volume = %1.1f", volume);
        length = [[array objectAtIndex:1] floatValue];
            NSLog(@"      reading array: length = %1.1f", length);
        width = [[array objectAtIndex:2] floatValue];
            NSLog(@"      reading array: width = %1.1f", width);
        depth = [[array objectAtIndex:3] floatValue];
            NSLog(@"      reading array: depth = %1.1f", depth);
        self.surface = [array objectAtIndex:4];
            NSLog(@"      reading array: surface = %@", surface);
        self.shape = [array objectAtIndex:5];
            NSLog(@"      reading array: shape = %@", shape);
        isMetric = [[array objectAtIndex:6] boolValue];
            NSLog(@"      reading array: isMetric = %d", isMetric);
        fcTarget = [[array objectAtIndex:7] intValue];
            NSLog(@"      reading array: fcTarget = %d", fcTarget);
        cyaTarget = [[array objectAtIndex:8] intValue];
            NSLog(@"      reading array: cyaTarget = %d", cyaTarget);
        taTarget = [[array objectAtIndex:9] intValue];
            NSLog(@"      reading array: taTarget = %d", taTarget);
        chTarget = [[array objectAtIndex:10] intValue];
            NSLog(@"      reading array: chTarget = %d", chTarget);
        saltTarget = [[array objectAtIndex:11] intValue];
            NSLog(@"      reading array: saltTarget = %d", saltTarget);
        [array release];
    }
    else {
        NSLog(@"    !fileExistsAtPath: intitializing values to nil/zero");
        volume = 0.0;
        length = 0.0;
        width = 0.0;
        depth = 0.0;
        surface = @"";
        shape = @"";
        isMetric = NO;
        fcTarget = 0.0;
        cyaTarget = 0.0;
        taTarget = 0.0;
        chTarget = 0.0;
        saltTarget = 0.0;
    }
}

- (void)saveState {
    NSLog(@"--saveState");
    NSMutableArray *array = [[NSMutableArray alloc] init];
        NSLog(@"      building array: volume = %1.1f", volume);
    [array addObject:[NSNumber numberWithFloat:volume]];
        NSLog(@"      building array: length = %1.1f", length);
    [array addObject:[NSNumber numberWithFloat:length]];
        NSLog(@"      building array: width = %1.1f", width);
    [array addObject:[NSNumber numberWithFloat:width]];
        NSLog(@"      building array: depth = %1.1f", depth);
    [array addObject:[NSNumber numberWithFloat:depth]];
        NSLog(@"      building array: surface = %@", surface);
    [array addObject:surface];
        NSLog(@"      building array: shape = %@", shape);
    [array addObject:shape];
        NSLog(@"      building array: isMetric = %d", isMetric);
    [array addObject:[NSNumber numberWithBool:isMetric]];
        NSLog(@"      building array: fcTarget = %d", fcTarget);
    [array addObject:[NSNumber numberWithInt:fcTarget]];
        NSLog(@"      building array: cyaTarget = %d", cyaTarget);
    [array addObject:[NSNumber numberWithInt:cyaTarget]];
        NSLog(@"      building array: taTarget = %d", taTarget);
    [array addObject:[NSNumber numberWithInt:taTarget]];
            NSLog(@"      building array: chTarget = %d", chTarget);
    [array addObject:[NSNumber numberWithInt:chTarget]];
        NSLog(@"      building array: saltTarget = %d", saltTarget);
    [array addObject:[NSNumber numberWithInt:saltTarget]];

    [array writeToFile:[self dataFilePath] atomically:YES];
    [array release];
}    

- (NSString*)dataFilePath {
    NSArray *paths = NSSearchPathForDirectoriesInDomains(NSDocumentDirectory, NSUserDomainMask, YES);
    NSString *documentsDirectory = [paths objectAtIndex:0];
    return [documentsDirectory stringByAppendingPathComponent:kFileName];
}

- (void)dealloc {
    [shape release], shape = nil;
    [surface release], surface = nil;
    [super dealloc];
}

@end

Answer 1

objectAtIndex: gives an autoreleased object. You should either retain it, or use the property accessor self.surface = ... and self.shape = ... when setting those.

Answer 2

Personally I prefer the following pattern for singletons:

+ (id) sharedPoolManager
{
    static id sharedPoolManager = nil;
    @synchronized (self) {
        if (sharedPoolManager == nil) {
             sharedPoolManager = [self new];
        }
    }
    return sharedPoolManager;
}

- (id) init
{
    if ((self = [super init]) != nil) {
         // Initialize ... nothing special here ...
    }
    return self;
}

(Note that self in a class method is equivalent to [SomeClass class])

The above is more concise and I prefer to keep any singleton code outside of init and alloc since I can then also create multiple instances if needed. For example in unit tests.

Personally I don't think you have to protect the programmer from creating multiple instances. The contract is clear: sharedPoolManager returns a singleton instance. If that is what you want/need then use that. Otherwise create instances the usual way.