IDisposable without finalizer in a Singleton scenario

Question

Is it a right expectation that if a C# class deals with unmanaged resources and implements IDisposable, then it also should implement some kind of finalizer logic?

We have a vendor-supplied utility class that uses some unmanaged resources. We don't have the source code, so don't know too much implementation details, apart from the fact that it implements IDisposable but does not have any finalizer or SafeHandle. So without calling Dispose, it leaks. Simplified sample code:

public class Utility : IDisposable
{
    //This class has some unmanaged resources
    ...
    //Utility function
    public void DoSomething(...) { ... }
    //IDisposable
    public void Dispose() { ... }
    protected virtual void Dispose(bool disposing) { ... }
}

In our code we created a facade around it to simplify the usage and also added a singleton as it is used at dozens of places in our complex legacy codebase. Example:

public class UtilityFacade : IDisposable
{
    protected Utility utility = new Utility();
    //Simplified utility function
    public void DoSomethingGood() { utility.DoSomething(1,2,3); }
    //IDisposable
    protected bool disposed = false;
    protected virtual void Dispose(bool disposing)
    {
        if (!disposed)
        {
            if (disposing)
            {
                utility.Dispose();
            }
            disposed = true;
        }
    }
    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }
    //Singleton
    public static UtilityFacade Instance { get; } = new UtilityFacade();
}

So from our applications (web site, web service, console apps) it would be used like this:

UtilityFacade.Instance.DoSomethingGood();

The problem is that since the Instance is a static member, it will never be disposed via IDisposable, so the Utility class will not be disposed either, and since it doesn't implement any finalizer, it leaks unmanaged resources at the end of the application process.

Which is the right choice here:

1. The vendor says we should call Utility.Dispose() when the application finishes. This would require extra coding for us, e.g. hooking up to Application_End for web apps, or adding try{}finally{...} blocks to console apps for deterministic cleanup, or adding a finalizer to the facade class (which would keep the code related to Utility at one place at least).
2. We request the vendor to implement the finalizer. Since the Utility class is dealing directly with the unmanaged resources, they should add the finalizer logic. According to some articles on the net it is the best practice recommended by Microsoft.

Or any other recommendation?

Answer 1

A finalizer should be implemented for any class that manages unmanaged resources; the vendor's class would be improved if they did so. However, this won't necessarily improve your code, because finalizers are not guaranteed to be called, and in fact finalizers of objects that are reachable through statics are very unlikely to be called.

So the right solution is to call Dispose on your facade at the right time. The best way to achieve this is to not make it a singleton in the first place. Inject the instance into the components that need it, and make sure that the instance is cleaned up correctly. For console apps, this may mean a simple using block in Main. For web apps, you should probably have a DI container already (especially if you're using ASP.Net Core) which is wired to be disposed at application shutdown, and will itself dispose all the non-transient instances it manages.

Answer 2

Dispose should be enough.
You can create facade class which will call dispose

public class UtilityFacade
{
    public void DoSomethingGood() 
    { 
        using (var utility = new Utility())
        {
            utility.DoSomething(1,2,3);
        }
    }

    // Make it singleton if you "really" need it.
    public static UtilityFacade Instance { get; } = new UtilityFacade();
}

Answer 3

it is the best practice recommended by Microsoft.

Well, you don't follow the best practices Microsoft laid out yourself. You just said following the basic guidelines and always call .Dispose() is too much work for you. So calling your vendor and asking them to build in something that is considered a guardrail against client failure, because you plan to fail is probably not making you look good. They might do it, because you pay them, but don't count on having any reputation for professional software development with them any time soon.

You want to follow the guidelines in just one spot, that would be your facade. If you insist on the finalizer being a good solution, write one for that class.

On the other hand, your problem is not exactly new or unique. The solution people use is called a "dependency injection container". Microsoft ASP.NET Core projects come with one already set up for you. Older projects will probably have tutorials all over the net on how to set one up. These things handle object lifetime. You can use their utility class and tell the container that one should be created for each scope for example. The container will call Dispose() on it when the scope ends. Or you could tell the container to have it as a singleton lifetime (this has nothing to do with the singleton (anti-)pattern). This means the one object will live as long as the container lives.

So my suggestion would be to aim for good craftsmanship and implement a dependency injection framework for your solution. At the same time, it won't hurt to suggest your vendor to implement a finalizer, but don't count on it and don't ask for it. It's something that's nice to have, not something you should need.

Answer 4

I would say that requiring the application process to do cleanup at exit is wrong, regardless of how it is coded. A process can be killed without giving a chance to run anything at the end. The system should not get into broken state, regardless what process did or did not before its exits.

Otherwise I agree with Sebastian Redl's answer, you should dispose your singleton instance before exit, either regular or erroneous. But keep in mind that whatever you do if process is killed something will stay and would need some cleanup.