Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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?

share|improve this question
2  
If this is Java, and you can use Java 7, check out if ARM blocks can solve your problem. –  Landei Mar 1 '13 at 12:11
    
@Landei, this solves it, but unfortunately we're not using Java 7. –  superM Mar 1 '13 at 12:15
    
I would say that the code you have shown isn't "Using a throw statement inside a finally block" and as such the logical progression is just fine. –  Mike Mar 1 '13 at 13:08
    
@Mike, I've used the standard summary that Fortify shows, but directly or indirectly there is an exception thrown inside finally. –  superM Mar 1 '13 at 13:10

3 Answers 3

up vote 10 down vote accepted

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.

share|improve this answer
    
+1 Though I'm among those who use that nasty code. But to my justification I will say that I don't do this always, only when the exception isn't critical. –  superM Mar 1 '13 at 13:43
1  
I find myself doing it as well. Some times creating any entire resource manager (anonymous or not) detracts from the purpose of the code. –  Travis Parks Mar 1 '13 at 13:45
    
+1 from me too; you basically said exactly what I was going to but better. Worth noting is that some of the Stream implementations in Java can't actually throw Exceptions on close(), but the interface declares it because some of them do. So, in some circumstances you might be adding a catch block which will never actually be needed. –  Baqueta Mar 1 '13 at 13:50
    
What's so nasty about using a try/catch block within a finally block? I would rather do that than bloat all my code by having to wrap all my connections and streams, etc. just so they can be closed quietly - which, in itself, introduces a new problem of not knowing if closing down a connection or stream (or whatever) causes an exception - which is something you may actually like to know about or do something about... –  ban-geoengineering May 28 at 17:18

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

share|improve this answer
    
This is helpful sometimes. But in case when I actually need to throw an exception when the file cannot be closed, this approach hides the problems. –  superM Mar 1 '13 at 14:57

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.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.