objective c - Thread safe instantiation of a singleton

Question

Which one synchronization method to use to ensure a singleton remains a singleton?

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

or using a mutex?

#import <pthread.h>

static pthread_mutex_t _mutex = PTHREAD_MUTEX_INITIALIZER;

+(Foo*)sharedInstance
{
   pthread_mutex_lock(&_mutex);
   if (nil == _sharedInstance)
   {
      _sharedInstance = [[Foo alloc] init];
      ...
   }
   pthread_mutex_unlock(&_mutex);
   return _sharedInstance;
}

Hmmm.. any comments on this?

Answer 1

Make sure you read the discussion on this question/answer, too. Why should we separate alloc and init calls to avoid deadlocks in Objective-C?

---

To expand on the race condition issue; the real fix is to not have indeterminate initialization within your application. Indeterminate or lazy initialization results in behavior that can easily change due to seemingly innocuous changes -- configuration, "unrelated" code changes, etc...

Better to explicitly initialize subsystems on a known-good point in the program's lifespan. I.e. drop [MyClass sharedInstance]; into your App delegate's applicationDidFinishLaunching: method if you really need that subsystem initialized early in the program (or move it even earlier, if you want to be extra defensive).

Better still to move initialization out of that method entirely. I.e. [MyClass initializeSharedInstance]; where +sharedInstance asserts() if that method isn't called first.

As much as I'm a a fan of convenience, 25 years of ObjC programming has taught me that lazy initialization is a source of more maintenance and refactoring headaches than it is worth.

---

While the race condition described below exists, this code doesn't fix what is described below. It did for a couple of decades when we didn't worry about concurrency in shared instance initializers. Leaving the wrong code for prosperity.

Keep in mind that for both Colin's and Harald's otherwise correct answers, there is a very subtle race condition that could lead you to a world of woe.

Namely, if the -init of the class being allocated happens to call the sharedInstance method, it will do so before the variable is set. In both cases it will lead to a deadlock.

This is the one time that you want to separate the alloc and the init. Cribbing Colin's code because it is the best solution (assuming Mac OS X):

+(MyClass *)sharedInstance
{   
    static MyClass *sharedInstance = nil;
    static dispatch_once_t pred;

    // partial fix for the "new" concurrency issue
    if (sharedInstance) return sharedInstance;
    // partial because it means that +sharedInstance *may* return an un-initialized instance
    // this is from https://stackoverflow.com/questions/20895214/why-should-we-separate-alloc-and-init-calls-to-avoid-deadlocks-in-objective-c/20895427#20895427

    dispatch_once(&pred, ^{
        sharedInstance = [MyClass alloc];
        sharedInstance = [sharedInstance init];
    });

    return sharedInstance;
}

note this only works on Mac OS X; X 10.6+ and iOS 4.0+, in particular. On older operating systems, where blocks are not available, use a lock or one of the various means of doing something once that isn't blocks based.

---

The above pattern does not actually prevent the problem described in the text and will cause a deadlock when it is encountered. The problem is that the dispatch_once() is not re-entrant and, thus, if the init calls sharedInstance, wedge city.