Thread safe singleton implementation : whats wrong?

Question

This question was closed as duplicate and hence making the question more explicit and reopening the question:

The questions are :

1) Is the implementation below thread safe or not?

2) What are the problems with the below approach (except for complexity)

class Singleton
{
    public:
    static void init (){
        static Singleton _single ;
        cout<<"Called"<<endl;
        m_Singleton = &_single ;
    }
    static Singleton & instance ()
    {
        static pthread_once_t once_flag = PTHREAD_ONCE_INIT;
        cout<<(unsigned int)PTHREAD_ONCE_INIT<<endl;
        pthread_once(&once_flag, &Singleton::init) ;
        return *m_Singleton;
    }
    static Singleton* m_Singleton;
    private:
    Singleton (){}
    Singleton (const Singleton&) ;
    Singleton operator=(Singleton&) ;
};

I'm counting on that something's wrong with this implementation hence not suggested anywhere

Thanks

@ScarletAmaranth: you search for design patterns and you find singleton is the most widely discussed. I may not use it but sure wanted to understand what are the challenges and problems underneath. — mukeshkumar, Jan 19 '14 at 12:48
@FredOverflow: I checked the provided link but Meyer's impl is not thread safe (before C++11). As well couple of links provided in the accepted answer are broken. The question shall be marked duplicate if I post Meyer's implementation and ask if it is thread safe. My question is if the implementation is question is thread safe or not and what are the problems with it. — mukeshkumar, Jan 19 '14 at 13:06
It's threadsafe, but not exception-safe. It has undefined behavior if the Singleton constructor throws an exception. — Casey, Jan 19 '14 at 18:12
@Casey: is it because the exception will be trapped within thread library function? How about a NULL check after pthread_once? — mukeshkumar, Jan 20 '14 at 16:35
@hype The exception will certainly propagate out of `pthread_once`, and `m_Singleton` will still be `nullptr`. The problem is that it's not clear what the state of `once_flag` will be, or what will happen to other threads waiting on the thread that was performing the initialization. For example, if `pthread_once` really is a C function implemented with a `pthread_mutex`, it's possible that the exception will bypass unlocking that mutex and the waiting threads will wait forever. You'd avoid all that using `std::call_once` with a `std::once_flag` from the C++11 standard library. — Casey, Jan 20 '14 at 18:10
@Casey: Thanks, makes sense. Exactly for these reasons I see the question was not duplicate. — mukeshkumar, Jan 21 '14 at 03:11
I agree that this specific instance is not covered by the"duplicate" question, one of those reopen votes is mine. — Casey, Jan 21 '14 at 03:20

juanchopanza · Answer 1 · 2014-01-19T12:42:24.483

1

what's wrong?

It is too complicated. Just use this (assuming you really want a singleton):

static Singleton & instance ()
    static Singleton _single;
    cout<<"Called"<<endl;
    return _single ;
}

edited Jan 19 '14 at 12:42

answered Jan 19 '14 at 12:37

juanchopanza

223,364
34
402
480

Is it thread safe (before C++11)? – mukeshkumar Jan 19 '14 at 12:47
@hype No, it is not, see [this related question](http://stackoverflow.com/questions/17924688/meyers-singleton-thread-safe-with-c-98). – juanchopanza Jan 19 '14 at 12:51
Yes..so I come back to my question :) Is the implementation in question thread safe and except that it is complicated (already agreed), are there any other problems with it? – mukeshkumar Jan 19 '14 at 12:53
@hype On the surface it looks OK, but my `pthreads` knowledge is rusty (I've been using `Boost.Thread` and C++11 threading lib for too long. – juanchopanza Jan 19 '14 at 12:56

Thread safe singleton implementation : whats wrong?

1 Answers1