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

Blastoff

Please don't edit your question in a way that makes the existing answers invalid or no longer relevant. – Surf

Why another Objective-C singleton thread? See stackoverflow.com/questions/145154 for examples. – Biebel

Wouldn't you just use Gallagher's .. projectswithlove.com/projects/SynthesizeSingleton.h.zip from cocoawithlove.com/2008/11/… ...? – Brakeman

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.

Pappas

I think that he was thinking "saves CPU and synchronization most of the time". However, with modern CPUs and optimizers it may very well be "does the correct thing most of the time". But I'll admit that the Windows remark was unnecessary. – Jennet

But double-checked locking is unreliable, or am I wrong? edit: I am right. developer.apple.com/mac/library/DOCUMENTATION/Cocoa/Conceptual/… – Pappas

In the general case it can be unreliable (if the state of the lock changes after you test it), but it's perfectly valid in the Singleton case (or in any other case where once the criteria is set, it is never unset). – Collegiate

thanks, folks. I have opted to use the class method initialize, and avoid locking altogether – Blastoff

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

Ulmaceous

- using alloc will call my allocWithZone override, which will get nil on sharedInstance, so this would be a problem – Blastoff

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

Surf

thanks. What is wrong with releasing resources in the singleton_remover method? – Blastoff

Responding to the NS/UIApplicationWillTerminateNotification is the documented way to gracefully handle resource management during application termination. – Surf

It's only safe to register for NS/UIApplicationWillTerminateNotification in the general case on the main thread only--be sure to take that into consideration – Modeling

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
}

Machinate

Interesting. My implementation avoids locks entirely, so better performance, and no deadlock potential. Also, you should be synchronizing on [MySingleton class], since self has not been initialized yet. – Blastoff

@Jacko, that is not true, self will be initialized (i.e. with +initialize) before any messages are sent to it, whether they invoke class or instance methods. – Surf

In a class level method, "self" is equivalent to [MyClass class] - That is to say, BOOL isSelf = self == [MySingleton class]; will be set to YES in the sharedInstance method above. Also, as the code is written there are no possibility of deadlocks because of the call sequence... however I agree the "initialize" method is probably better. How do you get to your sharedInstance variable from outside the class? I did not see that method. – Machinate

One other question - since the docs say initialize is called in a thread-safe manner, are you really avoiding locks? – Machinate

@Surf thanks for the pointer on self == [MySingleton class]. – Blastoff

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; \
}

Modeling

Apple said not to use methods with names beginning with underscores. The underscore prefix is reserved and used for Apple's private methods. – Surf

Very good point (as per devworld.apple.com/iphone/library/documentation/Cocoa/…); edited to reflect – Modeling

why do you need an autorelease pool in the initialize method? – Blastoff

@Blastoff When the singleton is initialized from a background thread, it's not always the case that autoreleased objects will be cleaned up soon. Usually this isn't the case, but for the sake of completeness it's included – Modeling