views:

303

answers:

8

Friends I have the following class that

class Singleton
{
  private:

    static Singleton *p_inst;
    Singleton();

  public:

    static Singleton * instance()
    {
      if (!p_inst)
      {
        p_inst = new Singleton();
      }

      return p_inst;
    }
};

Please do elaborate on precautions taken while implementing Singleton in multi-threaded environment ..

Many thanks

+4  A: 

You will have to use a mutex and lock the pointer before assigning or reading it, making this a slow (and imo just plain terrible) design pattern.

DeadMG
+1: My thoughts, exactly.
ereOn
+4  A: 

In multi-threading that clause

if(!p_inst)
{
    p_inst = new Singleton();
}

is actually 3 separate actions. You are getting the value of p_inst, setting the value of p_inst and writing the value of p_inst. So get-set-write means that you need to put a lock around p_inst otherwise you can have 2 threads which create a Singleton value that each thread uses.

Here is how you can view the issue, assume that your Singleton has a mutable field val:

thread A -> p_inst is NULL
    thread B -> p_inst is NULL
       thread A -> set to Singleton (1)
           thread B -> set to Singleton (2)
              thread C -> p_inst is Singleton (2)
                  thread A -> set val to 4
                      thread B -> set val to 6
                         thread C -> get val (it's 6)
                             thread A -> get val (it's 4!!)

You see? There's 2 copies of a Singleton floating about, neither of which knows about the other. The third thread which checks on the Singleton is only going to see the last assignment. But with locking, you can prevent multiple assignment and these types of problems.

wheaties
Is it something like ?static Singleton* instance() { if ( ! p_inst ) { Mutex m; m.lock(); // Critical section if ( ! p_inst ) p_inst = new Singleton(); // End critical section m.unlock(); } return p_inst; }
ronan
I understand that by usng mutex and lock it becomes a slow design patter but then is there any way to optimize it ?
ronan
@ronan: Yes, don't use this anti-pattern that is "singleton".
ereOn
You can just use a static variable instead of a pointer. It's not "lazy" but it will avoid your construction issue. On the other hand, never make a singleton mutable unless you know what you're doing. Reserve it for things like Socket pools, thread pools, and other OS dependent resources not under control of your application.
wheaties
@wheaties .. very good explanation
ronan
@ronan: This article explains why the approach you suggest isn't safe. http://www.drdobbs.com/184405726
Mike Seymour
+1  A: 

For multithreaded construction, use static variable in an instance() function. Initialization of static variables is automatically protected by the compiler. Any other operations require explicit locking. Use mutexes.

class Singleton
{
  private:

    Singleton();

  public:

    static Singleton * instance()
    {
      static Singleton inst;
      return &inst;
    }
};
Basilevs
I have to understand can I not use the keyword volatile ?How does it help ?
ronan
There are no guarantees from the pre-C++0x standard that the initialization of static variables within a method are protected from race conditions. Certain C++ compilers might make this safe, but many does not.
Cwan
A: 

You should check out Jon Skeet's singleton. Lazy init AND threadsafe. I know it's in C#, but you should get the idea.

http://www.yoda.arachsys.com/csharp/singleton.html

Alex Rouillard
Problem is that it's not portable to C++.
Matthieu M.
+1  A: 

There's a C++ Singleton implementation in the Wikipedia singleton article

David
Thanks everybody for all your answers :)
ronan
More accurately, that article has three failed attempts to implement a singleton in C++.
Mike Seymour
+1  A: 

You should ask yourself what you mean by thread safety.

  • Does your singleton actually need thread safety?

    If not, consider a thread-static approach

  • Do you want to guarantee that no two instances of the singleton ever get created?

    If not, your above solution is probably fine, without any locking: you've got a race condition on construction - but you don't care since eventually only one will survive - however, you may have a resource leak unless you're careful, which may or may not be significant. (This is essentially a cache).

  • Do you want to guarantee that eventually only one instance remains?

  • Do you care about locking costs?

    If not (which is quite common), you can just put a lock around it and be happy.

A singleton is a pattern that can address various problems - but what flavor of thread-safety is required has little to do with the singleton pattern itself and everything to do with what you want it for.

Eamon Nerbonne
@Eamon :- Thanks for the explanation. I will have to get Static Thread approach known here, basically I want Singleton be be thread safe.
ronan
+2  A: 

I will be brief: it depends on your compiler.

  • If your compiler implements multi-threading synchronization for local static (ie static instances embedded in a method), then use it and be done with it.
  • If not, Herb Sutter proved it was impossible.

Now, you have to realize that you may not need this.

There are 2 ways to deal with this, that do not require any multithread awareness.

  1. Simply use a static instance instead of dynamically allocate it. Safe and simple. May cause issue with initialization order if you need to access it from another static variable
  2. Create the singleton instance BEFORE having more than one thread. The usual trick is to call it from main.

Of course, the real question is: can't you just pass a reference to the object rather than creating a global variable ? It would make testing easier ;)

Matthieu M.
A: 

You can eliminate all issues by simply allocating (any way you choose) such objects before multiple threads are started. This many not always be possible due to design constraints (using the singletons in statics, you NEED lazy allocation, etc.), but it is simple and gives you control of the creation sequence. Sometimes tracking down issues with regard to order and time of allocation of such objects is a hassle that you can easily avoid.

P.S. - I know that this doesn't directly answer your question, but it may be a practical solution to a real problem without complexity.