67
votes
\$\begingroup\$

Is this the best way to implement this pattern in C#?

public sealed class Singleton
{
    private static readonly Singleton instance = new Singleton();
    public static Singleton Instance { get { return instance; } }

    static Singleton() {}
    private Singleton() {}
}
\$\endgroup\$
2

9 Answers 9

74
votes
\$\begingroup\$

I use Jon Skeet's version of a thread safe Singleton with fully lazy instantiation in C#:

public sealed class Singleton
{
    // Thread safe Singleton with fully lazy instantiation á la Jon Skeet:
    // http://csharpindepth.com/Articles/General/Singleton.aspx
    Singleton()
    {
    }

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

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

        internal static readonly Singleton instance = new Singleton();
    }
}

It works wonders for me! It's really easy to use, just get the instance from the Instance property like so; SingletonName instance = SingletonName.Instance;

\$\endgroup\$
17
  • 5
    \$\begingroup\$ I think that Instance is sufficient enough, but I suppose YMMV. \$\endgroup\$
    – Zolomon
    Jan 20, 2011 at 12:08
  • 3
    \$\begingroup\$ The nested class is what makes it lazy. The runtime will not instantiate the Singleton instance until Singleton.Instance is called. That way, if you have other static members or something on Singleton, you can access them without creating the Singleton instance. \$\endgroup\$
    – Wes P
    Jan 20, 2011 at 15:55
  • 9
    \$\begingroup\$ I'd like to encourage everyone to read Jon Skeet's article (following the link Zolomon has provided), as it coverts almost all answers in this topic, listing all their pros and cons. \$\endgroup\$
    – ShdNx
    Jan 21, 2011 at 11:37
  • 2
    \$\begingroup\$ This is also thread safe. \$\endgroup\$ Jan 27, 2011 at 17:43
  • 7
    \$\begingroup\$ @Chris: You wouldn't. That's part of why singletons suck -- they're essentially hidden global state, making it nearly impossible to reliably test code that contains or uses them. \$\endgroup\$
    – cHao
    Oct 28, 2011 at 11:54
18
votes
\$\begingroup\$

I always use this, it allows lazy initialisation of generics, instead of creating a new singleton class for each type of singleton I want. It's also threadsafe but does not use locks on every access.

public static class Singleton<T> where T : class, new()
{
    private T instance = null;

    public T Instance
    {
        get
        {
            if (instance == null)
                Interlocked.CompareExchange(ref instance, new T(), null);

            return instance;
        }
    }
}

If you're unfamiliar with the interlocked class, it performs atomic operations in a manner which is often quicker than a lock. Three possible cases:

1) First access by a single thread. Probably roughly the same performance as the obvious method using a lock

2) First access by many threads at the same time. Many threads might enter the interlocked exchange, in which case several items may get constructed but only 1 will "win". So long as your constructor has no global side effects (which is really shouldn't) behaviour will be correct. Performance will be slightly less than a lock, because of multiple allocations, but the overhead is small and this is a very rare case.

3) Later accesses. No locking or interlocked operations, this is pretty much optimal and is obviously the majority case.

\$\endgroup\$
6
  • \$\begingroup\$ +1 for good solution, but the problem #2 requiring your Constructors to have no side effects has always scared me off this approach. \$\endgroup\$
    – GWLlosa
    Jan 21, 2011 at 3:59
  • 1
    \$\begingroup\$ How often do your constructors have side effects? I've always considered that to be a pretty bad code smell. Especially in multithreaded code! \$\endgroup\$
    – Martin
    Jan 21, 2011 at 13:41
  • \$\begingroup\$ Additionally, your constructor can have side effects so long as they're threadsafe and do not assume that this is the only instance to be created. Remember, several instances may be created but only one will "win", so if you do have side effects they'd better be implemented properly! \$\endgroup\$
    – Martin
    Jan 21, 2011 at 15:04
  • \$\begingroup\$ @Martin: Isn't the whole point of singletons that only one instance can be created? It'd seem to me that relying on that assumption is a quite reasonable thing to do... \$\endgroup\$
    – cHao
    Oct 28, 2011 at 11:58
  • 1
    \$\begingroup\$ -1, Not thread safe (constructor could be called several times), thus defeating the purpose of a singleton. \$\endgroup\$ Nov 19, 2012 at 7:53
12
votes
\$\begingroup\$

If you are using .NET 4.0, you can take advantage of the System.Lazy class. It ensures the instance is created only once.

\$\endgroup\$
2
9
votes
\$\begingroup\$

I use a pattern similar to one already posted, but with the following difference:

public sealed class Logging
{
    static Logging instance = null;
    static readonly object lockObj = new object();

    private Logging()
    {
    }

    public static Logging Logger
    {
        get
        {
            **if (instance == null)**
            {
                 lock (lockObj)
                 {
                     if (instance == null)
                     {
                         instance = new Logging();
                     }

                 }
            }
            return instance;
        }
    }

}

The reason being that it avoids calling lock every time, which can help you with performance. So you check for null, then, if it is null, lock it. Then you have to check for null again (because someone may have come in right that second) but then you should be ok. That way, you'll only hit the lock the first time (or two), and blow past it without locking the rest of the time.

\$\endgroup\$
3
  • \$\begingroup\$ Great example.I only have to add the name: it is called "Double-checked Locking" \$\endgroup\$
    – Oleg Rybak
    Jan 21, 2011 at 10:33
  • 1
    \$\begingroup\$ The return statement needs to be outside of the first if(instance==null) statement. Otherwise it will not return an instance if the instance already exists. \$\endgroup\$
    – Dan
    Jan 21, 2011 at 13:30
  • \$\begingroup\$ @Dan quite correct, my mistake. \$\endgroup\$
    – GWLlosa
    Jan 21, 2011 at 13:56
6
votes
\$\begingroup\$

I always use this solution if I have to implement a Singleton in C#

public sealed class Logging
    {
        static Logging instance = null;
        static readonly object lockObj = new object();

        private Logging()
        {
        }

        public static Logging Logger
        {
            get
            {
                lock (lockObj)
                {
                    if (instance == null)
                    {
                        instance = new Logging();
                    }
                   return instance;
                }
            }
        }

}

Compared to your solution this is threadsafe and uses a lazy-creation method. (The object is only instantiated if actually needed)

The eager vs. lazy creation isn't really worth a discussion in this example, but I would whenever possible use the thread-safe implementation.

\$\endgroup\$
3
  • 4
    \$\begingroup\$ This is a very bad solution to the problem, it uses a lock on every single access. Which has a very high overhead. \$\endgroup\$
    – Martin
    Jan 21, 2011 at 0:38
  • \$\begingroup\$ @Martin: The overhead is not that onerous unless you say Logging.Logger.DoThis() and Logging.Logger.DoThat() every time. Which i'd prefer to avoid that anyway, as (1) it pretty much entrenches the Logging class, making it even more of a pain to swap out for something else later, and (2) Logging.Logger will, by the definition of a singleton, always return the same thing, so re-getting it each time is silly. Oh, and (3) it's repetitive, and i hate repetition. \$\endgroup\$
    – cHao
    Oct 28, 2011 at 12:03
  • \$\begingroup\$ You're using a singleton now, but later you may wish to change your singleton so, for example, it swaps out to a new logger when the file gets full - so I tend to try to avoid caching the singleton. There's no point keeping hundreds of references to something around when you can centralise access in one place easily and efficiently with e.g. double checked locking \$\endgroup\$
    – Martin
    Oct 28, 2011 at 13:41
4
votes
\$\begingroup\$

Dependency Injection containers like Unity support a singleton concept. Unity is from Microsoft (and is open source), but there are lots of open source DI containers.

\$\endgroup\$
4
votes
\$\begingroup\$

Here's an example from Wikipedia's Double-checked locking page:

public class MySingleton
{
    private static object myLock = new object();
    private static MySingleton mySingleton = null;

    private MySingleton()
    { }

    public static MySingleton GetInstance()
    {
        if (mySingleton == null)    // check
        {
            lock (myLock)
            {
                if (mySingleton == null)    // double check
                {
                    MySingleton newSingleton = new MySingleton();
                    System.Threading.Thread.MemoryBarrier();
                    mySingleton = newSingleton;
                }
            }
        }

        return mySingleton;
    }
}

Notice the use of System.Threading.Thread.MemoryBarrier();, unlike GWLlosa's answer. See The "Double-checked locking is broken" declaration for an explanation. (It is written for Java, but the same principles apply.)

\$\endgroup\$
0
4
votes
\$\begingroup\$

Here's an implementation that uses Lazy<T>:

public class Foo : IFoo
{
    private static readonly Lazy<IFoo> __instance
        = new Lazy<IFoo>(valueFactory: () => new Foo(), isThreadSafe: true);

    private Foo() { }

    public static IFoo Instance { get { return __instance.Value; } }
}
\$\endgroup\$
0
votes
\$\begingroup\$

Simple way to I would like to suggest a simple way using volatile keyword , static variable are not thread safe but atomic execution are considered as thread safe. by using volatile we can enforce atomic operation.

public sealed class Singleton
{
    private static **volatile** readonly Singleton instance = new Singleton();
    public static Singleton Instance { get { return instance; } }
    private Singleton() { }
}

Locking is not the sure way to go , as it is expensive.

I like john skeet way of creating the singleton way of creating the objects.

\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.