Should you implement IDisposable.Dispose() so that it never throws?
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 ...
... 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:
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
{
}
}
}
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.
The Framework Design Guidelines (2nd 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]:
Dispose(bool)
may be called from the finaliser, throwing from a finaliser is a bad idea and will block other 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.
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.