0

I am currently working on a project where I have to create several singleton instances of different types. I know that creating singletons for each type can be a bit tedious and error-prone, so I wanted to implement a generic singleton factory in C#. However, I am a bit unsure about the best way to accomplish this task.

The objective is to have a method which can create a singleton instance of any given type. Here is a simple code that I have so far:

public class SingletonFactory
{
    private static readonly Dictionary<Type, object> instances = new Dictionary<Type, object>();

    public static T GetInstance<T>() where T : class, new()
    {
        if (!instances.ContainsKey(typeof(T)))
        {
            instances[typeof(T)] = new T();
        }
        return (T)instances[typeof(T)];
    }
}

This is a minimalist version, and I understand that it does not take into account things like thread safety and disposing.

So, my questions are:

  1. How can I enhance the above code to make it thread-safe?
  2. Is there any better approach to implement this pattern which would also take into account the disposal of instances when they are no longer needed?
  3. Is there a way to make this pattern work if the class that needs to be instantiated has some parameters in its constructor?

I appreciate any help or advice that you can provide. Thanks!

Charlieface
  • 52,284
  • 6
  • 19
  • 43
BotMaster3000
  • 472
  • 4
  • 16
  • Your DI container should do this as close to the root as possible. Use middleware to create your singletons. – GH DevOps Jul 06 '23 at 11:39

1 Answers1

0
  1. Thread safety. What prevents you from just wrapping all GetInstance code into a lock operator block, locking instances? If you don't want a deadlock, you could use Monitor directly:
using System;
using System.Threading;

// ...

private const int timeout = 1000;  // Timeout for acquiring a lock in milliseconds

// Standard constructor not necessary, see next paragraph
public static T GetInstance<T>() where T : class
{
  bool lockTaken;
  Monitor.TryEnter(instances, timeout, ref lockTaken);

  if (lockTaken) {
    try {
      // Singleton factory code
    }
    finally { Monitor.Exit(instances); }
  } else {
    throw new SystemException(/* "Go wait I'm busy" */ "Lock timeout expired.");
  }
}

(Why I said "directly"? A lock operator is translated into a Monitor.Enter call by the compiler.)

  1. Passing parameters to constructor. Reflection helps you. Don't forget to add an object[] parameter to GetInstance called parameters or whatever you want.
instances[typeof(T)] = Activator.CreateInstance(typeof(T), parameters);  // Instead of "new T()"
  1. Disposal. To be disposed properly, a class should either have a destructor or implement IDisposable. If it has destructor, calling it won't be your work but the garbage collector's. In case it implements IDisposable, only the client code knows when to call Dispose, so the right way to handle that would be creating a method like this:
public static void DisposeInstance<T>() where T : class {
  if (!instances.ContainsKey(typeof(T))) {
    throw new ArgumentException("Instance not found");
  }

  var interfaces = typeof(T).FindInterfaces(((type, criteria) => type == typeof(IDisposable)), null);
  if (interfaces.Length == 0) {
    throw new ArgumentException("Specified instance does not implement IDisposable");
  }

  typeof(T).InvokeMember("Dispose", BindingFlags.InvokeMethod, null, instances[typeof(T)], new object?[] { });
}
  1. Exceptions. The constructor of T can easily throw an exception. Dispose (in an awfully designed class) can throw one, too. So it'd be nice to wrap both methods into a try block with a catch like this:
catch (Exception ex) {
  throw new ApplicationException(
    "Constructor threw an exception. See inner exception for details.", /* Or "Method \"Dispose\" threw an exception" */
    ex);
}
SNBS
  • 671
  • 2
  • 22