Hi,

I have a class similar to that:

class MyClass : IDisposable {
IDisposable obj1;
IDisposable obj2;
IDisposable obj3;

MyClass() {
obj1 = new SomeDisposableClass();
obj2 = new SomeDisposableClass();
obj3 = new SomeDisposableClass();
}

void Dispose() {
obj1.Dispose();
obj2.Dispose();
obj3.Dispose();
}
}

The problem I'm having with that is that if the Dispose method of obj2
throws an exception, obj3 will not get disposed.

How would you write the Dispose method of MyClass to correctly dispose
all three objects (without swallowing the exception)?

... something like.. uh oh..:

void Dispose() {
try {
obj1.Dispose();
}
finally {
try {
obj2.Dispose();
}
finally {
obj3.Dispose();
}
}
}

thanks,
Max

On Wed, 14 Jun 2006 22:47:55 +0200, Markus Stoeger wrote:


> void Dispose() {
> obj1.Dispose();
> obj2.Dispose();
> obj3.Dispose();
> }

>
> The problem I'm having with that is that if the Dispose method of obj2
> throws an exception, obj3 will not get disposed.
>
> How would you write the Dispose method of MyClass to correctly dispose
> all three objects (without swallowing the exception)?

Dispose() should not throw any exception. This is clearly mentionned in the
IDisposable documentation. I suppose that this rule has been put in place
to solve the exact same problem you are facing now. So if your
SomeDisposableClass class follows this rule, there is no problem (you can
safely assume that this is the case for all the .NET Framework classes). If
SomeDisposableClass is a class that you've developped, review it to check
that it doesn't throw any exception. If SomeDisposableClass is a third
party class over which you have no control and you are not sure whether it
has been properly implemented in that respect, then i would do something
like:

try
{
obj1.Dispose();
}
catch {}

try
{
obj2.Dispose();
}
catch {}

and so on. You should swallow these exceptions (Dispose() is the only place
where exception swallowing is a good practice).

On Wed, 14 Jun 2006 22:47:55 +0200, Markus Stoeger wrote:

> void Dispose() {
> obj1.Dispose();
> obj2.Dispose();
> obj3.Dispose();
> }

By the way, you should not implement your dispose logic directly in the
Dispose() method. Instead, follow the pattern suggested in the
documentation in which the Dispose() method suppresses the finalizer of
your class then calls the Dispose(bool) method. You should add a finalizer
to your class which also calls the Dispose(bool) method. Your dispose logic
should be placed in the Dispose(bool) method.

Using this pattern ensures that your objects will be properly cleaned up by
the GC (thanks to the finalizer) even if the programmer forgets to call the
Dispose() method, which is essential if your class or some of its member
variables contain umanaged resources. And if the developper does call
Dispose(), the fact that Dispose() suppresses the finalizer will prevent
the GC from wasting time by putting your object in the finalization queue
even though it's been already cleaned up and will allow the GC to free the
memory used by the object immediately. If you are not familiar with
finalizers and the way the GC works, then these articles are an absolutely
essential read:
<http://msdn.microsoft.com/msdnmag/issues/1100/GCI/default.aspx>
<http://msdn.microsoft.com/msdnmag/issues/1200/GCI2/default.aspx>

Mehdi <(E-Mail Removed)> wrote:
> On Wed, 14 Jun 2006 22:47:55 +0200, Markus Stoeger wrote:
>
> > void Dispose() {
> > obj1.Dispose();
> > obj2.Dispose();
> > obj3.Dispose();
> > }
>
> By the way, you should not implement your dispose logic directly in the
> Dispose() method. Instead, follow the pattern suggested in the
> documentation in which the Dispose() method suppresses the finalizer of
> your class then calls the Dispose(bool) method. You should add a finalizer
> to your class which also calls the Dispose(bool) method. Your dispose logic
> should be placed in the Dispose(bool) method.

No. There's no need to have a finalizer unless your class directly
handles unmanaged resources. If every class which does so has a
finalizer, there's no need for classes which only hold references to
other objects to have a finalizer.

--
Jon Skeet - <(E-Mail Removed)>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too

Mehdi wrote:
> On Wed, 14 Jun 2006 22:47:55 +0200, Markus Stoeger wrote:
>
> Dispose() should not throw any exception. This is clearly mentionned
> in the IDisposable documentation. I suppose that this rule has been
> put in place to solve the exact same problem you are facing now.

I couldn't find the place on msdn where they say that Dispose shouldn't
throw? See [1] in the remarks section, it says:

Dispose _can throw an exception_ if an error occurs because a resource
has already been freed and Dispose had not been called previously.

... that's at least one reason why Dispose should be allowed to throw.

If those exceptions are swallowed, it can actually lead to resource
leaks over time without anyone noticing why. No good imho.

I came up with the following simple piece of code. It does nested
try/finally blocks automatically using recursion. I think that'll do a
good job, making sure that _everything_ gets disposed without hiding the
exception in case something throws.

Call with Disposer.Dispose(obj1, obj2, obj3);

public sealed class Disposer {
private Disposer() {}

public static void Dispose(params IDisposable[] disposees) {
Dispose(disposees, 0);
}

private static void Dispose(IDisposable[] disposees, int index) {
if (index < disposees.Length) {
try {
IDisposable disposee = disposees[index];

if (disposee != null) {
disposee.Dispose();
}
}
finally {
Dispose(disposees, index + 1);
}
}
}
}

thanks,
Max

[1]
http://msdn.microsoft.com/library/de...sposetopic.asp

Mehdi wrote:
> On Wed, 14 Jun 2006 22:47:55 +0200, Markus Stoeger wrote:
>
>
>> void Dispose() {
>> obj1.Dispose();
>> obj2.Dispose();
>> obj3.Dispose();
>> }
>
>
> By the way, you should not implement your dispose logic directly in the
> Dispose() method.

That was just a short almost-pseudo-code-like example. Normally I follow
the IDisposable pattern! )

thanks anyway,
Max