Is this method of initializing singleton thread safe?

Question

this is a scope of code in our project, I think it is not thread-safe, because the scope

if(sharedHelper)
    return sharedHelper;

will cause problem, but I am not sure, could any one help me?

+(id) sharedHelper
{
static MyHelper *sharedHelper = nil;
static dispatch_once_t onceToken;

if(sharedHelper)
    return sharedHelper;

dispatch_once(&onceToken,^{

    sharedHelper = [[self alloc] init];
});

return sharedHelper;
}

You can omit the `if(sharedHelper) return sharedHelper;` business (since `dispatch_once`, as the name suggests, only calls the block once in the lifetime of the app), but regardless, it looks fine to me. What problem do you suspect? — maroux, May 27 '13 at 07:26
Check this SO answer http://stackoverflow.com/a/2202304/2315974 — danypata, May 27 '13 at 07:27
the second may use the sharedHelper while the first thread has not completed the initialization of it. @Mar0ux — haibarahu, May 27 '13 at 07:30
@haibarahu No it won't. `dispatch_once` has synchronized access on `onceToken`, so until the first thread has finished running the block, the second won't proceed. — maroux, May 27 '13 at 07:36
yes, I know this.But the second thread will not access the code of dispatch_once, it access the scope "if(sharedHelper) return sharedHelper;".@Mar0ux — haibarahu, May 27 '13 at 07:41
@haibarahu that's right, that's why you should remove that check. interesting read on why that check doesn't work as you expect it to: http://www.aristeia.com/Papers/DDJ_Jul_Aug_2004_revised.pdf — maroux, May 27 '13 at 07:42
@haibarahu Oh I see I said `regardless` in my original comment, which could have been misleading. Since I can't edit it now, I'll remove it. — maroux, May 27 '13 at 07:44

score 1 · Accepted Answer · edited May 23 '17 at 12:28

You are correct in thinking that if block might cause problems. Here's why (source):

This line

[[MyHelper alloc] init];

is actually made up of three steps:

Allocate memory
Construct object into allocated memory
Point static variable to the allocated memory address

Now, you would expect these things to happen in order from 1-3. But, in certain cases, compiler may reorder the instructions such that step 3 happens before step 2 -

sharedHelper = // Step 3
    operator new(sizeof(MyHelper)); // Step 1
new (sharedHelper) MyHelper; // Step 2

Now, consider this scenario: Thread 1 executes steps 1 and 3 and is suspended. So the static variable points to a memory location that has not yet been initialized. Now, if thread 2 checks if(sharedHelper), it finds a non-NULL address, and returns that address. But wait, this address is not initialized yet - and BAM, you have a EXC_BAD_ACCESS.

Also check this related excellent answer.

What you said, plus on a multi-core system, different threads are not guaranteed to see write operations in the same order. So even if the code does everything in the "right" order, another thread might not see things in the same way. dispatch_once does all the necessary read/write barriers to make everything work correctly. And after the first call to dispatch_once (and calls happening at the same time which must block), dispatch_once is _fast_ so there is absolutely no point checking a different condition. — gnasher729, Apr 04 '14 at 12:48

Is this method of initializing singleton thread safe?

1 Answers1