IDisposable in Singleton-- Good Practice?

Question

I have a Singleton class that manages the connection to an external device. The idea of my application is that I need that external device to be presented all the time when the application is alive.

The Singleton has the following functionalities:

1. Initialization, look for the device at the application startup time
2. communicate with the external device, note that this can spread over multiple assemblies at multiple points.
3. Close the connection when the application exits.

For the last part, I am thinking about putting the code inside Dispose method of the singleton class, to guarantee that the resource is always cleanup upon closing. But since I am using a Singleton, and since singleton life span will only be terminated when the application exits, there is no need to explicitly close the connection in Dispose since the connection will be close anyway.

So, the question is, should I put the close connection code inside Dispose method?

Answer 1

IDisposable != Destructor like C++, it has to be called explictly.

Anyway, having a Disposable Singleton does not make much sense, IMO. What if some code actually did Dispose off your Singleton? What happens if some other code tries to access that singleton now? You probably should not put the connection close there for that reason.

In fact, you should not even have the Singleton implement IDisposable in the first place.

A better option might be to have the Singleton hook onto one of the .NET exit events. Perhaps use Application.ApplicationExit (or Application.Exit) event, or something similar.

Answer 2

Implementing IDisposable alone doesn't mean Dispose will be called on exit.

Not enough details to understand your application but if closing incorrectly leaves the device in a bad state than maybe a try/finally or as Moron suggested Application.Exit (depending on the application of course).

Even then you're not guaranteed that code executes so maybe I'm not clear as to what you are trying to accomplish or what problem you are trying to solve.

Edit:

Per the OPs comment the best option (for his desire to have "cleanup" code execute without being specifically called and untestable) would be to put it in the deconstructor/finalizer. Note this is not guaranteed to run but should run in most circumstances without being called (unlike Dispose):

public class Foo
{
    public Foo()
    {
        //Constructor
    }

    ~Foo()
    {
        // Deconstructor/finalizer
    }
}

Answer 3

Do both, as in have an explicit Close() or Shutdown() method and also call that in the dispose (track if called etc, see dispose pattern help...)

That way you can implement the IDisposable interface (which makes the intention obvious) and call it in your code.

Are you using a "finally" block around the startup code?

internal static class App
{
    [STAThread]
    private static void Main(string[] args)
    {
        try
        {
            Thing.Startup();
            Application.Run();
        }
        finally
        {
            Thing.Shutdown(); // or dispose
        }
    }
}

Get my drift?!

PK :-)

PS: A "singleton" should be written as a normal class, the singleton part should be an external use thing

Answer 4

As others have said, Dispose will not be automatically called on exit, however a finalizer method will be called. I would do that instead to do your cleanup logic on application exit.

Answer 5

This is bad design. You would be better served having Open and Close methods that you call at the appropriate time. It will be easier to test, debug and support.

Answer 6

Instead of the finalizer or try...catch I would suggest to use the using stement.

Microsoft explains: "The using statement ensures that Dispose is called even if an exception occurs[...]" This sounds to me like what you are looking for: Close the connection in any case when the application closes.

Although it is internally translated into a try...finally statement, this is the recommended way to ensure that the Dispose() method is called. After leaving the using block the object gets disposed and goes out of scope.

internal static class App
{
    [STAThread]
    private static void Main(string[] args)
    {
        using (YourSingeletonClass thing = YourSingeletonClass.GetInstance()) 
        {
            thing.Startup();
            Application.Run();
        }
    }
}

MSDN documentation for using statement can be found here: http://msdn.microsoft.com/en-us/library/yh598w02.aspx