views:

3100

answers:

9

I have a singleton that uses the "static readonly T Instance = new T();" pattern. However, I ran into a case where T is disposable, and actually needs to be disposed for unit tests. How can I modify this pattern to support a disposable singleton?

The interface I would like is something like:

var x = Foo.Instance;
var y = Foo.Instance; // x == y
...
x.Release(); // this causes the next Foo.Instance to return a fresh object
             // also, it assumes no further operations on x/y will be performed.

Note - the pattern has to be thread-safe, of course.

Edit - for the purpose of production code, this is a true singleton. The thing is that it locks some files, and so for cleanup in unit tests we have to dispose it.

I would also prefer a pattern that can be reused, if possible.

+8  A: 

At that point I don't think I'd really consider it to be a singleton any more, to be honest.

In particular, if a client uses a singleton they're really not going to expect that they have to dispose of it, and they'd be surprised if someone else did.

What's your production code going to do?

EDIT: If you really, really need this for unit tests and only for unit tests (which sounds questionable in terms of design, to be frank) then you could always fiddle with the field using reflection. It would be nicer to work out whether it should really be a singleton or whether it should really be disposable though - the two very rarely go together.

Jon Skeet
It's an object that I would like to have only one in my system, and that holds some unmanaged resources (e.g. files).
ripper234
Personally I'd pass it in to the things which need it, avoiding the problem to start with. The singleton pattern is severely limited when it comes to this kind of thing - it's a test killer. But given the design as cast in stone, go with the hacky reflection solution to replace the instance.
Jon Skeet
A: 

Consider @Jon Skeet's article http://www.yoda.arachsys.com/csharp/singleton.html

kenny
How does that work? The article doesn't even have the word Dispose in it, and the interface is not as I described.
ripper234
see my first answer if you need to see how it works
Bogdan Maxim
A: 

You could use a nested lazy singleton (See here) with some simple modifications:

public sealed class Singleton : IDisposable
{
    Singleton()
    {
    }

    public static Singleton Instance
    {
        get
        {
            if (!Nested.released)
                return Nested.instance;
            else
                throw new ObjectDisposedException();
        }
    }

    public void Dispose()
    {
         disposed = true;
         // Do release stuff here
    }

    private bool disposed = false;

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

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

Remember to throw ObjectDisposedException in all public methods/properties of the object if it has been disposed.

You should also, provide a finalizer method for the object, in case Dispose doesn't get called. See how to correctly implement IDisposable here.

Bogdan Maxim
A: 

If the class implements IDisposable (as you imply it does) then just call x.Dispose()

Jon Grant
A: 
 public class Foo : IDisposable
  { [ThreadStatic] static Foo _instance = null;

    private Foo() {IsReleased = false;}

    public static Foo Instance
     { get
        { if (_instance == null) _instance = new Foo();
          return _instance;
        }
     }

    public void Release()
     { IsReleased = true;
       Foo._instance = null;
     }

    void IDisposable.Dispose() { Release(); }

    public bool IsReleased { get; private set;}

  }
Mark Cidade
A: 

For unit tests you could use a "manual" instance (but you would need a way to instantiate the object).

In your case, probably you should better use the factory pattern (abstract/method - whichever is the best for your case), combined with a singleton.

If you want to test if the singleton has disposed properly of the used objects (in unit test), then use the Factory method, otherwise use the singleton pattern.

By the way, if you don't have access to the singleton source code or you are not allowed to modify it, you would better wrap it to another singleton, and provide all the logic from the new one (more like a proxy). It sounds like overkill, but it could be a viable solution.

Also, in order to be able to control the access to it, provide a factory, and let the clients get the new object only if the object hasn't been disposed.

Bogdan Maxim
+2  A: 

Singletons should not be Disposable. Period. If someone calls Dispose prematurely, your application is screwed until it restarts.

jezell
+3  A: 

Mark Release as internal and use the InternalsVisibleTo attribute to expose it only to your unit testing assembly. You can either do that, or if you're wary someone in your own assembly will call it, you can mark it as private and access it using reflection.

Use a finalizer in your singleton that calls the Dispose method on the singleton instance.

In production code, only the unloading of an AppDomain will cause the disposal of the singleton. In the testing code, you can initiate a call to Release yourself.

Omer van Kloeten
A: 

Another option to make a disposable Singleton is to use SandCastle's [Singleton] atribute for your class, then Castle framework takes care of disposing all disposable Singleton objects

Boris Lipschitz