How to handle exception thrown from Dispose?

Question

Recently, I was researching some tricky bugs about object not disposed.

I found some pattern in code. It is reported that some m_foo is not disposed, while it seems all instances of SomeClass has been disposed.

public class SomeClass: IDisposable
{
    void Dispose()
    {
       if (m_foo != null)
       {
          m_foo.Dispose();
       }
       if (m_bar != null)
       {
          m_bar.Dispose();
       }   
    }

    private Foo m_foo;

    private Bar m_bar;

}

I suspects that Foo.Dispose might throw a exception, so that following code is not executed so m_bar is not disposed.

Since Foo/Bar might be from third party, so it is not guaranteed to not throwing exception.

If just wrap all Dispose invocation with try-catch, the code will turn to be clumsy.

What's best practice to handle this?

Answer 1

If Dispose() is called inside of a finalization context and it throws an exception, your process will be terminated.

If you suspect that Foo.Dispose() is throwing an exception, I would dispose of it last if possible, and wrap it in try/catch. Do everything you can to get rid of it in the catch - set the reference to null. It is very bad to throw exceptions from Dispose() and should be avoided.

Unfortunately, if this is buggy third-party code, your best bet would be to get them to fix it. You shouldn't have to manually clean up after it.

Hope that helps.

Answer 2

Its true that it can be pretty bad to leak out an exception of your dispose method, especially since stuff that implements IDisposable will usually specify a finalizer that will call Dispose.

The problem is that sweeping the problem under of the carpet by handling an exception may leave you with some very hard-to-debug situations. What if your IDisposable allocated a critical section of sorts that only gets released after dispose. If you ignore the fact that the exception happened, you may end up in deadlock central. I think failures in Dispose should be one of those cases where you want to fail early, so you can fix the bug as soon as its discovered.

Of course it all depends on the object being disposed, for some objects you may be able to recover, others not. As a general rule of thumb, Dispose should not throw exceptions when used correctly and you should not have to code defensively around exceptions in nested Dispose methods you are calling.

Do you really do not want to sweep an OutOfMemoryException under the carpet?

If I had a dodgy 3rd party component that arbitrarily threw exceptions on Dispose I would get it fixed AND host it in a separate process that I could tear down when it started playing up.

Answer 3

Because you don't have to allocate the variables in a using() statement - why not use 'stacked' using statements for this?

void Dispose()
{
    // the example in the question didn't use the full protected Dispose(bool) pattern
    // but most code should have if (!disposed) { if (disposing) { ...

    using (m_foo)
    using (m_bar)  
    {
        // no work, using statements will check null 
        // and call Dispose() on each object
    }

    m_bar = null;
    m_foo = null;
}

The 'stacked' using statements are expanded like this:

using (m_foo)
{
    using (m_bar) { /* do nothing but call Dispose */ }
}

So the Dispose() calls are put in seperate finally blocks:

try {
    try { // do nothing but call Dispose
    }
    finally { 
        if (m_bar != null)
            m_bar.Dispose(); 
    }
finally { 
    if (m_foo != null)
        m_foo.Dispose();
}

I had a hard time finding a reference for this in one place. The 'stacked' using statements are found in an old Joe Duffy blog post (see section 'C# and VB Using Statement, C++ Stack Semantics'). The Joe Duffy post is referenced by many StackOverflow answers on IDisposable. I also found a recent question where stacked using statements for local variables appear to be common. I couldn't find the chaining of finally blocks anywhere but the C# language spec (section 8.13 in C# 3.0 spec), and only for multiple varaibles within a single 'using' block, which isn't exactly what I'm proposing, but if you disasemble the IL you'll find the try/finally blocks are nested. On the null check, also from the C# spec: 'If a null resource is acquired, then no call to Dispose is made, and no exception is thrown.'

Answer 4

Hi,

To avoid repeating code for disposing objects, I wrote the following static method.

    public static void DisposeObject<T>(ref T objectToDispose) where T : class
    {
        IDisposable disposable = objectToDispose as IDisposable;
        if (disposable == null) return;

        disposable.Dispose();
        objectToDispose = null;
    }

The main point is that you can make it a function, so you type only one line per object you want to dispose, keeping the Dispose methods nice and clean. In our case, it was convention to null disposed pointers, hence the ref parameters.

In your case, you might want to add exception handling, or make a different flavor with exception handling. I would make sure that you log/breakpoint whenever a Dispose() throws exceptions, but if you can't prevent it, the next best thing is to make sure the problem does not spread.

Answer 5

Dispose is not supposed to throw any exceptions. It it does—it’s not well written, so…

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

should be enough.

• http://blogs.msdn.com/b/clyon/archive/2004/09/23/233464.aspx