A generic singleton

Question

What do you guys think about this for a generic singleton?

using System;
using System.Reflection;

// Use like this
/*
public class Highlander : Singleton<Highlander>
{
    private Highlander()
    {
        Console.WriteLine("There can be only one...");
    }
}
*/

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

    public static T GetInstance()
    {
        if (instance == null)
        {
            CreateInstance();
        }

        return instance;
    }

    private static void CreateInstance()
    {
        lock (initLock)
        {
            if (instance == null)
            {
                Type t = typeof(T);

                // Ensure there are no public constructors...
                ConstructorInfo[] ctors = t.GetConstructors();
                if (ctors.Length > 0)
                {
                   throw new InvalidOperationException(String.Format("{0} has at least one accesible ctor making it impossible to enforce singleton behaviour", t.Name));
                }

                // Create an instance via the private constructor
                instance = (T)Activator.CreateInstance(t, true);
            }
        }
    }
}

This locking technique is broken unless a volatile keyword is used for the locking object. See [this](http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html) — Sanjeevakumar Hiremath, Mar 03 '11 at 11:12

score 30 · Accepted Answer · edited Mar 25 '13 at 04:22

30

Creating a singleton class is just a few lines of code, and with the difficulty of making a generic singleton i always write those lines of code.

public class Singleton
{
    private Singleton() {}
    static Singleton() {}
    private static Singleton _instance = new Singleton();
    public static Singleton Instance { get { return _instance; }}
}

The

private static Singleton _instance = new Singleton();

line removes the need for locking, as a static constructor is thread safe.

edited Mar 25 '13 at 04:22

Alastair Pitts

19,423
9
68
97

answered Dec 19 '08 at 11:45

AndreasN

2,881
3
24
29

6

Exactly. Especially as the generic version doesn't actually give you a singleton anyway... – Jon Skeet Dec 19 '08 at 11:46
1

You should probably also add a static constructor. See http://www.yoda.arachsys.com/csharp/singleton.html – Winston Smith Dec 19 '08 at 11:48
Indeed. And for lazy loading, Jon's page covers that too with a nested class. – Marc Gravell Dec 19 '08 at 11:48
This locking technique is broken unless a volatile keyword is used for the locking object. See [this](http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html) – Sanjeevakumar Hiremath Mar 03 '11 at 11:12
I added the static constructor in-line with the linked Jon Skeet article. – Alastair Pitts Mar 25 '13 at 04:23

score 6 · Answer 2 · answered Dec 19 '08 at 11:43

6

Well, it isn't really singleton - since you can't control T, there can be as many T instances as you like.

(removed thread-race; noted the double-checked usage)

answered Dec 19 '08 at 11:43

Marc Gravell

1,026,079
266
2,566
2,900

score 5 · Answer 3 · answered Dec 19 '08 at 11:50

5

I've deleted my previous answer as I hadn't noticed the code which checks for non-public constructors. However, this is a check which is only performed at execution time - there's no compile-time check, which is a strike against it. It also relies on having enough access to call the non-public constructor, which adds some limitations.

In addition, it doesn't prohibit internal constructors - so you can end up with non-singletons.

I'd personally create the instance in a static constructor for simple thread safety, too.

Basically I'm not much of a fan - it's pretty easy to create singleton classes, and you shouldn't be doing it that often anyway. Singletons are a pain for testing, decoupling etc.

answered Dec 19 '08 at 11:50

Jon Skeet

1,421,763
867
9,128
9,194

1

Agreed; in most cases, a static class is more appropriate. Useful for implementing interfaces etc, but the standard pattern works fine without needing the generics. – Marc Gravell Dec 19 '08 at 12:04
2

Don't forget the link...Jon! :) http://www.yoda.arachsys.com/csharp/singleton.html – kenny Dec 19 '08 at 13:35
@kenny: That link's just what I needed. Thanks!! – fourpastmidnight Jul 20 '12 at 03:45

Alexandr · Answer 4 · 2012-05-16T12:52:29.390

5

This is my point using .NET 4

public class Singleton<T> where T : class, new()
    {
        Singleton (){}

        private static readonly Lazy<T> instance = new Lazy<T>(()=> new T());

        public static T Instance { get { return instance.Value; } } 
    }

and it's using is following:

   public class Adaptor
   {
     public static Adaptor Instance { get { return Singleton<Adaptor>.Instance;}}
   }

edited May 16 '12 at 12:52

answered Oct 25 '11 at 15:57

Alexandr

1,452
2
20
42

3

The issue with this approach is that it requires a public constructor for the singleton implementing class, which allows the possibility of usage outside the bounds of a singleton instance (i.e. compile time prevention not present here). – sweetfa Aug 22 '12 at 22:55
This is the cost of its generics. – Alexandr Aug 24 '12 at 10:29
1

Yep, just needs to be made obvious to the noob though – sweetfa Aug 24 '12 at 21:13
You should add a private constructor to Adaptor which breaks your design. – Wouter Aug 09 '19 at 21:42
This is the cost of it's generics – Alexandr Oct 22 '19 at 07:25

score 2 · Answer 5 · answered Mar 20 '13 at 02:17

Merging AndreasN answer and Jon Skeet's "Fourth version - not quite as lazy, but thread-safe without using locks" of a Singleton c# implementation, why don't use a code snippet to do all the hard work:

<CodeSnippets xmlns="http://schemas.microsoft.com/VisualStudio/2005/CodeSnippet">
    <CodeSnippet Format="1.0.0">
        <Header>
            <Title>Singleton Class</Title>
            <Author>TWSoft</Author>
            <Description>Generates a singleton class</Description>
            <SnippetTypes>
                <SnippetType>Expansion</SnippetType>
            </SnippetTypes>
            <Keywords>
                <Keyword>Singleton</Keyword>
            </Keywords>
            <Shortcut>singleton</Shortcut>
        </Header>
        <Snippet>
            <Declarations>
                <Literal>
                    <ID>ClassName</ID>
                    <ToolTip>Replace with class name</ToolTip>
                    <Default>MySingletonClass</Default>
                </Literal>
            </Declarations>

            <Code Language="CSharp">
                <![CDATA[
                public class $ClassName$
                {
                    #region Singleton
                    static readonly $ClassName$ mInstance = new $ClassName$();

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

                    private $ClassName$()
                    {
                    }

                    public static $ClassName$ Instance
                    {
                        get { return mInstance; }
                    }
                #endregion
                }
                ]]>
            </Code>
        </Snippet>
    </CodeSnippet>
</CodeSnippets>

Then you can save this into a .snippt file, and add it to VS IDE (Tools->Code Snippets Manager)

Wouter · Answer 6 · 2019-08-09T21:38:56.037

Using generics is not useful for singletons. Because you can always create multiple instances of the type parameter T and then it is not by definition a singleton.

Look at this:

public sealed class Singleton<T> where T : class, new()
{
    private static readonly Lazy<T> instance = new Lazy<T>(() => new T());
    public static T Instance => instance.Value;
    private Singleton() { }
}

And when you use it like this

public class Adapter
{
    public static Adapter Instance => Singleton<Adapter>.Instance; 
    // private Adapter(){ } // doesn't compile.
}

you can still create Adapters yourself just call

new Adapter();

Adding the private constructor to Adapter breaks the code. Note that this Adapter uses composition not inheritance. I.e. it is not a Singleton it has a Singleton. If something is a Singleton it should derive from or implement an interface.

public abstract class Singleton<T> 
{
    protected static Lazy<T> instance;
    public static T Instance => instance.Value;
}

public sealed class Adapter : Singleton<Adapter>
{
    static Adapter()
    {
        instance = new Lazy<Adapter>(() => new Adapter());
    }

    private Adapter() { }
}

Basically this only moves a static field into a generic base class while it is no longer readonly and therefore can be changed after initialization. Also it requires you to remember to add the private constructor, mark it as sealed and perform some initialization so it still isn't well encapsulated and prone to mistakes.

We can improve this by adding a check in the base constructor.

public abstract class Singleton<T> where T : Singleton<T>, new()
{
    private static bool instantiated;
    private static readonly Lazy<T> instance = new Lazy<T>(() => new T());
    public static T Instance => instance.Value;
    protected Singleton()
    {
        if (instantiated)
            throw new Exception();
        instantiated = true;
    }
}

public /* sealed */ class Adapter : Singleton<Adapter>
{
}

Due to the fact that the Singleton is lazy the pattern is still broken.

  new Adapter(); // this works
  Adapter.Instance; // this throws an error.

also

  Adapter.Instance; // this works
  // just running in production someone decided to call:
  new Adapter(); // this throws an error

These kind of error's can only be detected during testing.

I would still prefer to just use the pattern directly it's basically just 3 lines of code and it is compile time safe. Also when you need a different base classes but still need a lazy singleton and require it to be a true singleton a simple solution exists: (see: http://csharpindepth.com/Articles/General/Singleton.aspx)

public sealed class Adapter
{
    private static readonly Lazy<Adapter> instance = new Lazy<Adapter>(() => new Adapter());
    public static Adapter Instance { get { return instance.Value; } }
    private Adapter() { }
}

You cannot refactor this properly into a generic singleton that cannot be misused like the above investigated approaches.

A generic singleton

6 Answers6

Linked