How should I handle exceptions in my Dispose() method?

Question

I'd like to provide a class to manage creation and subsequent deletion of a temporary directory. Ideally, I'd like it to be usable in a using block to ensure that the directory gets deleted again regardless of how we leave the block:

static void DoSomethingThatNeedsATemporaryDirectory()
{
    using (var tempDir = new TemporaryDirectory())
    {
        // Use the directory here...
        File.WriteAllText(Path.Combine(tempDir.Path, "example.txt"), "foo\nbar\nbaz\n");
        // ...
        if (SomeCondition)
        {
            return;
        }
        if (SomethingIsWrong)
        {
            throw new Exception("This is an example of something going wrong.");
        }
    }
    // Regardless of whether we leave the using block via the return,
    // by throwing and exception or just normally dropping out the end,
    // the directory gets deleted by TemporaryDirectory.Dispose.
}

Creating the directory is no problem. The problem is how to write the Dispose method. When we try to delete the directory, we might fail; for example because we still have a file open in it. However, if we allow the exception to propagate, it might mask an exception that occurred inside the using block. In particular, if an exception occurred inside the using block, it might be one that caused us to be unable to delete the directory, but if we mask it we have lost the most useful information for fixing the problem.

It seems we have four options:

1. Catch and swallow any exception when trying to delete the directory. We might be left unaware that we're failing to clean up our temporary directory.
2. Somehow detect if the Dispose is running as part of stack unwinding when an exception was thrown and if so either suppress the IOException or throw an exception that amalgamates the IOException and whatever other exception was thrown. Might not even be possible. (I thought of this in part because it would be possible with Python's context managers, which are in many ways similar to .NET's IDisposable used with C#'s using statement.)
3. Never suppress the IOException from failing to delete the directory. If an exception was thrown inside the using block we will hide it, despite there being a good chance that it had more diagnostic value than our IOException.
4. Give up on deleting the directory in the Dispose method. Users of the class must remain responsible for requesting deletion of the directory. This seems unsatisfactory as a large part of the motivation for creating the class was to lessen the burden of managing this resource. Maybe there's another way to provide this functionality without making it very easy to screw up?

Is one of these options clearly best? Is there a better way to provide this functionality in a user-friendly API?

Answer 1

Instead of thinking of this as a special class implementing IDisposable, think of what it would be like in terms of normal program flow:

Directory dir = Directory.CreateDirectory(path);
try
{
    string fileName = Path.Combine(path, "data.txt");
    File.WriteAllText(fileName, myData);
    UploadFile(fileName);
    File.Delete(fileName);
}
finally
{
    Directory.Delete(dir);
}

How should this behave? It's the exact same question. Do you leave the content of the finally block as-is, thereby potentially masking an exception that occurs in the try block, or do you wrap the Directory.Delete in its own try-catch block, swallowing any exception in order to prevent masking the original?

I don't think there's any right answer - the fact is, you can only have one ambient exception, so you have to pick one. However, the .NET Framework does set some precedents; one example is WCF service proxies (ICommunicationObject). If you attempt to Dispose a channel that is faulted, it throws an exception and will mask any exception that is already on the stack. If I'm not mistaken, TransactionScope can do this too.

Of course, this very behaviour in WCF has been an endless source of confusion; most people actually consider it very annoying if not broken. Google "WCF dispose mask" and you'll see what I mean. So perhaps we shouldn't always try to do things the same way Microsoft does them.

Personally, I'm of the mind that Dispose should never mask an exception already on the stack. The using statement is effectively a finally block and most of the time (there are always edge cases), you would not want to throw (and not catch) exceptions in a finally block, either. The reason is simply debugging; it can be extremely hard to get to the bottom of an issue - especially an issue in production where you can't step through the source - when you don't even have the ability to find out where exactly the app is failing. I've been in this position before and I can confidently say that it will drive you completely and utterly insane.

My recommendation would be either to eat the exception in Dispose (log it, of course), or actually check to see if you're already in a stack-unwinding scenario due to an exception, and only eat subsequent exceptions if you know that you'll be masking them. The advantage of the latter is that you don't eat exceptions unless you really have to; the disadvantage is that you've introduced some non-deterministic behaviour into your program. Yet another trade-off.

Most people will probably just go with the former option and simply hide any exception occurring in finally (or using).