Is this the definitive ref counted Objective C singleton implementation??

Question

Here is what I have concocted, after poring over the singleton literature. Have I forgotten anything?

@implementation MySingleton

static MySingleton *mySharedInstance = nil;

//called by atexit on exit, to ensure all resources are freed properly (not just memory)  
static void singleton_remover()
{
    //free resources here
}

+ (MySingleton*) sharedInstance{
    return mySharedInstance;
}

+ (void)initialize {
    if (self == [MySingleton class]) {
        mySharedInstance = [[super allocWithZone:NULL] init];
    atexit( singleton_remover );    
    }
}

+ (id)allocWithZone:(NSZone *)zone
{
    return [self sharedInstance];   
}

- (id)copyWithZone:(NSZone *)zone
{
    return self;    
}

- (id)retain
{
    return self;    
}

- (NSUInteger)retainCount
{
    return NSUIntegerMax;  //denotes an object that cannot be released  
}

- (void)release
{
    //do nothing    
}

- (id)autorelease
{
    return self;    
}

Answer 1

which avoids the synchronize lock most of the time

If you want your software to be reliable, avoid constructs that work "most of the time"

http://developer.apple.com/mac/library/DOCUMENTATION/Cocoa/Conceptual/Multithreading/ThreadSafety/ThreadSafety.html

Table 4.1. Double-checked lock

A double-checked lock is an attempt to reduce the overhead of taking a lock by testing the locking criteria prior to taking the lock. Because double-checked locks are potentially unsafe, the system does not provide explicit support for them and their use is discouraged.

Answer 2

A few suggestions (more for Mac Cocoa than iPhone, but it may be useful to other people searching the objective-c tag):

• Don't bother with -allocWithZone:NULL, just plain -alloc will do fine.
• Consider using dispatch_once() or pthread_once() where available
• The atexit usage is clever but may not be compatible with fast app termination (not sure if this applies on the iPhone), since that effectively kill -9s the app

One additional fun pattern:

+ (Foo *)sharedFoo {
    static Foo *sharedInstance = NULL;
    if (!sharedInstance) {
        Foo *temp = [[Foo alloc] init]; //NOTE: This MUST NOT have side effects for it to be threadsafe
        if (!OSAtomicCompareAndSwapPtrBarrier(NULL, temp, &sharedInstance)) {
            [temp release];
        }
    }
    return sharedInstance;
}

Answer 3

The singleton_remover function will not do anything, because you've overridden release to do nothing. Your allocWithZone: method also performs a similar no-op when it sends retain to the shared instance (and completely disregards allocation in the specified zone). Perhaps you should have a flag that toggles whether your shared instance is invincible (i.e. unreleasable) or not.

Either way, the OS will clean up all the memory anyway. The documentation states that it is faster for the OS to just to reclaim all the memory at once than for your application to slowly give it back piece by piece.

If your shared instance is managing resources that always need to be cleaned up when your application terminates, you should have it register to receive the UIApplicationWillTerminateNotification, and perform the cleanup in there.

[[NSNotificationCenter defaultCenter] addObserver:self
                                         selector:@selector(performCleanup:)
                                             name:UIApplicationWillTerminateNotification
                                           object:nil];

Answer 4

EDIT

I am including this at top, below you can see my historical original questions and implementation. However I think I found the optimal way to provide a sharedInstance method with no locking overhead, I would love to hear potential concerns about this:

// Volatile to make sure we are not foiled by CPU caches
static volatile ALBackendRequestManager *sharedInstance;

// There's no need to call this directly, as method swizzling in sharedInstance
// means this will get called after the singleton is initialized.
+ (MySingleton *)simpleSharedInstance
{
    return (MySingleton *)sharedInstance;
}

+ (MySingleton*)sharedInstance
{
    @synchronized(self)
    {
        if (sharedInstance == nil)
        {
            sharedInstance = [[MySingleton alloc] init];
            // Replace expensive thread-safe method 
                    // with the simpler one that just returns the allocated instance.
            SEL orig = @selector(sharedInstance);
            SEL new = @selector(simpleSharedInstance);
            Method origMethod = class_getClassMethod(self, orig);
            Method newMethod = class_getClassMethod(self, new);
            method_exchangeImplementations(origMethod, newMethod);
        }
    }
    return (MySingleton *)sharedInstance;
}

---

And the historical discussion around initialize:

I see now the original code was actually rather like mine (below), except for having the check for the instance outside the lock.

Although the new + (void) initialize method is interesting, I am not sure I like this better. It seems like now to get a singleton instance you must now always call:

MySingleton instance = [[MySingleton alloc] init];

Is that not correct? That feels odd, and is it more efficient if the call to initialize is already locked for you? The double-lock method seems to work OK for this use case, while also avoiding locking (at the potential cost of a double allocation I think since more than one thread could fall through the if).

The other thing that seems odd if, if the initialize method were really preferred why do we not see it elsewhere? Objective-C has been around a LONG time and I am wary of fundamental mechanisms that differ from just about all published examples.

My code I currently use (which mirrors that I have seen elsewhere, including this answer):

+ (MySingleton *)sharedInstance
{
    @synchronized(self)
    {
        if (sharedInstance == nil)
            sharedInstance = [[MySingleton alloc] init];
    }
    return sharedInstance;
}

+ (id)allocWithZone:(NSZone *)zone 
{
    @synchronized(self) 
    {
        if (sharedInstance == nil) 
        {
            sharedInstance = [super allocWithZone:zone];
            return sharedInstance;  // assignment and return on first allocation
        }
    }
    return nil; // on subsequent allocation attempts return nil
}

Answer 5

Your implementation is thread-safe and appears to cover all the bases (+initialize is sent thread-safe by the runtime)

edit: A lot of code will be unsafe to call during an atexit function. Registering for UIApplicationWillTerminateNotification on the main thread is safer.

edit2: I have distilled and refined the pattern I use into a macro. -init is called once the first time +sharedInstance is called and -dealloc will be called as the application terminates.

#define IMPLEMENT_UIAPP_SINGLETON(class_name) \
static class_name *shared ## class_name; \
+ (void)cleanupFromTerminate \
{ \
    class_name *temp = shared ## class_name; \
    shared ## class_name = nil; \
    [temp dealloc]; \
} \
+ (void)registerForCleanup \
{ \
    [[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(cleanupFromTerminate) name:UIApplicationWillTerminateNotification object:nil]; \
} \
+ (void)initialize { \
    if (self == [class_name class]) { \
        NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init]; \
        if ([NSThread isMainThread]) \
            [[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(cleanupFromTerminate) name:UIApplicationWillTerminateNotification object:nil]; \
        else \
            [self performSelectorOnMainThread:@selector(registerForCleanup) withObject:nil waitUntilDone:NO]; \
        shared ## class_name = [[super allocWithZone:NULL] init]; \
        [pool drain]; \
    } \
} \
+ (class_name *)sharedInstance \
{ \
    return shared ## class_name; \
} \
+ (id)allocWithZone:(NSZone *)zone \
{ \
    return shared ## class_name; \
} \
- (id)copyWithZone:(NSZone *)zone \
{ \
    return self; \
} \
- (id)retain \
{ \
    return self; \
} \
- (NSUInteger)retainCount \
{ \
    return NSUIntegerMax; \
} \
- (void)release \
{ \
} \
- (id)autorelease \
{ \
    return self; \
}