Should you implement IDisposable.Dispose() so that it never throws?

Question

For the equivalent mechanism in C++ (the destructor), the advice is that it should usually not throw any exceptions. This is mainly because by doing so you might terminate your process, which is only very rarely a good strategy.

In the equivalent scenario in .NET ...

1. A first exception is thrown
2. A finally block is executed as a result of the first exception
3. The finally block calls a Dispose() method
4. The Dispose() method throws a second exception

... your process does not terminate immediately. However, you lose information because .NET unceremoneously replaces the first exception with the second one. A catch block somewhere up the call stack will therefore never see the first exception. However, one is usually more interested in the first exception because that normally gives better clues as to why things started to go wrong.

Since .NET lacks a mechanism to detect whether code is being executed while an exception is pending, it seems there are really only two choices how IDisposable can be implemented:

• Always swallow all exceptions that occur inside Dispose(). Not good as you might also end up swallowing OutOfMemoryException, ExecutionEngineException, etc. which I'd usually rather let tear down the process when they occur without another exception already pending.
• Let all exceptions propagate out of Dispose(). Not good as you might lose information about the root cause of a problem, see above.

So, which is the lesser of the two evils? Is there a better way?

EDIT: To clarify, I'm not talking about actively throwing exceptions from Dispose() or not, I'm talking about letting exceptions thrown by methods called by Dispose() propagate out of Dispose() or not, for example:

using System;
using System.Net.Sockets;

public sealed class NntpClient : IDisposable
{
    private TcpClient tcpClient;

    public NntpClient(string hostname, int port)
    {
        this.tcpClient = new TcpClient(hostname, port);
    }

    public void Dispose()
    {
        // Should we implement like this or leave away the try-catch?
        try
        {
            this.tcpClient.Close(); // Let's assume that this might throw
        }
        catch
        {
        }
    }
}

Answer 1

I would argue that swallowing is the lesser of the two evils in this scenario, as it is better to raise the original Exception - caveat: unless, perhaps the failure to cleanly dispose is itself pretty darned critical (perhaps if a TransactionScope couldn't dispose, since that could indicate a rollback failure).

See here for more thoughts on this - including a wrapper/extension method idea:

using(var foo = GetDodgyDisposableObject().Wrap()) {
   foo.BaseObject.SomeMethod();
   foo.BaseObject.SomeOtherMethod(); // etc
} // now exits properly even if Dispose() throws

Of course, you could also do some oddity where you re-throw a composite exception with both the original and second (Dispose()) exception - but think: you could have multiple using blocks... it would quickly become unmanageable. In reality, the original exception is the interesting one.

Answer 2

Dispose should be designed to do its purpose, disposing the object. This task is safe and does not throw exceptions most of the time. If you see yourself throwing exceptions from Dispose, you should probably think twice to see if you are doing too much stuff in it. Beside that, I think Dispose should be treated like all other methods: handle if you can do something with it, let it bubble if you can't.

EDIT: For the specified example, I would write the code so that my code does not cause an exception, but clearing the TcpClient up might cause an exception, which should be valid to propagate in my opinion (or to handle and rethrow as a more generic exception, just like any method):

public void Dispose() { 
   if (tcpClient != null)
     tcpClient.Close();
}

However, just like any method, if you know tcpClient.Close() might throw an exception that should be ignored (doesn't matter) or should be represented by another exception object, you might want to catch it.

Answer 3

I would probably use logging to capture detail about the first exception, then allow the second exception to be raised.

Answer 4

The "Framework Design Guidelines" (2^nd ed) has this as (§9.4.1):

AVOID throwing an exception from within Dispose(bool) except under critical situations where the containing process has been corrupted (leaks, inconsistent shared state, etc.).

Commentary [Edit]:

• There are guidelines, not hard rules. And this is an "AVOID" not a "DO NOT" guideline. As noted (in comments) the Framework breaks this (and other) guidelines in places. The trick is knowing when to break a guideline. That, in many ways, is the difference between a Journeyman and a Master.
• If some part of the clean-up could fail then should provide a Close method that will throw exceptions so the caller can handle them.
• If you are following the dispose pattern (and you should be if the type directly contains some unmanaged resource) then the Dispose(bool) may be called from the finaliser, throwing from a finaliser is a bad idea and will block otehr objects from being finalised.

My view: exceptions escaping from Dispose should only be those, as in the guideline, that as sufficiently catastrophic that no further reliable function is possible from the current process.

Answer 5

Releasing resources should be a "safe" operation - after all how can I recover from not being able to release a resource? so throwing an exception from Dispose just doesn't make sense.

However, if I discover inside Dispose that program state is corrupted it's better to throw the exception then to swallow it, its better to crush now then to continue running and produce incorrect results.