Singleton pattern confusion

Question

As per the singleton pattern,

public sealed class Singleton
{
    static Singleton instance=null;

    Singleton()
    {
    }

    public void abc(){
    }

    public static Singleton Instance
    {
        get
        {
            if (instance==null)
            {
                instance = new Singleton();
            }
            return instance;
        }
    }
}

the above is not thread-safe.Two different threads could both have evaluated the test if (instance==null) and found it to be true, then both create instances, which violates the singleton pattern.

Confusion is Instance is static, how this can be null once it is called on UI thread or other threads?

EDIT

I meant to said that once i have called Singleton.Instance.abc(); Singleton.Instance should not be null until it is disposed manually. Right?

Answer 1

Control is passed to ThreadA

ThreadA trys to get the Instance, it is found to be null.

Control is passed to ThreadB

ThreadB trys to get the Instance, it is found to be null.

Control is passed to ThreadA

ThreadA instantiates Instance.

Control is passed to ThreadB

ThreadB re-instantiates Instance.

Solution: You an use a static constructor to ensure this doesn't happen.

Answer 2

You are correct the singleton you shown is not thread safe. The trivial way in making it thread safe is:

public sealed class Singleton
{
    static Singleton instance=null;
    static lockObject = new object();
    Singleton()
    {
    }

    public static Singleton Instance
    {
        get
        {
            lock(lockObject)
            {
               if (instance==null)
               {
                   instance = new Singleton();
               }
            }
            return instance;
        }
    }
}

You can write a version thread safe without lock, leveraging how C# uses statics ( http://www.yoda.arachsys.com/csharp/singleton.html )

public sealed class Singleton
{
    static readonly Singleton instance=new Singleton();

    // Explicit static constructor to tell C# compiler
    // not to mark type as beforefieldinit
    static Singleton()
    {
    }

    Singleton()
    {
    }

    public static Singleton Instance
    {
        get
        {
            return instance;
        }
    }
}

The last version leverages the fact that static constructors in C# are specified to execute only when an instance of the class is created or a static member is referenced, once per AppDomain.

Answer 3

Edit:

I meant to said that once I have called Singleton.Instance.abc(); Singleton.Instance should not be null until it is disposed manually. Right?

Once a single thread has been used to access the Singleton it will already be initialized, and any threads you kick off after that call can safely access the Singleton without it ever being null.

If you kick off multiple threads that access the Singleton at the same time, and leave your code as-is, you will have a problem. If you don't use thread synchronization (lock) to make sure the if null block is entered only one-thread-at-a-time, then you would have a race condition.

If multiple threads sneak by the if block, then multiple threads would try to initialize instance. This is messy, and it is hard to tell what exactly will happen. One problem is that you could actually get multiple instances, one for each thread that snuck through. Any subsequent calls (after the race condition) would get the last instance created.

This may not be the worst problem you will see, though. There might be even worse problems that cause crashes (not sure). It is best to use thread-safe operations for your Singleton rather than finding out :)

See my original answer below for how to make your Singleton thread-safe:

---

You can create the instance in a static constructor.

The static constructor will only be called on first access, so you wouldn't need your lazy initialization code.

public sealed class Singleton
{
    private static Singleton instance;

    private Singleton() { }

    static Singleton()
    {
        instance = new Singleton();
    }

    public static Singleton Instance
    {
        get { return instance; }
    }
}

According to the answer to this question, it is guaranteed to be called only once, and is thread safe:

Static constructors are guaranteed to be run only once per application domain, before any instances of a class are created or any static members are accessed. http://msdn.microsoft.com/en-us/library/aa645612.aspx

The implementation shown is thread safe for the initial construction, that is, no locking or null testing is required for constructing the Singleton object. However, this does not mean that any use of the instance will be synchronised. There are a variety of ways that this can be done; I've shown one below.

Answer 4

Mr Skeet has an article about this on his website.

http://csharpindepth.com/Articles/General/Singleton.aspx

EDIT

Ok, so the question has been asked, how can a static call create 2 instances when static means there's only 1.

When executing on separate threads the code is shared, but the state of the code isn't. In this case it's the execution stack that's important. Here's an example.

Thread 1 and Thread 2 both execute the Instance getter at the same time.

Thread 1 evaluates if (instance==null) as true and branches into the if code.
Thread 2 evaluates if (instance==null) as true and branches into the if code.

Thread 1 now sets instance to a new Singleton object.
Thread 1 then returns instance.

Thread 2 now sets instance to a new Singleton object. Remember, thread 2 had already evaluated the condition earlier and had branched into this code.
Thread 2 then returns instance.

In this scenario we have returned 2 instances of Singleton.

Answer 5

There's only one moment where it won't be thread-safe: If it's called in two different threads running at the same time while not being initialized before.

Both threads will run Instance() and then it might happen that both evaluate instance==null before either one is able to assign the new reference.

To avoid this, use Shadow Wizard's approach or ensure it's called once before you start multiple threads (which, of course, is a lot more complicated and error prone).

Answer 6

Your example is not thread safe, but the probability of actually getting a race condition is very slim, so in many cases it would not matter.

If thread 1 evaluates instance, and thread 2 follows right behind, and evaluates instance before thread 1 has completed creating the singleton object and assigned it to instance, thread 2 will still create a new object, and assign it to instance.

Since the most common use case for a singleton is some sort of settings or configuration object, this will not matter (it will not cause your application to fail). There are other use cases though, where it could matter.

Edit: In answer to your edit: The first instance can be disposed by the GC once thread 2 created the second instance and assigned it to the instance variable of your singleton class, since then the first instance will not be referenced by your application any more.