«    »

A Tale of Bad Exception Handling in Finally Blocks in Java

There is always something more to learn. That was the lesson for me last week when I learned something new about the Java programming language, despite having used professionally it for almost 10 years.

I was upgrading a Java web application to WebSphere server version 6.1 and as the first step I switched the development environment to Rational Application Developer version 7. With the new IDE came an improved compiler that reported additional warnings, so it didn't surprise me to see hundreds of new warnings. It is a standard practice of mine to eliminate warnings, even harmless ones, since a significant warning can easily be missed if harmless warnings are allowed to remain. As I worked my way through the warnings, I came across a new one I did not recognize: "Finally block does not complete normally". A simplified version of the code producing this warning is shown below:

  boolean performBusinessOperation() {
    boolean operationResult = false;
    try {
      // Perform some business logic...
      operationResult = true;
    } catch (IllegalStateException e) {
      // Handle this exception..
      operationResult = false;
    } catch (IllegalArgumentException e) {
      // Handle this exception...
      operationResult = false;
    } finally {
      // Common cleanup...
      // Following line produces warning
      // "Finally block does not complete normally"
      return operationResult;
    }
  }

This was not code I had written, so I spent some time trying to figure out the reason for the warning. In Java, the finally block is guaranteed to be executed after the contained try block finishes execution, even if an exception is thrown within the try block. If an exception is thrown and not caught within a function, the exception is propagated up the call stack until it encounters an appropriate catch block. But in this particular case within the performBusinessOperation method, if an uncaught exception is thrown, the finally block will run and perform the return statement. So which will win - the exception or the return? I was not sure, which in my mind explained the warning - it is bad practice to write code with unclear behavior. So I fixed the code by moving the return statement outside the finally block and moved on to the next warning.

Once I was finished eliminating warnings, I ran the entire suite of automated unit tests. To my surprise, I had a few failures. When I tracked down the offending code, I was surprised to see that it was my fix for the "Finally block does not complete normally" warning that broke the tests. How could that be? After further tracing and debugging, I finally found the reason: the unit test incorrectly invoked the method in question, causing it to throw a NullPointerException from within the try block. Having the return statement within the finally block apparently was causing the exception to be silently discarded. I found this shocking. This is dangerous behavior for a language, and I had problems believing that was actually the case. So I wrote a quick unit test for verification, shown below.

public class ReturnInFinallyBlockExample
  extends junit.framework.TestCase
{
  @SuppressWarnings("finally")
  private boolean isReturnWithinFinally() {
    try {
      if (true) throw new RuntimeException();
    } finally {
      return true; // This hides the exception
    }
  }

  private boolean isReturnOutsideFinally() {
    try {
      if (true) throw new RuntimeException();
    } finally {
      // Return outside finally block.
    }
    return true;
  }

  public void testReturnFromFinallyBlockWithUnhandledException() {
    assertTrue(isReturnWithinFinally());
    try {
      isReturnOutsideFinally();
      fail("Expect exception");
    } catch (RuntimeException e) {
      // Expected case.
    }
  }
}

This test case passes, demonstrating that the uncaught exception within the try block is silently discarded if the return statement is within the finally block. Note my use of the Java 5 annotation @SuppressWarnings("finally") in order to stop the compiler from reporting the "Finally block does not complete normally" warning for this example.

Perhaps I should have been less surprised by this behavior in Java given that I was already aware of another suboptimal situation regarding Java exception handling in finally blocks: if an uncaught exception is thrown in a try block and then another exception is thrown in the finally block, it will be the second exception that is propagated out of the method. The first exception will be silently lost. The following test case demonstrates this behavior:

public class ExceptionInFinallyBlockExample
  extends junit.framework.TestCase
{
  private void haveExceptionInFinallyBlock() {
    try {
      if (true) throw new IllegalArgumentException();
    } finally {
      if (true) throw new NullPointerException();
    }
  }
  public void testHaveExceptionInFinallyBlock() {
    try {
      haveExceptionInFinallyBlock();
      fail("Expect exception");
    } catch (NullPointerException e) {
      // Expected case.
    }
  }
}

Ignoring errors is dangerous, as I have discussed in my article on Error Handling and Reliability. So I strongly feel that the Java language should have prohibited return statements in finally blocks. Fortunately, modern Java IDEs like Eclipse can make up for this shortcoming by allowing you to flag this code construct as an error rather than a warning.

The source code listed in this article is provided in the Java Examples project which can be downloaded from the Software page.

If you find this article helpful, please make a donation.

14 Comments on “A Tale of Bad Exception Handling in Finally Blocks in Java”

  1. stu says:

    Funny, I was asked in an interview what would happen in exactly this case.. (and wasn’t sure) so it seems people do use it

  2. JO says:

    Note that this applies to return statements as well!
    I.e. if the finally clause contains a return statement (and it is executed) THAT becomes the return value of the method, REGARDLESS of any other return statements executed within the try-finally block!

  3. Raph says:

    It would be funny if only it were more difficult to introduce this kind of bug. This kind of behavior has been warned by tools since a very long time now. I’m surprised this is the first time this reaches reddit.

  4. Geza says:

    Well explained!
    I’ve just looked up this site with google because I experienced the same behaviour in the code I wrote. It is suprising that Java/finally block works like this.

  5. Geza says:

    Hmmm. The problem in my case was that I used return in the finally block, after I removed that outside, the RuntimeException is propagated appropriately.

    Conslusion: do not use return statement in finally block, it can ruin your exception propagation or return policy.

  6. Arao says:

    do not u guys read Practical Java??

  7. Student says:

    So what is your recommended implementation if the caller of performBusinessOperation() expects a valid return value that only performBusinessOperation() can return. For example, an error code.

  8. @Student, you can do that by returning a special error code from a catch block. This clearly signals what is going on. Just don’t put a return statement in the finally block.

  9. chaumant says:

    No doubt, Its a good and helpful article.
    But it leaves certain questions to me.
    I am in the process of developing a code, which actually process set of teams. I wanted to continue if exception is thrown while processing Teams.
    And the moment it throws exception, it the process should send a mail for that team.
    for(team : Teams){
    try{
    //process Teams
    }
    catch(SomeException se){
    //Send mail if exception comes
    }
    catch(Exception e){
    //Send mail if exception comes
    }
    finally{
    //cleanup code for the team
    continue; // Ofcourse this line is giving waring – “Finally block does not complete normally” and i am also not interested in any warning
    }
    }

    Please advise..what could be alternatives.

  10. @chaumant, it sounds like in your catch blocks you do not want to re-throw an exception – just send an email. In this case execution will continue after the catch blocks, so you could just eliminate the finally block and have the cleanup code afterwards.

  11. Hi,
    I am just doing one interesting thing… Instead of catching a exception properly .. I just put one more try/catch inside the finally clause and expecting that exception will catched be here and no abnormal termination will occur but that doesn’t happen.
    Code :

    private void method1() {
    try {
    method2();
    } finally {
    System.out.println(“Finally executes”);
    try {

    } catch (Exception e) {
    System.out.println(“caught here in finally”);
    }
    }
    }

    private void method2() {
    throw new RuntimeException(“Exception thrown from here.”);
    }
    Can you please see over this??

  12. Hi Abhishek.

    I assume you want the try…catch block within the finally to catch the RuntimeException thrown by method2(). This won’t work because the RuntimeException is thrown outside of that try..catch block. In other words, exceptions thrown from within a try…finally block are NOT re-thrown from within the finally block, and thus cannot be caught there.

    The solution is to add a catch to the try…finally:
    private void method1() {
    try {
    method2();
    } catch (Exception e) {
    System.out.println(“Will catch exception from method2.”);
    } finally {
    System.out.println(“Finally executes”);
    }
    }

    private void method2() {
    throw new RuntimeException(“Exception thrown from here.”);
    }

  13. dayDreamer says:

    Hello,
    I understood your solution for Abhisheks question..but what is wrong in saying this…
    Lets have a step-by-step execution.
    1.RuntimeException thrown
    2.no catch statement,so it is still waiting for someone to catch
    3.finally encountered. (still exception not caught)
    4.it has a try and catch block.
    5.catch(Exception e) waiting for an exception if any.
    6.An uncaught exception found.(Maybe he found somewhere in buffer or so..)
    7.caught!!
    8.finish

    At what point am i wrong?
    Thanks.

  14. @dayDreamer if #7 (caught) is referring to catching the exception raised in #6, then you are right in how it would work. If you are expecting #7 to catch the exception from #1, I don’t believe it will, as exception #1 was not thrown within the scope of the #4/#5 try…catch block.

    But it is easy to write a simple test and see what will happen, like I did in my article. No need to take my word for it :)

«    »