Why should we separate alloc and init calls to avoid deadlocks in Objective-C?

Question

When reading about thread-safe singletons I found Thread safe instantiation of a singleton here on SO, and in the accepted answer this code:

    sharedInstance = [MyClass alloc];
    sharedInstance = [sharedInstance init];

Why should we separate alloc and init methods? The author of the answer wrote:

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.

Can someone please explain to me in detail what the benefits of this separation are? I couldn't understand what the author meant at all. Do I really need to separate alloc and init methods calls when I create a singleton, even if I do it in dispatch_once() which is thread safe??

Answer 1

@bbum's post has been updated to mention that this solution does not solve the problem being described. Regardless of whether you separate +alloc and -init or not, this problem still exists.

The reasoning is in the edit to his post, but to expand on that, dispatch_once() is not reentrant. In this case, this means that calling dispatch_once() inside a dispatch_once() block (ie. recursively) will result in a deadlock.

So for example, if you have the following code for +sharedInstance:

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

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

    return sharedInstance;
}

..and MyClass's -init method directly or indirectly also calls through to its own +sharedInstance class method (e.g. maybe some other object that MyClass -init allocates calls through to MyClass's +sharedInstance), that would mean that you are attempting to call dispatch_once from inside itself.

Since dispatch_once is thread safe, synchronous, and designed such that it executes exactly once, you can not invoke dispatch_once again before the block inside has finished executing once. Doing so will result in a deadlock, because the second call of dispatch_once will be waiting for the first call (already in the middle of execution) to complete, while the first call is waiting on the second (recursive) call to dispatch_once to go through. They are waiting on each other, hence there's a deadlock.

If you want a solution that provides reentrancy, you would need to use something like NSRecursiveLock which is considerably more expensive than dispatch_once, which doesn't use a locking mechanism.

EDIT: Reasoning for the split of +alloc/-init in @bbum's original answer as requested:

The original code @bbum posted before editing it looked like this:

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

    if (sharedInstance) return sharedInstance;

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

    return sharedInstance;
}

Note this line: if (sharedInstance) return sharedInstance;

The idea here is that assigning a non-nil value to sharedInstance before calling -init would result in the existing value of sharedInstance (returned from +alloc) being returned before hitting the dispatch_once() call (and avoiding the deadlock) in the case that the -init call results in a recursive call to +sharedInstance as discussed earlier in my answer.

However, this is a brittle fix because the if statement there is not thread-safe.