86

The usual pattern for a singleton class is something like

static Foo &getInst()
{
  static Foo *inst = NULL;
  if(inst == NULL)
    inst = new Foo(...);
  return *inst;    
}

However, it's my understanding that this solution is not thread-safe, since 1) Foo's constructor might be called more than once (which may or may not matter) and 2) inst may not be fully constructed before it is returned to a different thread.

One solution is to wrap a mutex around the whole method, but then I'm paying for synchronization overhead long after I actually need it. An alternative is something like

static Foo &getInst()
{
  static Foo *inst = NULL;
  if(inst == NULL)
  {
    pthread_mutex_lock(&mutex);
    if(inst == NULL)
      inst = new Foo(...);
    pthread_mutex_unlock(&mutex);
  }
  return *inst;    
}

Is this the right way to do it, or are there any pitfalls I should be aware of? For instance, are there any static initialization order problems that might occur, i.e. is inst always guaranteed to be NULL the first time getInst is called?

user168715
  • 5,469
  • 1
  • 31
  • 42
  • 6
    But you don't have time to find an example and tee up a close vote? I'm fresh out at the moment. – bmargulies Apr 04 '10 at 22:00
  • 1
    possible duplicate of http://stackoverflow.com/questions/6915/thread-safe-lazy-contruction-of-a-singleton-in-c – kennytm Apr 04 '10 at 22:04
  • 3
    @bmargulies No, the questioner obviously couldn't be bothered, so why should I? I've decided to give up downvoting and closing as dupes, as I seem to be one of the few bothering to keep crap out of SO. And do you know, laziness feels good! –  Apr 04 '10 at 23:00
  • I did take time to carefully describe my problem, with snippets and a discussion of what I knew/had tried. I'm sorry I've wasted your time with "crap." :( – user168715 Apr 04 '10 at 23:06
  • @Neil: If you don't feel like searching for dupes and closing questions, why bother pointing them out and earning bad karma? This is a pretty good thread on the subject: http://stackoverflow.com/questions/6915. I voted to close. – sbi Apr 05 '10 at 07:06
  • 1
    @sbi: so did I. Scattering answers throughout thousands of question is the best way to make it hard to search through them later on. – Matthieu M. Apr 05 '10 at 12:01
  • Possible duplicate of [C++ Singleton design pattern](https://stackoverflow.com/questions/1008019/c-singleton-design-pattern) – Trevor Boyd Smith Sep 04 '18 at 12:11

9 Answers9

112

If you are using C++11, here is a right way to do this:

Foo& getInst()
{
    static Foo inst(...);
    return inst;
}

According to new standard there is no need to care about this problem any more. Object initialization will be made only by one thread, other threads will wait till it complete. Or you can use std::call_once. (more info here)

Sat
  • 1,355
  • 3
  • 9
  • 9
  • 3
    This is the C++11 solution I would expect people to implement. – Alexander Oh Nov 11 '13 at 13:59
  • 8
    Sadly, this is not thread safe in VS2013, see "Magic Statics" here: http://msdn.microsoft.com/en-gb/library/hh567368.aspx – Chris Drew Aug 06 '14 at 08:29
  • 4
    VS 14 seems to fix this -- http://blogs.msdn.com/b/vcblog/archive/2014/06/03/visual-studio-14-ctp.aspx – Sea Coast of Tibet Mar 03 '15 at 17:52
  • 9
    To avoid confusion, maybe you could either add static to the function declaration or explicitly state that it is a non-member function. – MikeMB May 16 '16 at 11:02
  • Are calls for this instances from different threads are thread-safe or functions of instanced class must care of atomicity by itself? – Shadasviar May 01 '18 at 10:58
  • using std::call_once : http://www.modernescpp.com/index.php/thread-safe-initialization-of-data, and look at scott meyers singleton – Swapnil Aug 19 '21 at 09:01
48

Your solution is called 'double checked locking' and the way you've written it is not threadsafe.

This Meyers/Alexandrescu paper explains why - but that paper is also widely misunderstood. It started the 'double checked locking is unsafe in C++' meme - but its actual conclusion is that double checked locking in C++ can be implemented safely, it just requires the use of memory barriers in a non-obvious place.

The paper contains pseudocode demonstrating how to use memory barriers to safely implement the DLCP, so it shouldn't be difficult for you to correct your implementation.

JoeG
  • 12,994
  • 1
  • 38
  • 63
  • if(inst == NULL) { temp = new Foo(...); inst=temp;} Doesn't that guarantee that the constructor finished before inst was assigned? I realize it can (and probably will be) optimized away, but logically that solves the problem, no? – stu Mar 24 '15 at 22:17
  • 1
    That doesn't help because a compliant compiler is free to reorder the assignment and construction steps as it sees fit. – JoeG Mar 24 '15 at 22:38
  • I read the paper carefully, and it seems that the recommendation is simply avoid DLCP with Singleton. You will have to volatile the heck out of the class, and add memory barriers (wouldn't that affect efficiency as well?). For practical needs, use a simple, single lock, and cache the object you get from "GetInstance". – Guy Apr 24 '15 at 18:15
  • also just read the paper and I understood the main conclusion as: DLCP can be implemented thread safe using memory barriers, **but** not in a portable way (before c++11) – 463035818_is_not_an_ai Sep 11 '17 at 12:58
13

Herb Sutter talks about the double-checked locking in CppCon 2014.

Below is the code I implemented in C++11 based on that:

class Foo {
public:
    static Foo* Instance();
private:
    Foo() {}
    static atomic<Foo*> pinstance;
    static mutex m_;
};

atomic<Foo*> Foo::pinstance { nullptr };
std::mutex Foo::m_;

Foo* Foo::Instance() {
  if(pinstance == nullptr) {
    lock_guard<mutex> lock(m_);
    if(pinstance == nullptr) {
        pinstance = new Foo();
    }
  }
  return pinstance;
}

you can also check complete program here: http://ideone.com/olvK13

qqibrow
  • 2,942
  • 1
  • 24
  • 40
  • 1
    @Etherealone what's your suggestion? – qqibrow Oct 22 '15 at 01:19
  • 5
    A simple `static Foo foo;` and `return &foo;` inside the instance function would be enough; `static` initialization is thread-safe in C++11. Prefer reference to pointers though. – Etherealone Dec 27 '15 at 10:38
  • I get the error message in MSVC 2015: Severity Code Description Project File Line Source Suppression State Error (active) more than one operator "==" matches these operands: – user2286810 Nov 07 '16 at 23:06
  • @qqibrow may be you can also make copy constructor, move constructor, assignment operator, move assignment operator as private. – Mayur May 11 '20 at 20:41
10

Use pthread_once, which is guaranteed that the initialization function is run once atomically.

(On Mac OS X it uses a spin lock. Don't know the implementation of other platforms.)

kennytm
  • 510,854
  • 105
  • 1,084
  • 1,005
3

TTBOMK, the only guaranteed thread-safe way to do this without locking would be to initialize all your singletons before you ever start a thread.

sbi
  • 219,715
  • 46
  • 258
  • 445
0

Your alternative is called "double-checked locking".

There could exist multi-threaded memory models in which it works, but POSIX does not guarantee one

Steve Jessop
  • 273,490
  • 39
  • 460
  • 699
0

ACE singleton implementation uses double-checked locking pattern for thread safety, you can refer to it if you like.

You can find source code here.

baris.aydinoz
  • 1,902
  • 2
  • 18
  • 28
0

Does TLS work here? https://en.wikipedia.org/wiki/Thread-local_storage#C_and_C++

For example, 

static _thread Foo *inst = NULL;
static Foo &getInst()
{
  if(inst == NULL)
    inst = new Foo(...);
  return *inst;    
 }

But we also need a way to delete it explicitly, like

static void deleteInst() {
   if (!inst) {
     return;
   }
   delete inst;
   inst = NULL;
}
Joe C
  • 2,757
  • 2
  • 26
  • 46
-2

The solution is not thread safe because the statement

inst = new Foo();

can be broken down into two statements by compiler:

Statement1: inst = malloc(sizeof(Foo));
Statement2: inst->Foo();

Suppose that after execution of statement 1 by one thread context switch occurs. And 2nd thread also executes the getInstance() method. Then the 2nd thread will find that the 'inst' pointer is not null. So 2nd thread will return pointer to an uninitialized object as constructor has not yet been called by the 1st thread.

Ishekk ҆
  • 87
  • 1
  • 6