Thread safe Singleton class

Discussion in 'C++' started by Udit Sharma, Jun 26, 2009.

  1. Udit Sharma

    Udit Sharma Guest

    Hi,

    I was informed by my colleague that the following code is not thread
    safe but I still don't know why because his arguments weren't very
    convincing.

    //Singleton.h
    class Singleton
    {
    private:
    static Singleton instance;
    Singleton(){}
    Singleton(const Singleton&){}
    Singleton& operator=(Singleton const&){}
    public:
    static Singleton& getInstance();
    };

    //Singleton.cpp
    #include "Singleton.h"
    Singleton Singleton::instance;

    Singleton& Singleton::getInstance()
    {
    return instance;
    }

    Since am maintaining a static instance this should be created only
    once irrespective of how many threads are accessing the getInstance()
    function at the same time. So is this code not thread safe?

    Cheers
     
    Udit Sharma, Jun 26, 2009
    #1
    1. Advertising

  2. Udit Sharma wrote:

    > Hi,
    >
    > I was informed by my colleague that the following code is not thread
    > safe but I still don't know why because his arguments weren't very
    > convincing.
    >
    > //Singleton.h
    > class Singleton
    > {
    > private:
    > static Singleton instance;
    > Singleton(){}
    > Singleton(const Singleton&){}
    > Singleton& operator=(Singleton const&){}
    > public:
    > static Singleton& getInstance();
    > };
    >
    > //Singleton.cpp
    > #include "Singleton.h"
    > Singleton Singleton::instance;
    >
    > Singleton& Singleton::getInstance()
    > {
    > return instance;
    > }
    >
    > Since am maintaining a static instance this should be created only
    > once irrespective of how many threads are accessing the getInstance()
    > function at the same time. So is this code not thread safe?


    It is not directly related to thread safety, but your code has the
    potential of running into C++'s static initialisation fiasco: If you
    have objects with static lifetime and namespace scope (or as static
    members of a class), and these objects are defined in different source
    files, then it is unspecified in which order these objects are
    initialised.
    This becomes a problem if these objects can refer to each other.

    For example, if you have another source file with this code:

    #include "Singleton.h"
    struct SingletonUser {
    SingletonUser() { Singleton::getInstance().doSomething(); }
    };

    static SingletonUser su;

    Then there is a significant chance that Singleton::getInstance() gets
    called BEFORE Singleton::instance was initialised.

    This problem can usually be prevented by writing getInstance like this:
    Singleton& Singleton::getInstance()
    {
    static Singleton instance;
    return instance;
    }

    But that has potential threading issues, if the first call can occur
    from multiple threads and the compiler does not add sufficient
    protection.

    >
    > Cheers


    Bart v Ingen Schenau
    --
    a.c.l.l.c-c++ FAQ: http://www.comeaucomputing.com/learn/faq
    c.l.c FAQ: http://c-faq.com/
    c.l.c++ FAQ: http://www.parashift.com/c -faq-lite/
     
    Bart van Ingen Schenau, Jun 26, 2009
    #2
    1. Advertising

  3. Udit Sharma

    James Kanze Guest

    On Jun 26, 12:50 pm, Udit Sharma <> wrote:

    > I was informed by my colleague that the following code is not
    > thread safe but I still don't know why because his arguments
    > weren't very convincing.


    > //Singleton.h
    > class Singleton
    > {
    > private:
    > static Singleton instance;
    > Singleton(){}
    > Singleton(const Singleton&){}
    > Singleton& operator=(Singleton const&){}
    > public:
    > static Singleton& getInstance();
    > };


    > //Singleton.cpp
    > #include "Singleton.h"
    > Singleton Singleton::instance;


    > Singleton& Singleton::getInstance()
    > {
    > return instance;
    > }


    > Since am maintaining a static instance this should be created
    > only once irrespective of how many threads are accessing the
    > getInstance() function at the same time. So is this code not
    > thread safe?


    It depends. Principly, it's not safe from order of
    initialization problems, which means that using the singleton in
    the constructors of static objects is problematic. If one of
    these constructors starts additional threads, there may be a
    threading problem as well.

    --
    James Kanze (GABI Software) email:
    Conseils en informatique orientée objet/
    Beratung in objektorientierter Datenverarbeitung
    9 place Sémard, 78210 St.-Cyr-l'École, France, +33 (0)1 30 23 00 34
     
    James Kanze, Jun 26, 2009
    #3
  4. On Jun 26, 3:50 am, Udit Sharma <> wrote:
    > Hi,
    >
    > I was informed by my colleague that the following code is not thread
    > safe but I still don't know why because his arguments weren't very
    > convincing.
    >
    > //Singleton.h
    > class Singleton
    > {
    > private:
    > static Singleton instance;
    > Singleton(){}
    > Singleton(const Singleton&){}
    > Singleton& operator=(Singleton const&){}
    > public:
    > static Singleton& getInstance();
    >
    > };
    >
    > //Singleton.cpp
    > #include "Singleton.h"
    > Singleton Singleton::instance;
    >
    > Singleton& Singleton::getInstance()
    > {
    > return instance;
    >
    > }
    >
    > Since am maintaining a static instance this should be created only
    > once irrespective of how many threads are accessing the getInstance()
    > function at the same time. So is this code not thread safe?


    As Bart van Ingen Schenau and James Kanze have said, it's thread-safe
    as is, but there's the static initialization order problem. Generally,
    you solve this problem either with library support ala pthread_once,
    or you make your singleton instance be a function local static to
    "initialize on demand" to solve the static initialization order
    problem, and then you make a global whose sole purpose is to
    initialize the singleton before main(), and thus presumably before
    there are threads. (If there are nontrivial threads before main, you
    cannot solve this problem with mutexes alone in C++. You would need
    the power of pthread_once or equivalent.)

    Ex:

    //hpp
    class singleton
    {
    public:
    static singleton& getSingleton();
    };

    //cpp
    singleton& singleton::getSingleton()
    { static singleton * x = new singleton;
    return *x;
    }
    namespace { bool forceInit = (singleton::getSingleton(), true); }


    This will generally ensure a single initialization of a singleton in a
    thread safe way, and avoid the static order initialization problem.
    (At least under the assumption that there are no threads before main.
    If there are nontrivial threads before main, see pthread_once, and
    good luck for non-posix systems like windows. I haven't looked into
    it.) This will in no way guarantee thread-safe access to the
    singleton; if the singleton is changeable you still need to worry
    about guarding access such as with a mutex inside the singleton
    instance. (Specifically do not make the mutex another static member of
    the class, because then you run into how to safely initialize the
    mutex and you hit the exact same issues as above: namely static
    initialization order and thread safety of construction.)
     
    Joshua Maurice, Jun 26, 2009
    #4
  5. * Joshua Maurice:
    > On Jun 26, 3:50 am, Udit Sharma <> wrote:
    >>
    >> I was informed by my colleague that the following code is not thread
    >> safe but I still don't know why because his arguments weren't very
    >> convincing.
    >>
    >> //Singleton.h
    >> class Singleton
    >> {
    >> private:
    >> static Singleton instance;
    >> Singleton(){}
    >> Singleton(const Singleton&){}
    >> Singleton& operator=(Singleton const&){}
    >> public:
    >> static Singleton& getInstance();
    >>
    >> };
    >>
    >> //Singleton.cpp
    >> #include "Singleton.h"
    >> Singleton Singleton::instance;
    >>
    >> Singleton& Singleton::getInstance()
    >> {
    >> return instance;
    >>
    >> }
    >>
    >> Since am maintaining a static instance this should be created only
    >> once irrespective of how many threads are accessing the getInstance()
    >> function at the same time. So is this code not thread safe?

    >
    > As Bart van Ingen Schenau and James Kanze have said, it's thread-safe
    > as is,


    AFAICS neither of them said that, and anyway that's wrong.

    The code above is not generally thread-safe.

    Assuming no shared libraries in the picture the creation of the singleton is
    however thread safe if no threads are started before the first statement of
    'main' is executed, because at that point the the initialization's finished.


    > but there's the static initialization order problem. Generally,
    > you solve this problem either with library support ala pthread_once,
    > or you make your singleton instance be a function local static to
    > "initialize on demand" to solve the static initialization order
    > problem, and then you make a global whose sole purpose is to
    > initialize the singleton before main(), and thus presumably before
    > there are threads.


    Yah.


    Cheers & hth.,

    - Alf

    --
    Due to hosting requirements I need visits to <url: http://alfps.izfree.com/>.
    No ads, and there is some C++ stuff! :) Just going there is good. Linking
    to it is even better! Thanks in advance!
     
    Alf P. Steinbach, Jun 26, 2009
    #5
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. =?Utf-8?B?V2lsbGlhbSBTdWxsaXZhbg==?=

    Thread safe singleton to access the cache?

    =?Utf-8?B?V2lsbGlhbSBTdWxsaXZhbg==?=, Oct 6, 2005, in forum: ASP .Net
    Replies:
    1
    Views:
    789
    Bruce Barker
    Oct 6, 2005
  2. Anand
    Replies:
    11
    Views:
    9,881
    Jacob
    Feb 10, 2006
  3. Angus
    Replies:
    6
    Views:
    444
    James Kanze
    Nov 15, 2007
  4. Gabriel Rossetti
    Replies:
    0
    Views:
    1,326
    Gabriel Rossetti
    Aug 29, 2008
  5. John Nagle
    Replies:
    5
    Views:
    473
    John Nagle
    Mar 12, 2012
Loading...

Share This Page