Plainionist Journey of a Software Craftsman

How I made my finally block not being called (.Net)

In .Net the source code of a finally block is always called, right? Even if there is a return statement or an exception is thrown in the try block, correct? I mean, this is the reason why we use try-finally in the first place.

Well, I got into a case where the code of the finally block was not called …

Let’s start with an example of a try-finally statement:

public class Repository
{
    private int myTransactionsCount = 0;

    public int TransactionsCount => myTransactionsCount;

    public IEnumerable<string> FindAll()
    {
        try
        {
            myTransactionsCount++;

            yield return "a";
            yield return "b";
            yield return "c";
            yield return "d";
        }
        finally
        {
            myTransactionsCount--;
        }
    }
}

This code simulates the access to some database. We first increase the open transactions count, then return the queried data and use the finally block to ensure that the transaction counter is decreased before returning from the method.

When we call this code we would expect that after the execution of FindAll the transaction count is the same as before the call - assuming concurrency free execution. Here is a test case:

[Test]
public void LinQ_WillWork()
{
    var repository = new Repository();

    // just a few more calls to have higher "transaction count"
    var isNotEmpty = System.Linq.Enumerable.Any(repository.FindAll());
    isNotEmpty = System.Linq.Enumerable.Any(repository.FindAll());
    isNotEmpty = System.Linq.Enumerable.Any(repository.FindAll());

    Assert.That(isNotEmpty, Is.True);
    Assert.That(repository.TransactionsCount, Is.EqualTo(0));
}

This test case uses LinQ to operate on the data returned from the FindAll method and as expected this test case goes green.

Now let’s imaging we do not work directly with the enumerator returned by FindAll but pass it on to some generic code which works with IEnumerable instead of IEnumerable<T>. In this code we use a custom implementation of LinQ’s Any which supports the non-generic IEnumerable and the implementation looks like this:

public static bool Any(this IEnumerable self)
{
    return self.GetEnumerator().MoveNext();
}

Simple!

And here is the adapted test case which now uses our custom extension method Any:

[Test]
public void CustomLinQ_WillFail()
{
    var repository = new Repository();

    // just a few more calls to have higher "transaction count"
    var isNotEmpty = AboutCleanCode.EnumerableExtensions.Any(repository.FindAll());
    isNotEmpty = AboutCleanCode.EnumerableExtensions.Any(repository.FindAll());
    isNotEmpty = AboutCleanCode.EnumerableExtensions.Any(repository.FindAll());

    Assert.That(isNotEmpty, Is.True);
    Assert.That(repository.TransactionsCount, Is.EqualTo(0));
}

Surprisingly this test case fails!

Message: 
      Expected: 0
      But was:  3

In order to understand the root cause we have to understand how the C# compiler treats a method with yield return statements. Inspecting the compiled code with a dnSpy reveals that the generated code looks much different than what we have typed:

public class Repository
{
  public int TransactionsCount
  {
    get { return this.myTransactionsCount; }
  }

  public IEnumerable<string> FindAll()
  {
    Repository.<FindAll>d__3 <FindAll>d__ = new Repository.<FindAll>d__3(-2);
    <FindAll>d__.<>4__this = this;
    return <FindAll>d__;
  }

  private int myTransactionsCount = 0;

  [CompilerGenerated]
  private sealed class <FindAll>d__3 : IEnumerable<string>, IEnumerable, IEnumerator<string>, IEnumerator, IDisposable
  {
    [DebuggerHidden]
    public <FindAll>d__3(int <>1__state)
    {
      this.<>1__state = <>1__state;
      this.<>l__initialThreadId = Environment.CurrentManagedThreadId;
    }

    [DebuggerHidden]
    void IDisposable.Dispose()
    {
      int num = this.<>1__state;
      if (num == -3 || num - 1 <= 3)
      {
        try
        {
        }
        finally
        {
          this.<>m__Finally1();
        }
      }
    }

    bool IEnumerator.MoveNext()
    {
      bool result;
      try
      {
        switch (this.<>1__state)
        {
        case 0:
          this.<>1__state = -1;
          this.<>1__state = -3;
          this.<>4__this.myTransactionsCount = this.<>4__this.myTransactionsCount + 1;
          this.<>2__current = "a";
          this.<>1__state = 1;
          result = true;
          break;
        case 1:
          this.<>1__state = -3;
          this.<>2__current = "b";
          this.<>1__state = 2;
          result = true;
          break;
        case 2:
          this.<>1__state = -3;
          this.<>2__current = "c";
          this.<>1__state = 3;
          result = true;
          break;
        case 3:
          this.<>1__state = -3;
          this.<>2__current = "d";
          this.<>1__state = 4;
          result = true;
          break;
        case 4:
          this.<>1__state = -3;
          this.<>m__Finally1();
          result = false;
          break;
        default:
          result = false;
          break;
        }
      }
      catch
      {
        this.System.IDisposable.Dispose();
        throw;
      }
      return result;
    }

    private void <>m__Finally1()
    {
      this.<>1__state = -1;
      this.<>4__this.myTransactionsCount = this.<>4__this.myTransactionsCount - 1;
    }

    string IEnumerator<string>.Current
    {
      [DebuggerHidden]
      get { return this.<>2__current; }
    }

    [DebuggerHidden]
    void IEnumerator.Reset()
    {
      throw new NotSupportedException();
    }

    object IEnumerator.Current
    {
      [DebuggerHidden]
      get
      {
        return this.<>2__current;
      }
    }

    [DebuggerHidden]
    IEnumerator<string> IEnumerable<string>.GetEnumerator()
    {
      Repository.<FindAll>d__3 <FindAll>d__;
      if (this.<>1__state == -2 && this.<>l__initialThreadId == Environment.CurrentManagedThreadId)
      {
        this.<>1__state = 0;
        <FindAll>d__ = this;
      }
      else
      {
        <FindAll>d__ = new Repository.<FindAll>d__3(0);
        <FindAll>d__.<>4__this = this.<>4__this;
      }
      return <FindAll>d__;
    }

    [DebuggerHidden]
    IEnumerator IEnumerable.GetEnumerator()
    {
      return this.System.Collections.Generic.IEnumerable<System.String>.GetEnumerator();
    }

    private int <>1__state;

    private string <>2__current;

    private int <>l__initialThreadId;

    public Repository <>4__this;
  }
}

Whenever we use yield return the compiler generates a state machine. If we search for the code of our finally block we realize that it was moved to a separate method

    private void <>m__Finally1()
    {
      this.<>1__state = -1;
      this.<>4__this.myTransactionsCount = this.<>4__this.myTransactionsCount - 1;
    }

which is only called in two cases

  1. the enumerator is moved to the very end, which means the SAME enumerator is used to traverse the complete data (case 4)
  2. Dispose is called on the enumerator after usage

As our custom Any implementation only fetches only the first item AND does NOT call Dispose the <>m__Finally1 method is not called, so our code from the finally block is NOT executed!

Looking into the implementation of LinQ’s Any shows that Dispose is always called (note that IEnumerator<T> even derives from IDisposable so the using statement can be used):

public static bool Any<TSource>(this IEnumerable<TSource> source) {
    if (source == null) throw Error.ArgumentNull("source");
    using (IEnumerator<TSource> e = source.GetEnumerator()) {
        if (e.MoveNext()) return true;
    }
    return false;
}

(From: Microsoft Reference Source)

Now that the root cause is understood it is also clear how to fix the custom Any implementation:

public static bool Any(this IEnumerable self)
{
    var enumerator = self.GetEnumerator();
    try
    {
        return enumerator.MoveNext();
    }
    finally  
    {
        // important to call dispose if implemented by enumerator
        // to support "yield return" in "try-finally"
        (enumerator as IDisposable)?.Dispose();
    } 
}

Re-running the test case shows that the issue is fixed and the test case passes.

What about exceptions?

If our code would throw an exception while computing the value which should be returned using yield return then the state machine implemented by the compiler would catch this exception and call Dispose on the enumerator as well.

    bool IEnumerator.MoveNext()
    {
      bool result;
      try
      {
        switch (this.<>1__state)
        {
          // our code would be here and might through an exception
        }
      }
      catch
      {
        this.System.IDisposable.Dispose();
        throw;
      }
      return result;
    }

Sample Code

The sample code used in this post can be found at Plainionist/AboutCleanCode

Conclusion

If you call IEnumerable.GetEnumerator and work with the enumerator directly make sure you call Dispose when you are done!

Tags: coding-tips