Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

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() {}
}
share

locked by Jamal Dec 10 '13 at 0:46

This question exists because it has historical significance, but it is not considered a good, on-topic question for this site, so please do not use it as evidence that you can ask similar questions here. This question and its answers are frozen and cannot be changed. More info: help center.

8  
Singleton is an antipattern. –  Arjang May 7 '12 at 2:53
    
Please watch: "Global State and Singletons" –  tereško Sep 10 '12 at 15:00
comments disabled on deleted / locked posts

9 Answers 9

up vote 75 down vote accepted

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;

share
5  
I think that Instance is sufficient enough, but I suppose YMMV. –  Zolomon Jan 20 '11 at 12:08
3  
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. –  Wes P Jan 20 '11 at 15:55
9  
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. –  ShdNx Jan 21 '11 at 11:37
2  
This is also thread safe. –  David Basarab Jan 27 '11 at 17:43
7  
@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. –  cHao Oct 28 '11 at 11:54
show 12 more comments

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.

share
    
+1 for good solution, but the problem #2 requiring your Constructors to have no side effects has always scared me off this approach. –  GWLlosa Jan 21 '11 at 3:59
1  
How often do your constructors have side effects? I've always considered that to be a pretty bad code smell. Especially in multithreaded code! –  Martin Jan 21 '11 at 13:41
    
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! –  Martin Jan 21 '11 at 15:04
    
@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... –  cHao Oct 28 '11 at 11:58
1  
-1, Not thread safe (constructor could be called several times), thus defeating the purpose of a singleton. –  Konrad Rudolph Nov 19 '12 at 7:53
show 1 more comment

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

share
5  
You should add a code sample since I believe this is by far the best way to implement a singleton with .NET 4.0. –  Scott Lerch Jun 22 '12 at 1:05
    
A code sample is available at csharpindepth.com/Articles/General/Singleton.aspx ("sixth version") –  Morawski Dec 9 '13 at 9:30
add comment

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.

share
    
Great example.I only have to add the name: it is called "Double-checked Locking" –  Oleg Rybak Jan 21 '11 at 10:33
1  
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. –  Dan Jan 21 '11 at 13:30
    
@Dan quite correct, my mistake. –  GWLlosa Jan 21 '11 at 13:56
add comment

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.

share
4  
This is a very bad solution to the problem, it uses a lock on every single access. Which has a very high overhead. –  Martin Jan 21 '11 at 0:38
    
@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. –  cHao Oct 28 '11 at 12:03
    
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 –  Martin Oct 28 '11 at 13:41
add comment

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.

share
add comment

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.)

share
add comment

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; } }
}
share
add comment

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.

share
add comment

protected by Jamal Dec 10 '13 at 0:46

Thank you for your interest in this question. Because it has attracted low-quality answers, posting an answer now requires 10 reputation on this site.

Would you like to answer one of these unanswered questions instead?

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