What are the Dangers of using a Singleton in a multithreaded application

Question

I'm looking at using a singleton in a multithreaded Win service for doing logging, and wanted to know what are some of the problems I might encounter. I have already set up the get instance to handle syncing with

    private static volatile Logging _instance;
    private static object _syncRoot = new object();

    private Logging(){}
    public static Logging Instance
    {
        get
        {
            if (_instance==null)
            {
                lock(_syncRoot)
                {
                    if (_instance == null)
                    {
                        _instance = new Logging();
                    }
                }
            }
            return _instance;
        }
    }

Is there anything else I might need to worry about?

Answer 1

That looks pretty good to me.

See Implementing the Singleton Pattern in C# for more info.

Edit: Should probably put the return inside the lock, though.

Answer 2

This is more informational than anything else.

What you've posted is the double-checked locking algorithm - and what you've posted will work, as far as I'm aware. (As of Java 1.5 it works there, too.) However, it's very fragile - if you get any bit of it wrong, you could introduce very subtle race conditions.

I usually prefer to initialize the singleton in the static initializer:

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

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

    private Singleton()
    {
        // Do stuff
    }
}

(Add a static constructor if you want a bit of extra laziness.)

That pattern's easier to get right, and in most cases it does just as well.

There's more detail on my C# singleton implementation page (also linked by Michael).

As for the dangers - I'd say the biggest problem is that you lose testability. Probably not too bad for logging.

Answer 3

Singleton's have the potential to become a bottleneck for access to the resource embodied by the class, and force sequential access to a resource that could otherwise be used in parallel.

In this case, that may not be a bad thing, because you don't want multiple items writing to your file at the same instant, and even so I don't think your implementation will have that result. But it's something to be aware of.