The .NET bug: the finally block could be executed twice

Introduction

You remember the matrix "Déjà vu" bug where the same sequence is executed twice. I think I have found the root cause of this. The matrix encounters a .NET Framework bug. To make a long debugging story short, I have been chasing an intermittent lock leak that was not the kind you find in textbooks. One of my threads acquires a lock but does not release it (so far nothing exciting).

The thread in question runs the following code:

lock (this)
{
    // Do A
    Monitor.Exit(this);
    try
    {
        // Do B
    }
    finally
    {
        Monitor.Enter(this);
    }
    // Do C
}

In theory, this code is safe and the lock on this should be released no matter if the normal execution flow A, B, and C is performed or if an exception is thrown in either A, B or C. In practice, the lock is not released when my thread leaves this code.

The root cause

I initially thought the lock leak was due to the result of mixing the C# lock statement with Monitor.Exit/Enter. I quickly ruled this out after I found that the same leak was happening no matter if I use the lock statement, the calls to Monitor.Enter/Exit or the .NET attribute [MethodImpl(MethodImplOptions.Synchronized)]. Then I thought of putting together a simple C# console application to repro and study the behavior of try-finally and was surprised with the result that I got. I hit the Déjà vu bug: the finally clause is sometimes executed twice!!!

This is hard to believe, but here is the code that proves me right:

using System;
delegate void Handler(int i);
class DejaVu
{
    static void Main(string[] args)
    {
        int recurse = 2;
        try
        {
            Handler h = new Handler(Throw);
            Console.WriteLine("BeginInvoke");
            IAsyncResult ar = h.BeginInvoke(recurse, null, null);
            Console.WriteLine("WaitOne");
            ar.AsyncWaitHandle.WaitOne();
            Console.WriteLine("EndInvoke");
            h.EndInvoke(ar);
        }
        catch
        {
            Console.WriteLine("Caught exception");
        }
    }
    
    private static void Throw(int i)
    {
        try
        {
            Console.WriteLine("Try({0})", i);
            if (i == 0)
                m_object.ToString(); // cause access violation
            else
                Throw(i-1);
        }
        catch
        {
            Console.WriteLine("Catch({0})", i);
            throw;
        }
        finally
        {
            Console.WriteLine("Finally({0})", i);
        }
    }
    private static object m_object = null;
}

The main thread of my application executes the method Throw asynchronously using the .NET Framework asynchronous delegate's capability. The method Throw calls itself recursively until the parameter i reaches 0. When it is 0, the method causes an access violation exception.

In theory, this code should produce the following output:

BeginInvoke
WaitOne
Try(2)
Try(1)
Try(0)
Catch(0)
Finally(0)
Catch(1)
Finally(1)
Catch(2)
Finally(2)
EndInvoke
Caught exception

In practice, you get the following:

BeginInvoke
WaitOne
Try(2)
Try(1)
Try(0)
Catch(0)
Finally(0)
Catch(1)
Finally(1)
Catch(2)
Finally(0)
Finally(1)
Finally(2)
EndInvoke
Caught exception

The first two finally clauses Finally(0) and Finally(1) are executed twice! First, when the exception is re-thrown from Catch(0) and Catch(1) handlers respectively (this is normal). The second time when the exception is re-thrown from Catch(2) handler (this is abnormal).

This bug occurs only if the method Throw gets called via an asynchronous delegate. When the Throw method is called directly:

Throw(recurse);

or, via an asynchronous delegate:

Handler h = new Handler(Throw);
h(recurse);

The behavior and the output are correct. The behavior is correct even if you replace the access violation exception caused by m_object.ToString() with an explicit exception like:

throw new ArgumentException();

Repeated testing showed me that the bug occurs only in the following situations:

1. The method is called via an asynchronous delegate.
2. The method itself or any of the sub method throws a Win32 exception (I have tested access violation and division by zero but I bet the other would cause the same issue).
3. The exception is caught and re-thrown.

You are going to tell me that this is a rare situation and I would agree but I hit this anyway. Murphy law!

Workaround

The workaround I am using is simple: I always make sure that no exceptions ever leave a method invoked via asynchronous delegates. To this end I am using a mediator method that catches all the exceptions and returns them to the caller in a custom fashion. In the snippet below the mediator just writes the exception to the console:

delegate void MediatorHandler(ThrowHandler h, int i);
static void Main(string[] args)
{
    int recurse = 2;
    try
    {
        Handler h = new Handler(Throw);
        MediatorHandler mh = new MediatorHandler(Mediator);
    
        Console.WriteLine("BeginInvoke");
        IAsyncResult ar = mh.BeginInvoke(h, max, null, null);
    
        Console.WriteLine("WaitOne");
        ar.AsyncWaitHandle.WaitOne();
    
        Console.WriteLine("EndInvoke");
        mh.EndInvoke(ar);
    }
    catch
    {
        Console.WriteLine("Caught exception");
    }
}

private static void Mediator(Handler h, int i)
{
    try
    {
        h(i);
    }
    catch (Exception ex)
    {
        Console.WriteLine("Mediator caught exception {0}", ex);
    }
}

Since the exception never leaves the mediator, the finally clauses are called once and only once.

Finally

I have been looking on the web and newsgroups for references on this issue but couldn't find any. Even though this is a rare case, I doubt I am the only one to have encountered this problem.

I have tested this against .NET Framework 1.1 SP1. I plan to test it against .NET Framework 2 beta 2 as soon as possible.