CODEWITHSUNDEEP
c++multithreading
user1242638
user1242638

Reputation: 11

Is the following singleton implementation thread safe?

#include<iostream>
using namespace std;
class singleton
{
      private:
      static singleton* ref;
      singleton()
      {
                 cout<<"singleton ctor"<<endl;
      }
      public:
      static singleton* getInstance()
      {
         return ref;
      }
};

singleton* singleton::ref=new singleton();

int main()
{

    singleton* ref=singleton::getInstance()
}

Upvotes: 1

Views: 134

Answers (3)

Dietmar K&#252;hl
Dietmar K&#252;hl

Reputation: 153792

There are no threads in your example. Assuming there are more objects initialized during static initialization which may be spawning threads and access singleton::ref, the code is prone to access uninitialized memory and it isn't thread-safe. If the first thread is started after entering main() the code is thread-safe.

In case you want to make sure the object is properly constructed upon first access and, at the same time, make the construction thread-safe even when accessed during static initialization from multiple threads, you'd use

singleton* singleton::getInstance() {
    static singleton rc;
    return &rc;
}

The relevant section in the standard which guarantees that the above is thread safe is 6.7 [stmt.dcl] paragraph 4:

... If control enters the declaration concurrently while the variable is being initialized, the concurrent execution shall wait for completion of the initialization ...

The object would be constructed the first time getInstance() is called. Even if multiple threads concurrently call getInstance() the object will be constructed only once and the concurrent calls to getInstance() will block until construction is complete. You can make sure that the construction of getInstance() happens during static initialization by using it in the implementation of singleton:

static singleton* constructionDummy = singleton::getInstance();

Note that singletons generally cause major problems, doubly so in multi-threaded programs. In addition to the problems singletons already create in single-threaded programs they further introduce the potential for data races and, in an attempt to address the data races, tend to introduce serialization. With the possible exception of singletons which set up strictly immutable data I recommend to not use them.

Upvotes: 3

rakeshdn
rakeshdn

Reputation: 135

While as a general principle I agree with the point of view that creating objects in well defined order is preferable, from thread safety perspective the implementation posted in the question is safer than constructing the singleton object within getInstance. In the latter case unless the programmer explicitly use atomic operations, the contructor can, in theory be invoked more than once. Following is dis-assembly using VS2013 (debug with no optimizations) for x86 target for this code.

singleton* singleton::getInstance() {
    static singleton rc;
    return &rc;
} 

static singleton* getInstance()
{
    00E85820  push        ebp  
    00E85821  mov         ebp,esp  
    00E85823  push        0FFFFFFFFh  
    00E85825  push        0E89E6Eh  
    00E8582A  mov         eax,dword ptr fs:[00000000h]  
    00E85830  push        eax  
    00E85831  sub         esp,0C0h  
    00E85837  push        ebx  
    00E85838  push        esi  
    00E85839  push        edi  
    00E8583A  lea         edi,[ebp-0CCh]  
    00E85840  mov         ecx,30h  
    00E85845  mov         eax,0CCCCCCCCh  
    00E8584A  rep stos    dword ptr es:[edi]  
    00E8584C  mov         eax,dword ptr ds:[00E8F000h]  
    00E85851  xor         eax,ebp  
    00E85853  push        eax  
    00E85854  lea         eax,[ebp-0Ch]  
    00E85857  mov         dword ptr fs:[00000000h],eax  
    static singleton ref;
    00E8585D  mov         eax,dword ptr ds:[00E8F330h]  
    00E85862  and         eax,1  
    00E85865  jne         singleton::getInstance+6Ch (0E8588Ch)  
    00E85867  mov         eax,dword ptr ds:[00E8F330h]  
    00E8586C  or          eax,1  
    00E8586F  mov         dword ptr ds:[00E8F330h],eax  
    00E85874  mov         dword ptr [ebp-4],0  
    00E8587B  mov         ecx,0E8F32Ch  
    00E85880  call        singleton::singleton (0E810D2h)  
    00E85885  mov         dword ptr [ebp-4],0FFFFFFFFh  
    return &ref;
    00E8588C  mov         eax,0E8F32Ch  
}

dword ptr ds:[00E8F330h] is used as flag to check whether to call constructor or not.

As expected , no atomic compare and exchange instructions are emitted by the compiler. Therefore, if thread B executes instruction at 00E8585D before thread A executes instruction 00E8586F, the constructor will be invoked by both. However, to see this in practice, we would probably need to use special test cases where multiple threads are spawned (on a multicore processor) and blocked on an event/semaphore, . Then signal the semaphore to release all thread at once, and hopefully we will see the constructor will be called more than once. The same applies if you do a new singleton() inside getInstance.

This issue is not specific to singletons.

Upvotes: 0

Danvil
Danvil

Reputation: 22981

If you mean by thread safe that different threads will get the correct (same) pointer by calling singleton::getInstance, than: yes, it is thread safe.

And (as pointed out in the comment) you should create all you non trivial static variables in a well-defined order in the main function (or another function)!

So write this:

singleton* singleton::ref = NULL;

int main()
{
    singleton::ref = new singleton();
    // create objects which use the singleton
    singleton* ref = singleton::getInstance();
    // create threads...

}

Upvotes: -1

Related Questions