views:

1080

answers:

6

Is the following implementation, using lazy initialization, of Singleton (Meyers Singleton) thread safe?

static Singleton& instance()
{
     static Singleton s;
     return s;
}

If not, why and how to make it thread safe?

+4  A: 

The correct answer depends on your compiler. It can decide to make it threadsafe; it's not "naturallly" threadsafe.

MSalters
+13  A: 

The code in your question is not thread safe. There is an article by Meyers which discusses thread safe implementations of the pattern (as well as many others on the net):

Groo
+1 for links to good articles.
Ankur
There is also an extensive discussion on the Singleton Pattern (lifetime and threadsafety) by Alexandrescu in Modern C++ Design. See Loki's site: http://loki-lib.sourceforge.net/index.php?n=Pattern.Singleton
Matthieu M.
+7  A: 

To answer your question about why it's not threadsafe, it's not because the first call to instance() must call the constructor for Singleton s. To be threadsafe this would have to occur in a critical section, and but there's no requirement in the standard that a critical section be taken (the standard to date is completely silent on threads). Compilers often implement this using a simple check and increment of a static boolean - but not in a critical section. Something like the following pseudocode:

static Singleton& instance()
{
    static bool initialized = false;
    static char s[sizeof( Singleton)];

    if (!initialized) {
        initialized = true;

        new( &s) Singleton(); // call placement new on s to construct it
    }

    return (*(reinterpret_cast<Singleton*>( &s)));
}

So here's a simple thread-safe Singleton (for Windows). It uses a simple class wrapper for the Windows CRITICAL_SECTION object so that we can have the compiler automatically initialize the CRITICAL_SECTION before main() is called. Ideally a true RAII critical section class would be used that can deal with exceptions that might occur when the critical section is held, but that's beyond the scope of this answer.

The fundamental operation is that when an instance of Singleton is requested, a lock is taken, the Singleton is created if it needs to be, then the lock is released and the Singleton reference returned.

#include <windows.h>

class CritSection : public CRITICAL_SECTION
{
public:
    CritSection() {
        InitializeCriticalSection( this);
    }

    ~CritSection() {
        DeleteCriticalSection( this);
    }

private:
    // disable copy and assignment of CritSection
    CritSection( CritSection const&);
    CritSection& operator=( CritSection const&);
};


class Singleton
{
public:
    static Singleton& instance();

private:
    // don't allow public construct/destruct
    Singleton();
    ~Singleton();
    // disable copy & assignment
    Singleton( Singleton const&);
    Singleton& operator=( Singleton const&);

    static CritSection instance_lock;
};

CritSection Singleton::instance_lock; // definition for Singleton's lock
                                      //  it's initialized before main() is called


Singleton::Singleton()
{
}


Singleton& Singleton::instance()
{
    // check to see if we need to create the Singleton
    EnterCriticalSection( &instance_lock);
    static Singleton s;
    LeaveCriticalSection( &instance_lock);

    return s;
}

Man - that's a lot of crap to "make a better global".

The main drawbacks to this implemention (if I didn't let some bugs slip through) is:

  • if new Singleton() throws, the lock won't be released. This can be fixed by using a true RAII lock object instead of the simple one I have here. This can also help make things portable if you use something like Boost to provide a platform independent wrapper for the lock.
  • this guarantees thread safety when the Singleton instance is requested after main() is called - if you call it before then (like in a static object's initialization) things might not work because the CRITICAL_SECTION might not be initialized.
  • a lock must be taken each time an instance is requested. As I said, this is a simple thread safe implementation. If you need a better one (or want to know why things like the double-check lock technique is flawed), see the papers linked to in Groo's answer.
Michael Burr
Good explanation. Thanks.
Ankur
+1 'that's a lot of crap to "make a better global"' - that got a laugh! ;-)
Bob Cross
Uh oh. What happens if `new Singleton()` throws?
sbi
@Bob - to be fair, with a proper set of libraries, all the cruft having to do with non-copyability and a proper RAII lock would go away or be minimal. But I wanted the example to be reasonably self-contained. Even though singleton's are a lot of work for maybe minimal gain, I have found them useful in managing the use of globals. They tend to make it easier to find out where and when they're used a little better than just a naming convention.
Michael Burr
@sbi: in this example, if `new Singleton()` throws there's definitely a problem with the lock. A proper RAII lock class should be used, something like `lock_guard` from Boost. I wanted the example to be more or less self-contained, and it was already a bit of a monster so I left off exception safety (but called it out). Maybe I should fix that so this code doesn't get cut-n-pasted somewhere inappropriate.
Michael Burr
Why dynamically allocate the singleton? Why not just make 'pInstance' a static member of 'Singleton::instance()'?
Martin York
@Martin - done. You're right, that makes it a bit simpler - would be even better if I used an RAII lock class.
Michael Burr
+4  A: 

Is the following implementation [...] thread safe?

On most platforms, this is not thread-safe. (Append the usual disclaimer explaining that the C++ standard doesn't know about threads, so, legally, it doesn't say whether it is or not.)

If not, why [...]?

The reason it isn't is that nothing prevents more than one thread from simultaneously executing s' constructor.

how to make it thread safe?

"C++ and the Perils of Double-Checked Locking" by Scott Meyers and Andrei Alexandrescu is a pretty good treatise on the subject of thread-safe singletons.

sbi
+1  A: 

As MSalters said: It depends on the C++ implementation you use. Check the documentation. As for the other question: "If not, why?" -- The C++ standard doesn't yet mention anything about threads. But the upcoming C++ version is aware of threads and it explicitly states that the initialization of static locals is thread-safe. If two threads call such a function, one thread will perform an initialization while the other will block & wait for it to finish.

sellibitze
+1  A: 

Looking at the next standard (section 6.7.4), it explians how static local initialization is thread safe. So once that section of standard is widely implemented, Meyer's Singleton will be the preferred implementation.

I disagree with many answers already. Most compilers already implement static initialization this way. The one notable exception is Microsoft Visual Studio.

caspin