Minimal IDisposable implimenation for managed resources only

Question

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

Answer 1

1. The implementation is correct, there are no issues, provided no derived class directly owns an unmanaged resource.

2. 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 :)

3. Yes it's OK for Dispose to be virtual in this case.

4. The overhead of the unnecessary finalizer is negligible if Dispose has been called, and is correctly implemented (i.e. 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.

Answer 2

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.