Handling exceptions thrown by "Dispose" while unwinding nested "using" statements

Question

Apparently, some exceptions may just get lost while using nested using statement. Consider this simple console app:

using System;

namespace ConsoleApplication
{
    public class Throwing: IDisposable
    {
        int n;

        public Throwing(int n)
        {
            this.n = n;
        }

        public void Dispose()
        {
            var e = new ApplicationException(String.Format("Throwing({0})", this.n));
            Console.WriteLine("Throw: {0}", e.Message);
            throw e;
        }
    }

    class Program
    {
        static void DoWork()
        {
            // ... 
            using (var a = new Throwing(1))
            {
                // ... 
                using (var b = new Throwing(2))
                {
                    // ... 
                    using (var c = new Throwing(3))
                    {
                        // ... 
                    }
                }
            }
        }

        static void Main(string[] args)
        {
            AppDomain.CurrentDomain.UnhandledException += (sender, e) =>
            {
                // this doesn't get called
                Console.WriteLine("UnhandledException:", e.ExceptionObject.ToString());
            };

            try
            {
                DoWork();
            }
            catch (Exception e)
            {
                // this handles Throwing(1) only
                Console.WriteLine("Handle: {0}", e.Message);
            }

            Console.ReadLine();
        }
    }
}

Each instance of Throwing throws when it gets disposed of. AppDomain.CurrentDomain.UnhandledException never gets called.

The output:

Throw: Throwing(3)
Throw: Throwing(2)
Throw: Throwing(1)
Handle: Throwing(1)

I prefer to at least be able to log the missing Throwing(2) and Throwing(3). How do I do this, without resorting to a separate try/catch for each using (which would kinda kill the convenience of using)?

In real life, those objects are often instances of classes over which I have no control. They may or may not be throwing, but in case they do, I'd like to have an option to observe such exceptions.

This question came along while I was looking at reducing the level of nested using. There's a neat answer suggesting aggregating exceptions. It's interesting how this is different from the standard behavior of nested using statements.

[EDITED] This question appears to be closely related: Should you implement IDisposable.Dispose() so that it never throws?

Answer 1

There's a code analyzer warning for this. CA1065, "Do not raise exceptions in unexpected locations". The Dispose() method is on that list. Also a strong warning in the Framework Design Guide, chapter 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.).

This goes wrong because the using statement calls Dispose() inside a finally block. An exception raised in a finally block can have an unpleasant side-effect, it replaces an active exception if the finally block was called while the stack is being unwound because of an exception. Exactly what you see happening here.

Repro code:

class Program {
    static void Main(string[] args) {
        try {
            try {
                throw new Exception("You won't see this");
            }
            finally {
                throw new Exception("You'll see this");
            }
        }
        catch (Exception ex) {
            Console.WriteLine(ex.Message);
        }
        Console.ReadLine();
    }
}

Answer 2

Maybe some helper function that let you write code similar to using:

 void UsingAndLog<T>(Func<T> creator, Action<T> action) where T:IDisposabe
 {  
      T item = creator();
      try 
      {
         action(item);
      }
      finally
      { 
          try { item.Dispose();}
          catch(Exception ex)
          {
             // Log/pick which one to throw.
          } 
      }      
 }

 UsingAndLog(() => new FileStream(...), item => 
 {
     //code that you'd write inside using 
     item.Write(...);
 });

Note that I'd probably not go this route and just let exceptions from Dispose to overwrite my exceptions from code inside normal using. If library throws from Dispose against strong recommendations not to do so there is a very good chance that it is not the only issue and usefulness of such library need to be reconsidered.

Answer 3

What you are noticing is a fundamental problem in the design of Dispose and using, for which no nice solution as yet exists. IMHO the best design would be to have a version of Dispose which receives as an argument any exception which may be pending (or null, if none is pending), and can either log or encapsulate that exception if it needs to throw one of its own. Otherwise, if you have control of both the code which could cause an exception within the using as well as within the Dispose, you may be able to use some sort of outside data channel to let the Dispose know about the inner exception, but that's rather hokey.

It's too bad there's no proper language support for code associated with a finally block (either explicitly, or implicitly via using) to know whether the associated try completed properly and if not, what went wrong. The notion that Dispose should silently fail is IMHO very dangerous and wrongheaded. If an object encapsulates a file which is open for writing, and Dispose closes the file (a common pattern) and the data cannot be written, having the Dispose call return normally would lead the calling code to believe the data was written correctly, potentially allowing it to overwrite the only good backup. Further, if files are supposed to be closed explicitly and calling Dispose without closing a file should be considered an error, that would imply that Dispose should throw an exception if the guarded block would otherwise complete normally, but if the guarded block fails to call Close because an exception occurred first, having Dispose throw an exception would be very unhelpful.

If performance isn't critical, you could write a wrapper method in VB.NET which would accept two delegates (of types Action and an Action<Exception>), call the first within a try block, and then call the second in a finally block with the exception that occurred in the try block (if any). If the wrapper method was written in VB.NET, it could discover and report the exception that occurred without having to catch and rethrow it. Other patterns would be possible as well. Most usages of the wrapper would involve closures, which are icky, but the wrapper could at least achieve proper semantics.

An alternative wrapper design which would avoid closures, but would require that clients use it correctly and would provide little protection against incorrect usage would have a usage batter like:

var dispRes = new DisposeResult();
... 
try
{
  .. the following could be in some nested routine which took dispRes as a parameter
  using (dispWrap = new DisposeWrap(dispRes, ... other disposable resources)
  {
    ...
  }
}
catch (...)
{
}
finally
{
}
if (dispRes.Exception != null)
  ... handle cleanup failures here

The problem with this approach is that there's no way to ensure that anyone will ever evaluate dispRes.Exception. One could use a finalizer to log cases where dispRes gets abandoned without ever having been examined, but there would be no way to distinguish cases where that occurred because an exception kicked code out beyond the if test, or because the programmer simply forgot the check.

PS--Another case where Dispose really should know whether exceptions occur is when IDisposable objects are used to wrap locks or other scopes where an object's invariants may temporarily be invalidated but are expected to be restored before code leaves the scope. If an exception occurs, code should often have no expectation of resolving the exception, but should nonetheless take action based upon it, leaving the lock neither held nor released but rather invalidated, so that any present or future attempt to acquire it will throw an exception. If there are no future attempts to acquire the lock or other resource, the fact that it is invalid should not disrupt system operation. If the resource is critically necessary to some part of the program, invalidating it will cause that part of the program to die while minimizing the damage it does to anything else. The only way I know to really implement this case with nice semantics is to use icky closures. Otherwise, the only alternative is to require explicit invalidate/validate calls and hope that any return statements within the part of the code where the resource is invalid are preceded by calls to validate.