Singleton with multithreads

Question

This question was asked in an interview. The first part was to write the singleton class:

class Singleton
{
    static Singleton *singletonInstance;
    Singleton() {}

  public:
    static Singleton* getSingletonInstance()
    {
        if(singletonInstance == null)
        {
            singletonInstance = new Singleton();
        }
        return singletonInstance;
    }
};

Then I was asked how to handle this getSingletonInstance() in a multithreaded situation. I wasn't really sure, but I modified as:

class Singleton 
{
    static Singleton *singletonInstance;
    Singleton() {}
    static mutex m_;

  public:
    static Singleton* getSingletonInstance()
    {
        m_pend();
        if(singletonInstance == null)
        {
            singletonInstance = new Singleton();
        }
        return singletonInstance;
    }

    static void releaseSingleton()
    {
        m_post();
    }
};

Then I was told that although a mutex is required, pending and posting a mutex is not efficient as it takes time. And there is a better way to handle to this situation.

Does anybody know a better and more efficient way to handle the singleton class in a multithreaded situation?

Using global state in a multithreaded code is a nice way to get infinite headaches. Doubly so in case of stupidgleton. — Cat Plus Plus, Sep 03 '12 at 13:30

Mike Seymour · Accepted Answer · 2012-09-03T14:05:40.157

In C++11, the following is guaranteed to perform thread-safe initialisation:

static Singleton* getSingletonInstance()
{
    static Singleton instance;
    return &instance;
}

In C++03, a common approach was to use double-checked locking; checking a flag (or the pointer itself) to see if the object might be uninitialised, and only locking the mutex if it might be. This requires some kind of non-standard way of atomically reading the pointer (or an associated boolean flag); many implementations incorrectly use a plain pointer or bool, with no guarantee that changes on one processor are visible on others. The code might look something like this, although I've almost certainly got something wrong:

static Singleton* getSingletonInstance()
{
    if (!atomic_read(singletonInstance)) {
        mutex_lock lock(mutex);
        if (!atomic_read(singletonInstance)) {
            atomic_write(singletonInstance, new Singleton);
        }
    }
    return singletonInstance;
}

This is quite tricky to get right, so I suggest that you don't bother. In C++11, you could use standard atomic and mutex types, if for some reason you want to keep the dynamic allocation of you example.

Note that I'm only talking about synchronised initialisation, not synchronised access to the object (which your version provides by locking the mutex in the accessor, and releasing it later via a separate function). If you need the lock to safely access the object itself, then you obviously can't avoid locking on every access.

is it garanteed thread safe if returning Singleton& instead of Singleton * ? — Romain-p, Mar 04 '19 at 16:15
@Romain-p, provided example is called Meyers singleton. Thread safety is achieved by relying on a C++ standard that states: "static variables initialization should be thread-safe". In other words, thread-safe initialization of static variable it is a compiler headache. — Awesome Man, Jan 01 '21 at 14:02

score 16 · Answer 2 · answered Sep 03 '12 at 15:56

16

As @piokuc suggested, you can also use a once function here. If you have C++11:

#include <mutex>

static void init_singleton() {
    singletonInstance = new Singleton;
}
static std::once_flag singleton_flag;

Singleton* getSingletonInstance() {
    std::call_once(singleton_flag, init_singleton);
    return singletonInstance;
}

And, yes, this will work sensibly if the new Singleton throws an exception.

answered Sep 03 '12 at 15:56

Pete Becker

74,985
8
76
165

2

Thank you. I wasn't aware of the once function. – madu Sep 04 '12 at 00:43
@PeteBecker this is the best answer, perfect use of std ns. – Yogesh May 29 '19 at 07:12
@PeteBecker getSingletonInstance() is non static, which is a problem for singleton pattern, you will have to make it static. – Yogesh May 29 '19 at 07:39
@Yogesh — in the code I wrote, `getSingletonInstance` is not a member of a class. It is not, and should not be, static. – Pete Becker May 30 '19 at 02:50
@PeteBecker oh, my bad, was biased for cpp. – Yogesh May 30 '19 at 06:33

score 3 · Answer 3 · answered Sep 03 '12 at 13:26

3

If you have C++11 you can make singletonInstance an atomic variable, then use a double-checked lock:

if (singletonInstance == NULL) {
    lock the mutex
    if (singletonInstance == NULL) {
        singletonInstance = new Singleton;
    }
    unlock the mutex
}
return singletonInstance;

answered Sep 03 '12 at 13:26

Pete Becker

74,985
8
76
165

12

If your compiler correctly implements C++11, you don't need to lock it at all. `static Foo& getSingleton(){ static Foo foo{}; return foo; }` will *just work* under concurrent execution. – Xeo Sep 03 '12 at 13:29
@Xeo - good point. Make it an answer and I'll give it an up vote, but with a comment that this may be inefficient, depending on how the compiler manages locks for function static objects (if there's one shared lock, you could get contention). – Pete Becker Sep 03 '12 at 13:31
1

Well, I already wrote one, but the question isn't really a duplicate, atleast in my opinion: http://stackoverflow.com/a/11711991/500104. – Xeo Sep 03 '12 at 13:34
If you're going to explicitly write a line of pseudo-code for "unlock the mutex" then should the pseudo-code also ensure that it is unlocked when the `new` expression throws? Or can that go without saying and/or the exception be assumed to cause program termination? In a real program you'd have to decide which, so really I suppose I'm talking about pseudo-code etiquette here... – Steve Jessop Sep 03 '12 at 13:45
@SteveJessop - you're right in your implication that the unlock **must** be done if the allocation throws an exception. I was only trying to avoid calls to `m_pend()` and `m_post()`, as in the original code, since I have no idea what they do. Of course, in C++11 you'd use a lock object whose destructor unlocked the mutex. – Pete Becker Sep 03 '12 at 15:49

sehe · Answer 4 · 2012-09-03T13:37:44.063

You should actually lock the singleton, and not the instance. If the instance requires locking, that should be handled by the caller (or perhaps by the instance itself, depending on what kind of an interface it exposes)

Update sample code:

#include <mutex>

class Singleton 
{
    static Singleton *singletonInstance;
    Singleton() {}
    static std::mutex m_;

  public:

    static Singleton* getSingletonInstance()
    {
        std::lock_guard<std::mutex> lock(m_);
        if(singletonInstance == nullptr)
        {
            singletonInstance = new Singleton();
        }
        return singletonInstance;
    }
}

score 3 · Answer 5 · answered Sep 03 '12 at 13:47

If you use POSIX threads you can use pthread_once_t and pthread_key_t stuff, this way you can avoid using mutexes altogether. For example:

template<class T> class ThreadSingleton : private NonCopyable {
public:
    ThreadSingleton();
    ~ThreadSingleton();

    static T& instance();

private:
    ThreadSingleton( const ThreadSingleton& );
    const ThreadSingleton& operator=( const ThreadSingleton& )

    static pthread_once_t once_;
    static pthread_key_t  key_;

    static void init(void);
    static void cleanUp(void*);
};

And implementation:

template<class T> pthread_once_t ThreadSingleton<T>::once_ = PTHREAD_ONCE_INIT;
template<class T> pthread_key_t ThreadSingleton<T>::key_;

template<class T>  
T& ThreadSingleton<T>::instance()
{
    pthread_once(&once_,init);

    T* value = (T*)pthread_getspecific(key_);
    if(!value)
    {   

        value = new T();
        pthread_setspecific(key_,value);
    }   
    return *value;
}

template<class T> void ThreadSingleton<T>::cleanUp(void* data)
{
    delete (T*)data;
    pthread_setspecific(key_,0);
}

template<class T> void ThreadSingleton<T>::init()
{
    pthread_key_create(&key_,cleanUp);
}

I think the question is about creating a single instance, not one per thread. — Mike Seymour, Sep 03 '12 at 13:51
That's not how I understood the question.. I just read it again and I think you're right. Sorry. — piokuc, Sep 03 '12 at 14:01

Singleton with multithreads

5 Answers5