9

I've typically gotten used to implementing a singleton pattern in this manner because it's so easy:

class MyClass
{
    public:
        MyClass* GetInstance()
        {
            static MyClass instance;
            return &instance;
        }

    private:
        //Disallow copy construction, copy assignment, and external
        //default construction.
};

This seems significantly easier than creating a static instance pointer, initializing it in the source file, and using dynamic memory allocation in the instance function with guards.

Is there a downside that I'm not seeing? It looks thread-safe to me because I would think the first thread to get to the first line would cause the instantiation - and it seems nice and concise. I figure there has to be a problem I'm not seeing since this is not common though - I'd like to get some feedback before I keep using it

John Humphreys
  • 37,047
  • 37
  • 155
  • 255
  • 2
    You could return a MyClass& instead of a MyClass* – Paolo Tedesco Feb 10 '12 at 14:10
  • 10
    You mean, aside from the fact that it's a singleton? http://stackoverflow.com/q/137975/10077 – Fred Larson Feb 10 '12 at 14:19
  • Yeah, yeah I know they're an anti-pattern and should be avoided :) I'm still interested in why this may not be thread-safe though. I know modifying the singleton's data-members should be mutex-guarded, but I thought it's creation via this function would be safe. – John Humphreys Feb 10 '12 at 14:57

5 Answers5

3

The downside is that you have no control over exactly when the object is destroyed. This will be a problem if other static objects try to access it from their destructors.

A C++11 compliant compiler must implement this in a thread-safe way; however, older compilers might not. If you're in doubt, and you don't especially want lazy initialisation, you could force the object to be created by calling the accessor before starting any threads.

Mike Seymour
  • 249,747
  • 28
  • 448
  • 644
  • So, you're saying that even though there's a static in that function, in non-C++11 compilers it could be doubly-created? but as long as I call the instance function prior to entering my multi-threaded phase of the code it is thread-safe creation? just clarifying :) Thanks for the answer thus far! – John Humphreys Feb 10 '12 at 15:00
  • @w00te: Yes, that's right. Before C++11, the standard had nothing to say about thread safety, so it was entirely up to the compiler implementation whether to protect against double creation. It should be thread safe with most reasonably recent compilers. – Mike Seymour Feb 10 '12 at 15:46
  • +1 Thank you for the help, it's nice to know it's standard-version- specific. – John Humphreys Feb 10 '12 at 16:31
3

This is not an inherent threadsafe solution: while constructing the instance, another thread can preempt and try to get the instance, resulting in either a double instance or in using an unconstructed instance.

This is handled by several compilers by adding a guard (in gcc, I think there is a flag to disable this) because there is no way to guard this with a userdefined mutex.

stefaanv
  • 14,072
  • 2
  • 31
  • 53
  • So, you're saying that even know there is a static variable here, more than one of it can be created somehow? – John Humphreys Feb 10 '12 at 14:54
  • The instance is initialised upon first use of the function. I guess it is more double initialised than double instance. – stefaanv Feb 10 '12 at 14:56
  • I would have thought that the first thread to hit the line with the static would trigger it's creation/initialization and that the second thread hitting that line would be fully aware of that creation and initialization because of how statics work. Why wouldn't that be the case? – John Humphreys Feb 10 '12 at 14:59
  • I found something to back me up: http://stackoverflow.com/questions/1270927/are-function-static-variables-thread-safe-in-gcc – stefaanv Feb 10 '12 at 15:02
  • +1 for that it's a nice reference. I have to follow up one last time though and bug you :) It says: "Thread-safe initialization of local statics is very important. If you need generally thread-safe access to local statics then you will need to add the appropriate guards yourself." This sounds to me like there is only one version and it is only created once, but If I want to protect access to it after that point, I need to add a mutex (i.e. make sure it's not changed by more than one thing simultaneously via the Instance() return value). Do you interpret this the same way? – John Humphreys Feb 10 '12 at 15:26
  • Yes, you need to protect access with the mutex afterwards, but my point was, technically if compilers don't handle this, a thread can be preempted while being in the constructor, what will the running thread do when entering the function? – stefaanv Feb 10 '12 at 15:40
2

There are two issues in the interface:

  • You should return a reference
  • You should make either the destructor or the delete operator private

Also, there is a slight risk of attempting to using this class after it's been destructed.

Regarding your multi-threading concerns (and initialization I guess): it's fine in C++11 and have been fine for a long time on good C++ compilers anyway.

Matthieu M.
  • 287,565
  • 48
  • 449
  • 722
0

Aside from in a multi-threaded scenario - no. Well - let me qualify this, construction is lazy (so the first call may have a hit) and destruction - well, there is no guarantee (aside from it will be - at some point)

Nim
  • 33,299
  • 2
  • 62
  • 101
0

Generally speaking, qualifier static for local variable in a method doesn't guarantee that the variable is created only once. If the method is called by different threads, it could be created once for each thread so many times, as many threads called it. It should be not confused with static member of the class, which is created once before program is started. Thread safety of local static variables depends on particular realization of c++. Useful link : Are function static variables thread-safe in GCC?

Hope it helps.

Community
  • 1
  • 1
Dmitriy Kachko
  • 2,804
  • 1
  • 19
  • 21