c++multithreadingsingletondouble-checked-locking

Double-check locking issues, c++


I left the rest of implementation for simplicity because it is not relevant here. Consider the classical implemetation of Double-check loking descibed in Modern C++ Design.

Singleton& Singleton::Instance()
{
    if(!pInstance_) 
    { 
         Guard myGuard(lock_); 
         if (!pInstance_) 
         {
            pInstance_ = new Singleton; 
         }
     }
     return *pInstance_;
}

Here the author insists that we avoid the race condition. But I have read an article, which unfortunately I dont remeber very well, in which the following flow was described.

  1. Thread 1 enters first if statement
  2. Thread 1 enters the mutex end get in the second if body.
  3. Thread 1 calls operator new and assigns memory to pInstance than calls a constructor on that memory;
  4. Suppose the thread 1 assigned the memory to pInstance but not created the object and thread 2 enters the function.
  5. Thread 2 see that the pInstance is not null (but yet not initialized with constructor) and returns the pInstance.

In that article the author stated then the trick is that on the line pInstance_ = new Singleton; the memory can be allocated, assigned to pInstance that the constructor will be called on that memory.

Relying to standard or other reliable sources, can anyone please confirm or deny the probability or correctness of this flow? Thanks!


Solution

  • The problem you describe can only occur if for reasons I cannot imagine the conceptors of the singleton uses an explicit (and broken) 2 steps construction:

         ...
         Guard myGuard(lock_); 
         if (!pInstance_) 
         {
            auto alloc = std::allocator<Singleton>();
            pInstance_ = alloc.allocate(); // SHAME here: race condition
            // eventually other stuff
            alloc.construct(_pInstance);   // anything could have happened since allocation
         }
         ....
    

    Even if for any reason such a 2 step construction was required, the _pInstance member shall never contain anything else that nullptr or a fully constructed instance:

            auto alloc = std::allocator<Singleton>();
            Singleton *tmp = alloc.allocate(); // no problem here
            // eventually other stuff
            alloc.construct(tmp);              // nor here
            _pInstance = tmp;                  // a fully constructed instance
    

    But beware: the fix is only guaranteed on a mono CPU. Things could be much worse on multi core systems where the C++11 atomics semantics are indeed required.