Should a static class dispose of its IDisposable variables in a "static destructor"?

user_1250500 0
Viewed 158

If a static class has any IDisposable static variables, should that class have a "static destructor" to dispose of them? For example:

public static StaticClass
{
    static SomeDisposableType
        someDisposable = new SomeDisposableType();

    static readonly StaticDestructor
        staticDestructor = new StaticDestructor();

    private sealed class StaticDestructor
    {
        ~StaticDestructor()
        {
            someDisposable.Dispose();
        }
    }
}
idisposable dispose .net c#
2 Answers

No, it should not.

There's no way for the runtime to know when your static class will be used for the last time. There's no way it can know to invoke cleanup early.

Therefore, the only "sensible" time to perform cleanup would be when the entire process is about to terminate. But that's not the right time to be cleaning up either. For a similar, unmanaged, take on this, read Raymond Chen's When DLL_PROCESS_DETACH tells you that the process is exiting, your best bet is just to return without doing anything:

The building is being demolished. Don’t bother sweeping the floor and emptying the trash cans and erasing the whiteboards.

Now, some may bring up the argument that some of your disposables may represent external resources that will not be cleaned up/released when the OS destroys your process. Whilst that's true, those external resources have to cope with e.g. your process being terminated by the user or (if not co-located on the same machine) a power supply failure taking the entire machine away. You don't get to run any cleanup code when there's a power cut. So they already have to be coded to deal with your process not being able to release the resources.

user_154980

There are some code smells happening here.

  1. StaticClass is tightly coupled to the specific types that it depends upon, rather than just their interfaces.
  2. StaticClass determines the lifetime of the services that it uses.

This is preventing StaticClass from being thoroughly unit-testable. For example, you cannot test the behavior of StaticClass without also testing the behavior of SomeDisposableType.

I'd almost always recommend making your StaticClass non-static, and using constructor injection to inject the services it depends on as interfaces, allowing a Dependency Injection framework's configuration to determine the lifetime of those objects.

If there's no compelling reason to have StaticClass be a singleton, then just let it be transient. Your DI framework should take care of cleaning up the disposable that gets injected into it.

If there is a compelling reason to have StaticClass be a singleton, think really hard about your separation of concerns: is StaticClass doing too much? For example, maybe it's doing some work to find values, and then storing those values to avoid doing that work again later. Or perhaps it's saving the state of certain properties of your application, and acting based on that state. In these cases, you can usually separate the state-saving or memoizing/caching work in a separate class that can be singleton-bound. Then your service that consumes this state or cached values can still be transient, and its disposable dependencies can still be disposed after it's done a specific job.

If, after considering all of the above, you're still convinced this class needs to have a long lifetime, you should carefully consider the lifetime of your disposable dependency. Usually if a class is disposable, that's because it holds on to resources that should be released from time to time. In that case, rather than injecting that class directly, perhaps you should inject a factory which you can use to construct the service on-demand and then dispose it as soon as an action is complete via a using statement.

It's hard to make more specific recommendations without knowing more about your specific classes, but these are the patterns I've found to work best in the vast majority of cases.

user_1209550
Related