I've stumbled across Microsoft's recommended way to implement the IDisposable pattern many times, it's even present in Visual Studio as an "Implement Interface" option in the lamp icon menu. It looks like this:
// Override only if 'Dispose(bool disposing)' has code to free unmanaged resources
~Foo() {
// Do not change this code.
Dispose(calledByFinalizer: true);
}
public void Dispose() {
// Do not change this code.
Dispose(calledByFinalizer: false);
GC.SuppressFinalize(this);
}
// Put cleanup code here
protected virtual void Dispose(bool calledByFinalizer) {
if (_disposed) return;
if (!calledByFinalizer) { /* dispose managed objects */ }
/* free unmanaged resources and set large fields to null */
_disposed = true;
}
I refactored the suggested code a bit (because Dispose(bool disposing) can break someone's brain, and nested if's can break someone's eyes).
But I still have some questions on my mind:
_disposed = true placed at the end of the method and not at the beginning? If IDisposable.Dispose() is called from different threads, then they can all bypass the if (_disposed) return; check and actually execute the method body twice. Why not do it like this: if (_disposed) return;
else _disposed = true;
protected virtual void Dispose(bool disposing) flagged as virtual? Any derived class does not have access to the _disposed field and can easily break its behavior. We can only mark as virtual the optional part where the derived class can do anything without calling base.Dispose():~Foo() => FreeUnmanagedResources();
public void Dispose() {
if (_disposed) return;
else _disposed = true;
DisposeManagedObjects();
FreeUnmanagedResources();
GC.SuppressFinalize(this);
}
protected virtual void DisposeManagedObjects() { }
protected virtual void FreeUnmanagedResources() { }
You can't assume that Dispose is only going to get called once. In best practice, yes. In the worst practice, not at all. Every situation cannot conveniently use a using statement. So rather than risk the code trying to clean up unmanaged resources twice -- which could go really bad, depending on the type of resource -- there's a flag added that prevents it. This takes a burden off the calling code as far as remembering if dispose has already been called.
Dispose has to be declared as virtual to support any separate cleanup that might need to occur if subclasses are created that instantiate any unmanaged resources that are distinctly different than those used in the base class. Dispose in the subclass should call base.Dispose(); before or after it cleans up its own mess.
The pattern is correct, but assumes the worst case scenario, where you must also implement a finalizer. That is, if you need a finalizer you must also follow the entire pattern. However...
You don't usually need a finalizer at all.
You only need a finalizer if you are creating the original managed wrapper for an unmanaged resource.
For example, let's say you create a brand new, never before seen database system. You want to provide a .Net ADO provider for this new kind of database, including the connections (it will inherit from Dbconnection). The underlying network operations here will be an unmanaged resource, and there isn't already a finalizer to release them anywhere in your inhertitance tree. Therefore, you must implement your own finalizer.
On the other hand, if you are creating a wrapper object for your application to manage connections to an existing database type — just repacking (wrapping or inheriting) an existing SqlConnection, OleDbConnection, MySqlConnection, etc — then you should still implement IDisposable, but there is already a finalizer provided for the unmanaged resource and you don't need to write another.
And it turns out, when you don't have a finalizer you can safely remove a lot of the code from the documented IDisposable pattern.