Minimal IDisposable implimenation for managed resources only
There is a LOT of info around about the "standard full" IDisposable implementation for disposing of unmanaged resources - but in reality this case is (very) rare (most resources are already wrapped by managed classes). This question focuses on a mimimal implementation of IDisposable for the much more common "managed resources only" case.
1: Is the mimimal implementation of IDisposable in the code below correct, are there issues?
2: Is there any reason to add a full standard IDisposable implementation ( Dispose() , Dispose(bool) , Finalizer etc) over the minimal implimentation presented?
3: Is it OK/wise in this minimal case to make the Dispose virtual (since we are not providing Dispose(bool) )?
4: If this minimal implementation is replaced with a full standard implementation that includes a (useless, in this case) finalizer - does this change how the GC handles the object? Are there any downsides?
5: The example includes Timer and event handlers as these cases are particularly important not to miss as failing to dispose them will keep objects alive and kicking ( this in the case of Timer , eventSource in case of the event handler) until the GC gets round to disposing them in its time. Are there any other examples like these?
class A : IDisposable {
private Timer timer;
public A(MyEventSource eventSource) {
eventSource += Handler
}
private void Handler(object source, EventArgs args) { ... }
public virtual void Dispose() {
timer.Dispose();
if (eventSource != null)
eventSource -= Handler;
}
}
class B : A, IDisposable {
private TcpClient tpcClient;
public override void Dispose() {
(tcpClient as IDispose).Dispose();
base.Dispose();
}
}
refs:
MSDN
SO: When do I need to manage managed resources
SO: How to dispose managed resource in Dispose() method in C#
SO: Dispose() for cleaning up managed resources
The implementation is correct, there are no issues, provided no derived class directly owns an unmanaged resource.
One good reason to implement the full pattern is the "principle of least surprise". Since there is no authoritative document in MSDN describing this simpler pattern, maintenance developers down the line might have their doubts - even you felt the need to ask StackOverflow :)
Yes it's OK for Dispose to be virtual in this case.
The overhead of the unnecessary finalizer is negligible if Dispose has been called, and is correctly implemented (ie calls GC.SuppressFinalize)
The overwhelming majority of IDisposable classes outside the .NET Framework itself are IDisposable because they own managed IDisposable resources. It's rare for them to directly hold an unmanaged resource - this only happens when using P/Invoke to access unmanaged resources that aren't exposed by the .NET Framework.
Therefore there is probably a good argument for promoting this simpler pattern:
In the rare cases that unmanaged resources are used, they should be wrapped in a sealed IDisposable wrapper class that implements a finalizer (like SafeHandle). Because it's sealed, this class doesn't need the full IDisposable pattern.
In all other cases, the overwhelming majority, your simpler pattern could be used.
But unless and until Microsoft or some other authoritative source actively promotes it, I'll continue to use the full IDisposable pattern.
Another option is to refactor your code to avoid inheritance and make your IDisposable classes sealed. Then the simpler pattern is easy to justify, as the awkward gyrations to support possible inheritance are no longer necessary. Personally I take this approach most of the time; in the rare case where I want to make a non-sealed class disposable, I just follow the 'standard' pattern. One nice thing about cultivating this approach is that it tends to push you toward composition rather than inheritance, which generally makes code easier to maintain and test.
My recommended Dispose pattern is for a non-virtual Dispose implementation to chain to a virtual void Dispose(bool) , preferably after something like:
int _disposed;
public bool Disposed { return _disposed != 0; }
void Dispose()
{
if (System.Threading.Interlocked.Exchange(ref _disposed, 1) != 0)
Dispose(true);
GC.SuppressFinalize(this); // In case our object holds references to *managed* resources
}
Using this approach will ensure that Dispose(bool) only gets called once, even if multiple threads try to invoke it simultaneously. Although such simultaneous disposal attempts are rare(*), it's cheap to guard against them; if the base class doesn't do something like the above, every derived class must have its own redundant double-dispose guard logic and likely a redundant flag as well.
(*) Some communications classes which are mostly single-threaded and use blocking I/O allow Dispose to be called from any threading context to cancel an I/O operation which is blocking its own thread [obviously Dispose can't be called on that thread, since that thread can't do anything while it's blocked]. It's entirely possible--and not unreasonable--for such objects, or objects which encapsulate them, to have an outside thread try to Dispose them as a means of aborting their current operation at just the moment they were going to be disposed by their main thread. Simultaneous Dispose calls are likely to be rare, but their possibility doesn't indicate any "design problem" provided that the Dispose code can act upon one call and ignore the other.