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

Question

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?

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

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;
}

Does "*§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.*", this also apply to global static variables? Thanks — Rick, Apr 29 '20 at 13:28
@Rick `static` on a global variable means something different: the variable has [internal linkage](https://en.cppreference.com/w/cpp/language/storage_duration#Linkage). Only the translation unit containing the variable can see and directly interact with it. Note that if you have the same identifier in multiple translation units they are all different instances with the same name and `static` prevents them from colliding when the linker assembles the program. It can only be initialized once at program start-up so threading is not an issue. — user4581301, Aug 12 '21 at 22:45

score 5 · Answer 2 · answered Jan 16 '09 at 06:32

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).

Yes use critical section on windows. G++ guarantees that only one thread will initialize it. — Martin York, Jan 16 '09 at 08:14
Create a second, private method _getInstance() that actually contains the definition of the static instance, then have the (public) GetInstance() method calls that method in between OS-specific synch primitives. C++ cannot reorder in this case, and you avoid a heap allocation. — j_random_hacker, Jan 16 '09 at 16:01
@j_random_hacker: Yeah that's a neat idea. @Martin York: Thanks for the tip about g++ I did not know that. — Henk, Jan 16 '09 at 20:35

score 1 · Answer 3 · edited Jan 05 '12 at 08:51

1

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;
}

edited Jan 05 '12 at 08:51

sra

23,820
7
55
89

answered Aug 18 '10 at 15:25

Sal

35
1

1

Which doc is that? For instance http://msdn.microsoft.com/en-us/library/12a04hfd.aspx does not mention mutex. – Bryan Mar 03 '14 at 16:00
5

volatile has nothing to do with thread safety – Giovanni Funchal Jul 27 '16 at 08:57

score -12 · Answer 4 · answered Jan 16 '09 at 04:07

-12

It shares all of the common failings of Singleton implementations, namely:

It is untestable
It is not thread safe (this is trivial enough to see if you imagine two threads entering the function at the same time)
It is a memory leak

I recommend never using Singleton in any production code.

answered Jan 16 '09 at 04:07

1800 INFORMATION

131,367
29
160
239

1

The question isn't related to singleton pattern usage, but to it's particular implementation. – okutane Jan 16 '09 at 04:13
This particular implementation has all of those failings. Please feel free to argue the point any time you like – 1800 INFORMATION Jan 16 '09 at 04:20
Or you know, just vote down silently. It's all the same to me – 1800 INFORMATION Jan 16 '09 at 07:08
2

Actually this IS thread safe under G++. The compiler has logic that guarantees that only one thread will initialise static function members. – Martin York Jan 16 '09 at 08:11
Can't argue with the general recommendation as Singeltons are extremly hard to use correctly. But everything has a use. I just can't think of one. – Martin York Jan 16 '09 at 08:13
2

It is not thread safe in C++, just because it happens to be thread safe in some non-standard implementation doesn't change that fact – 1800 INFORMATION Jan 16 '09 at 20:31
I am sure MS will not be long before they add it and with the new C++ standard coming soon and being thread aware it will (hopefully) be fixed here. But technically you are correct. – Martin York Jan 17 '09 at 05:30
It is testable. But what I think you mean is that it makes testing other things that use a singleton harder, but this is still not imposable. – Martin York Jan 17 '09 at 05:32
I think you'll find that mine and your definition of "testable" are different. You should have a good read of the google testing blog (link given above). Testability depends on being able to replace external components (such as your singleton) with a mock - the Singleton implementation prevents that – 1800 INFORMATION Jan 17 '09 at 08:43
1

In this specific design yes it is not possible to replace with a mock object. But this pattern is relatively easy to extend with a factory. The signgelton should not create itself it delegates this work to a factory. Different factory can be used for test and production. The test version of the factory just instantiates a mock version of the singelton. – Martin York Mar 01 '10 at 15:50
1

@Martin: About the leak - I guess he meant that the static instance might stick around until doom's day without ever getting used after the first use and instantiation, making it leak-ish as the memory cannot be reclaimed. – Johann Gerell Aug 03 '10 at 07:49
1

@Johann Gerell: I suppose yes just like all objects with a static storage duration its space will not be reclaimed until program termination. But that's not a leak (just like a global is not a leak) as a reference to the object can be obtained. And the destructor will be called correctly. Remember this is a pattern not a design it is meant to be adaptable to the situation and it is relatively easy to adapt the pattern to release the object when no longer in use. But that is another question (maybe you should ask (It is also a common interview question)) – Martin York Aug 03 '10 at 11:07
@Johann Gerell: But as emphasized above the worst thing about singeltons is they make the code exceedingly hard to test (as they are basically global state mascaraing as a pattern). This is why singeltons can not be used carelessly (to make the code testable you must be able to switch them out appropriately which requires the use of other patterns (see above where I suggest the use of a factory)). – Martin York Aug 03 '10 at 11:13

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

4 Answers4

Linked