Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Singleton instance declared as static variable of GetInstance method, is it thread-safe? [duplicate]

I've seen implementations of Singleton patterns where instance variable was declared as static variable in GetInstance method. Like this:

SomeBaseClass &SomeClass::GetInstance()
{
   static SomeClass instance;
   return instance;
}

I see following positive sides of this approach:

  • The code is simpler, because it's compiler who responsible for creating this object only when GetInstance called for the first time.
  • The code is safer, because there is no other way to get reference to instance, but with GetInstance method and there is no other way to change instance, but inside GetInstance method.

What are the negative sides of this approach (except that this is not very OOP-ish) ? Is this thread-safe?

like image 201
okutane Avatar asked Jan 16 '09 03:01

okutane


People also ask

Are Singleton thread-safe?

Is singleton thread safe? A singleton class itself is not thread safe. Multiple threads can access the singleton same time and create multiple objects, violating the singleton concept. The singleton may also return a reference to a partially initialized object.

What is Singleton design pattern How will you make it thread-safe?

Thread Safe Singleton: A thread safe singleton is created so that singleton property is maintained even in multithreaded environment. To make a singleton class thread safe, getInstance() method is made synchronized so that multiple threads can't access it simultaneously.

How do you make a singleton class thread-safe volatile?

Thread Safe Singleton in JavaCreate the private constructor to avoid any new object creation with new operator. Declare a private static instance of the same class. Provide a public static method that will return the singleton class instance variable.

Why instance is static in Singleton?

The purpose of Singleton is to have only one instance of that object[1]. By making a sealed class with a private static instance which is automatically instantiated on first access, you provide its necessary trait of only creating one copy of the actual object that is then accessed by the Singleton.


3 Answers

In C++11 it is thread safe:

§6.7 [stmt.dcl] p4 If control enters the declaration concurrently while the variable is being initialized, the concurrent execution shall wait for completion of the initialization.

In C++03:

  • Under g++ it is thread safe.
    But this is because g++ explicitly adds code to guarantee it.

One problem is that if you have two singletons and they try and use each other during construction and destruction.

Read this: Finding C++ static initialization order problems

A variation on this problem is if the singleton is accessed from the destructor of a global variable. In this situation the singleton has definitely been destroyed, but the get method will still return a reference to the destroyed object.

There are ways around this but they are messy and not worth doing. Just don't access a singleton from the destructor of a global variable.

A Safer definition but ugly:
I am sure you can add some appropriate macros to tidy this up

SomeBaseClass &SomeClass::GetInstance()
{
#ifdef _WIN32 
Start Critical Section Here
#elif  defined(__GNUC__) && (__GNUC__ > 3)
// You are OK
#else
#error Add Critical Section for your platform
#endif

    static SomeClass instance;

#ifdef _WIN32
END Critical Section Here
#endif 

    return instance;
}
like image 154
Martin York Avatar answered Oct 17 '22 23:10

Martin York


It is not thread safe as shown. The C++ language is silent on threads so you have no inherent guarantees from the language. You will have to use platform synchronization primitives, e.g. Win32 ::EnterCriticalSection(), to protect access.

Your particular approach would be problematic b/c the compiler will insert some (non-thread safe) code to initialize the static instance on first invocation, most likely it will be before the function body begins execution (and hence before any synchronization can be invoked.)

Using a global/static member pointer to SomeClass and then initializing within a synchronized block would prove less problematic to implement.

#include <boost/shared_ptr.hpp>

namespace
{
  //Could be implemented as private member of SomeClass instead..
  boost::shared_ptr<SomeClass> g_instance;
}

SomeBaseClass &SomeClass::GetInstance()
{
   //Synchronize me e.g. ::EnterCriticalSection()
   if(g_instance == NULL)
     g_instance = boost::shared_ptr<SomeClass>(new SomeClass());
   //Unsynchronize me e.g. :::LeaveCriticalSection();
   return *g_instance;
}

I haven't compiled this so it's for illustrative purposes only. It also relies on the boost library to obtain the same lifetime (or there about) as your original example. You can also use std::tr1 (C++0x).

like image 44
Henk Avatar answered Oct 18 '22 00:10

Henk


According to specs this should also work in VC++. Anyone know if it does?

Just add keyword volatile. The visual c++ compiler should then generate mutexes if the doc on msdn is correct.

SomeBaseClass &SomeClass::GetInstance()
{
   static volatile SomeClass instance;
   return instance;
}
like image 43
Sal Avatar answered Oct 18 '22 00:10

Sal