Throwing an exception inside finally

Question

Static code analyzers like Fortify "complain" when an exception might be thrown inside a finally block, saying that Using a throw statement inside a finally block breaks the logical progression through the try-catch-finally. Normally I agree with this. But recently I've come across this code:

SomeFileWriter writer = null; 
try { 
     //init the writer
     //write into the file
} catch (...) {
     //exception handling
} finally {
     if (writer!= null) writer.close();  
}

Now if the writer cannot be closed properly the writer.close() method will throw an exception. An exception should be thrown because (most probably) the file wasn't saved after writing.

I could declare an extra variable, set it if there was an error closing the writer and throw an exception after the finally block. But this code works fine and I'm not sure whether to change it or not.

What are the drawbacks of throwing an exception inside the finally block?

Answer 1

Basically, finally clauses are there to ensure proper release of a resource. However, if an exception is thrown inside the finally block, that guarantee goes away. Worse, if your main block of code throws an exception, the exception raised in the finally block will hide it. It will look like the error was cause by the call to close, not for the real reason.

Some people follow a nasty pattern of nested exception handlers, swallowing any exceptions thrown in the finally block.

SomeFileWriter writer = null; 
try { 
     //init the writer
     //write into the file
} finally {
    if (writer!= null) {
        try {
            writer.close();
        } catch (...) {
        }
    }
}

In older versions of Java, you can "simplify" this code by wrapping resources in classes that do this "safe" clean up for you. A good friend of mine creates a list of anonymous types, each that provide the logic for cleaning up their resources. Then his code simply loops over the list and calls the dispose method within the finally block.

Answer 2

What Travis Parks said is true that exceptions in the finally block will consume any return values or exceptions from the try...catch blocks.

If you're using Java 7, though, the problem can be solved by using a try-with-resources block. According to the docs, as long as your resource implements java.lang.AutoCloseable (most library writers/readers do now), the try-with-resources block will close it for you. The additional benefit here is that any exception that occurs while closing it will be suppressed, allowing the original return value or exception to pass up.

From

FileWriter writer = null;
try {
  writer = new FileWriter("myFile.txt");
  writer.write("hello");
} catch(...) {
  // return/throw new exception
} finally {
  writer.close(); // an exception would consume the catch block's return/exception
}

To

try (FileWriter writer = new FileWriter("myFile.txt")) {
  writer.write("hello");
} catch(...) {
  // return/throw new exception, always gets returned even if writer fails to close
}

http://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html

Answer 3

I think this is something that you will need to address on a case by case basis. In some cases what the analyser is saying is correct in that the code you have is not that great and needs a rethink. But there may be other cases when throwing or even re-throwing might be the best thing. It's not something you can mandate.