Exception handling in IDisposable.Dispose

[Guest]

What is the correct pattern for handling exceptions in IDisposable.Dispose,
especially in a class that manages multiple unmanaged resources? An example
of such a class is System.ComponentModel.Container.

I have always understood that the IDisposable contract was that Dispose
guarantees to release unmanaged resources owned by the object - even if the
Dispose method throws an exception.

This seems to be underlined by the .NET 2.0 implementation of StreamWriter.

The StreamWriter Dispose method attempts to flush the stream before closing
it: this can of course throw an exception (e.g. the stream may represent a
network file).

In .NET 1.1, an exception thrown while Dispose is flushing the stream is
propagated to the caller and the stream is not closed. The unmanaged
resource is not released.

This has been fixed in .NET 2.0: a try/finally block is used to ensure the
stream is always closed.

The fact that this was fixed seems to imply that Microsoft considers the
correct behavior is to release unmanaged resources whether or not there is an
exception.

What if a class, like System.ComponentModel.Container contains multiple
unmanaged resources?

Using Reflector on System.ComponentModel.Container shows it contains code
something like the following:

....
while (siteCount > 0)
{
ISite site = sites[siteCount-1];
site.Component.Site = null;
site.Component.Dispose();
}
....

This means if one of the components has a Dispose method which throws an
exception, then Dispose will not be called on the remaining components.
Which appears to break the IDispose.Dispose contract.

The only way I can see to guarantee that all resources are disposed would be
to use try/catch blocks - which signficiantly complicates the Dispose
pattern. In the case of Container, it could be something like:

....
Exception savedException = null;
while (siteCount > 0)
{
try
{
ISite site = sites[siteCount-1];
site.Component.Site = null;
site.Component.Dispose();
}
catch(Exception ex)
{
if (savedException == null) savedException = ex;
}
}
if (savedException == null)
{
// Wrap savedException in a new exception and throw it.
// Or better, save all exceptions above in a collection and add
// the collection to the new custom exception.
...
}
....

 

[David Levine]

The problem is that Dipose is not a contract, it is a convention. I agree
that it ought to behave the way you describe, but there is nothing in the
CLR itself to enforce that behavior, it requires that every class library
developer get it right.

 

[Guest]

The problem is that Dipose is not a contract, it is a convention.
I don't really understand the distinction you're making between a contract
and a convention. Neither is enforcable, and both require that class library
developers get it right.

The points I'm making is that it seems to me that:

1. the specification of the contract or convention is ambiguous: an
authoritative specification needs to be published in the framework
documentation.

2. If the expected behavior is as I understood it, then the
System.ComponentModel.Container class has a bug which needs to be fixed.