Design Guideline update: put cleanup code in finally blocks


Another new guideline.. this one sparked a lot of discussion during our internal review, lets see if I cleaned it up well enough or not 😉

 

             

            Do put cleanup code in finally blocks.
Even code that immediately follows a catch() can be interrupted by an asynchronous exceptions (an exception caused by another thread, such as ThreadAbortException.  Once the try block is entered, your cleanup code will be guaranteed to run if you put it in a finally block.  Notice that the CLR uses a two pass exception model which means that exception filters
[1] all the way up the call stack are executed after the catch block but before the finally block.

Incorrect:  Cleanup() will not be executed in the face of an asynchronous exception

            try

            {

                //…

            }

            catch   //catching *ALL* exceptions and

  // swallowing them

            {      

                //…

            }

            //Execute clean up in the exceptional and

//non-exceptional case

            Cleanup();

 

Correct:  Cleanup() will be executed even in the face of an asynchronous exception*      

            try

            {

                //…

            }

            catch   // catching *ALL* exceptions and

// swallowing them

            {      

                //…

            }

            finally

            {

                //Execute clean up in the exceptional and

 // non-exceptional case

                Cleanup();

            }

Notice that in this example we catch all exceptions. That is rarely the correct coding practice.  It is used here to better illustrate the point of the guideline.  Please see section X.y on exception handling.

 

Many methods perform some form of cleanup.  All methods that perform cleanup should do so in a finally block.  Here is an example:

public void UpdateSet(){
   Set mySet = new Set();
   try{
     
      foreach (string s in GetValues() ) {
          mySet.Add(s);
      }     
   }finally{
      Set.Close();
   }
}

 




[1] Exception filters are not supported by C#, but supported by VB, IL, etc

*  There are some async exceptions such as StackOverflow and Rude Aborts that can cause even code in the finally to be skipped.  However these situations are relatively rare and don’t change the spirit of this guideline. 

Comments (15)

  1. Nicholas Allen says:

    Your cleanup code still isn’t guaranteed to run since, as you mentioned, an exception filter filter may be invoked, and that filter might never return control back to you. If you’re really paranoid about that case, I think it’s sufficient to do:

    try {



    } catch {

    throw;

    } finally {



    }

  2. RJ says:

    I don’t think that is sufficient Nicholas. The Throw statement would be executed before the Finally block. So you are back where you are started.

    You really need a 2nd try-catch block to force the finally to happen before leaving the function. However, I believe it is only a problem if the calling code is specifically trying to exploit your code.

  3. Marco Russo says:

    I think that in a guideline you should put more enphasis on the fact that a method with try/catch/finally blocks don’t necesssarily have to put all in this order. If you don’t need the resource that has to be cleaned up in the finally block, it would be better to put a try/finally inside a try/catch:

    try {

    try {

    }

    finally {

    }

    }

    catch (…) {

    }

    I’ve found many people who don’t realize you can write the code in this way, and probably lack of examples in MSDN or in guidelines could be a partial justification for that.

  4. David Levine says:

    The statement "Notice that the CLR uses a two pass exception model which means that exception filters[1] all the way up the call stack are executed after the catch block but before the finally block. " is not quite correct.

    It uses a two-pass model, but filters are invoked up the call stack before invoking the catch block, not after – the filter determines if the catch block is executed (filters are executed before both catch and finally blocks). When a filter returns a value indicating that the catch block should be executed the 1st pass has ended; the runtime notes the catch block that is to get control, then goes back down to the beginning of the stack where the exception occurred and begins the 2nd pass, executing finally blocks until it reaches the catch block, where normal execution continues. At this point the stack has been unwound.

    As a sidenote, if another exception is thrown in a filter the runtime swallows it and treats it as if it had returned a value indicating that the search for a catch block should continue. so the 1st pass continues up the call stack.

    Also, under normal conditions finally blocks are guaranteed to begin to run, but they are not guaranteed to run to completion. Code in the finally block can itself throw an exception, resulting in early termination of the finally block. A ThreadAbortException can be injected by another thread (or itself), interrupting a finally block. Also, if the runtime begins to exit the application (e.g. if a thread calls Environment.Exit), all threads are suspended, including those running finally blocks, and execution never resumes at all in those threads. The GC still runs finalizers but no other threads.

    Ensuring/guaranteeing that all critical paths in a finally block actually execute can be tricky. I understand that Whidbey introduces a new concept called a critical region to help with this but I have no direct experience with it yet.

  5. Nicholas Allen says:

    RJ- As far as I can tell, rethrowing causes finally blocks up to the current catch level to be unwound before walking the stack for the rethrow.

    David- Are you sure that Abort will interrupt out of a finally block? I wrote a test program to abort a thread that had an infinite loop in a finally block, and the thread never broke out of the loop.

  6. Joe Duffy says:

    David, note that the point about ThreadAborts interrupting finally has been fixed with v2.0. We now suspend the teardown of a thread until all finally blocks have completed execution. This means that transfer of control from within a try to its finally occurs as normal under a thread abort, and also that attempts to abort a thread while in its finally will merely let the runtime know about your desires (i.e. ThreadState = AbortRequested). The thread will then abort immediately once it has completed running all relevant finally blocks up the call stack.

    With that said, yes this is a nasty situation that can still occur in v1.1 and v1.0.

  7. RJ says:

    Nicholas – I only have v1.1 installed here at home, maybe other versions are different. But check out this VB code:

    Sub Outer()

    Try

    Debug.WriteLine("Outer try")

    Inner()

    Catch When Filter("Outer")

    Debug.WriteLine("Outer catch")

    Finally

    Debug.WriteLine("Outer finally")

    End Try

    End Sub

    Sub Inner()

    Try

    Debug.WriteLine("Inner try")

    Throw New Exception

    Catch When Filter("Inner")

    Debug.WriteLine("Inner catch")

    Throw

    Finally

    Debug.WriteLine("Inner finally")

    End Try

    End Sub

    Function Filter(ByVal name As String) As Boolean

    Debug.WriteLine(name + " filter")

    Return True

    End Function

    Note the output has the outer filter before the inner finally:

    [980] Outer try

    [980] Inner try

    [980] Inner filter

    [980] Inner catch

    [980] Outer filter <— pwned

    [980] Inner finally

    [980] Outer catch

    [980] Outer finally

    Moving the catch/throw to a higher scope does seem to solve the problem. Rewrite Inner as:

    Sub Inner()

    Try

    Try

    Debug.WriteLine("Inner try")

    Throw New Exception

    Finally

    Debug.WriteLine("Inner finally")

    End Try

    Catch When Filter("Inner")

    Throw

    End Try

    End Sub

    Outputs:

    [3732] Outer try

    [3732] Inner try

    [3732] Inner filter

    [3732] Inner finally

    [3732] Outer filter

    [3732] Outer catch

    [3732] Outer finally

    But 90% of the time, try-finally should be good enough with no catch at all (and looks much better, imo).

  8. David Levine says:

    Joe,

    Thanks for the information. Your description implies that it handles nesting of finally blocks to an arbitrary depth. Will this behavior occur for all finally blocks (this would be preferred), or is the guarantee only enforced for sections that are specially marked as critical?

    Nick, yes I am sure. There is no guarantee as to when the abort is actually injected into the thread. If your test code was spinning in a tight loop in a finally block then your results have been distorted by that thread hogging the cpu. Add a few sleep statements, or make a system call that puts the thread into an alertable wait state, and you should see it get aborted.

  9. Nicholas Allen says:

    RJ- thanks for the sample. I wrote mine in IL for 2.0 but running ildasm on yours doesn’t show any major differences. I’ll have to see if 2.0 is different or if I made an error.

    David- My infinite loop spends plenty of time sleeping although that doesn’t actually make a difference. What is important though is to try it on 2.0.

  10. Dmitriy Zaslavskiy says:

    I just in V2.0 Beta 1

    try

    {

    }

    catch

    {

    }

    Did NOT get aborted either!

    Can someone from MSFT comment on those changes in CLR

  11. Dmitriy Zaslavskiy says:

    Sorry previous email did not make much sense.

    Here is a sample

    try

    {

    }

    finally

    {

    Console.WriteLine("I am in finally");

    Thread.Sleep(5000);

    Console.WriteLine("I am done with finally ");

    }

    }

    When Abort is called from a different thread in

    V1 I am done with finally is NOT printed and in V2 it is.

    (Fine we just discussed this change)

    try

    {

    Console.WriteLine("Calling from thread X”);

    throw new NotImplementedException();

    }

    catch

    {

    Console.WriteLine("I am in catch");

    Thread.Sleep(5000);

    Console.WriteLine("I am done with catch");

    }

    Prints I am done with catch in V1 and V2

    So the code could have been written

    Try

    {

    }

    Catch

    {

    ….

    Cleanup()

    }

    And would work in both v1 and v2.

  12. Dmitriy Zaslavskiy says:

    Actually the code below doesn’t make sence

    So the code could have been written

    Try

    {

    }

    Catch

    {

    ….

    Cleanup()

    }

    And would work in both v1 and v2

  13. Dmitriy,

    You cleanup code will never execute if your "…" throws another exception. Placing your cleanup code in "finally" will always execute, even if an exception is thrown from your catch block.

  14. David Levine says:

    Nick,

    You are correct; I was referring to behavior under v1.0 and v1.1. Once the guarantees that Joe referred are made then the finally should finish executing before the abort is injected – this is a significant change. AFAIK the other restriction (threads being suspended when the system is exiting) still holds true.

    Seems like there are enough changes between v1.1/1.0 and 2.0 that we need to reference the version when talking about behavior.