CODEWITHSUNDEEP
c#.netdesign-patternsthread-safetysingleton
Mykhailo Seniutovych
Mykhailo Seniutovych

Reputation: 3711

What are the dangers of non thread safe singleton?

Recently I found this article that explains how to implement a thread safe singleton pattern in C#. However I wonder how important it is to make your singleton thread safe what and what are the dangers of having your singleton class non thread safe. Consider a very simple, non thread safe, implementation of a singleton pattern:

public class ThreadNotSafeSingleton
{
    public static ThreadNotSafeSingleton Instance
    {
        get
        {
            if (_instance == null)
            {
                _instance = new ThreadNotSafeSingleton();
            }

            return _instance;
        }
    }

    private static ThreadNotSafeSingleton _instance;

    private ThreadNotSafeSingleton()
    {
    }
}

Now let's say, due to the fact that this code is non thread safe, two threads will enter this line of code _instance = new ThreadNotSafeSingleton();. In this case, the first thread will initialize _instance field and then the second one will initialize _instance field again, any way as a result you will have a single _instance existing in your application.

So what is the big deal here then? I know, if you have more complex code, and your constructor runs some other code that may not be thread safe it may be dangerous. But are there any problems with non thread safety if your singleton is that simple?

Upvotes: 1

Views: 554

Answers (2)

user1781290
user1781290

Reputation: 2874

Since the singleton pattern restricts the class to only one instance, your approach violates that pattern.

If for any reason this is ok for your implementation, and we follow your stated example, there is a very high chance that each of the two concurrent accesses will get another instance of ThreadNotSafeSingleton. This may happen due to optimization if the compiler decides that there is no need to read back the _instance variable it just wrote before returning. This optimization behaviour is defined by the memory model of your C# implementation.

The volatile keyword is often cited as a possible solution, but it will not solve the synchronization issue (as pointed out by BionicCode), when thread 1 passes the if (_instance == null)line and gets put to sleep, then thread 2 also evaluates the same if and gets to instanciate the singleton. When thread 1 wakes up later, it will then instanciate another singleton.

Upvotes: 3

user6755993
user6755993

Reputation:

The purpose of the Singleton pattern is to create just and only just one object from a Type.

Imagine thread1 and thread2 come to the get accessor of the Instance property for the first time (_instance is still null) at the same time:

thread1 checks the if (_instance == null) expression and sees that _instance is null, and at the same time, thread2 checks the if expression and sees that _instance is null yet too. thread1 comes to this _instance = new ThreadNotSafeSingleton(); expression and creates a new object from the ThreadNotSafeSingleton type. Because thread2 checks the if expression and sees that _instance is null too, comes to the if code block and creates a new object that will overwrite the reference of the _instance variable. Finally, thread1 uses an object that is different from thread2's object.

This problem occurs when _instance is null and two or more threads try to get an instance of your Type at the same time. You must restrict simultaneous access to the if code body for threads (thread synchronization), especially when your _instace is null yet.

Upvotes: 3

Related Questions