Is there an elegant way to handle exceptions that are thrown in the a finally block?
For example:
try {
// Use the resource.
}
catch( Exception ex ) {
// Problem with the resource.
}
finally {
try{
resource.close();
}
catch( Exception ex ) {
// Could not close the resource?
}
}
How do you avoid the try / catch in the finally block?
Source: Tips4all
I usually do it like this:
ReplyDeletetry {
// Use the resource.
} catch( Exception ex ) {
// Problem with the resource.
} finally {
// Put away the resource.
closeQuietly( resource );
}
Elsewhere:
protected void closeQuietly( Resource resource ) {
try {
if (resource != null) {
resource.close();
}
} catch( Exception ex ) {
log( "Exception during Resource.close()", ex );
}
}
I typically use one of the closeQuietly methods in org.apache.commons.io.IOUtils:
ReplyDeletepublic static void closeQuietly(OutputStream output) {
try {
if (output != null) {
output.close();
}
} catch (IOException ioe) {
// ignore
}
}
Arguably a bit over the top, but maybe useful if you're letting exceptions bubble up and you can't log anything from within your method (e.g. because it's a library and you'd rather let the calling code handle exceptions and logging):
ReplyDeleteResource resource = null;
boolean isSuccess = false;
try {
resource = Resource.create();
resource.use();
// Following line will only run if nothing above threw an exception.
isSuccess = true;
} finally {
if (resource != null) {
if (isSuccess) {
// let close throw the exception so it isn't swallowed.
resource.close();
} else {
try {
resource.close();
} catch (ResourceException ignore) {
// Just swallow this one because you don't want it
// to replace the one that came first (thrown above).
}
}
}
}
UPDATE: I looked into this a bit more and found a great blog post from someone who has clearly thought about this more than me: http://illegalargumentexception.blogspot.com/2008/10/java-how-not-to-make-mess-of-stream.html He goes one step further and combines the two exceptions into one, which I could see being useful in some cases.
One solution, if the two Exceptions are two different classes
ReplyDeletetry {
...
}
catch(package1.Exception err)
{
...
}
catch(package2.Exception err)
{
...
}
finally
{
}
But sometimes you cannot avoid this second try-catch. e.g. for closing a stream
InputStream in=null;
try
{
in= new FileInputStream("File.txt");
(..)// do something that might throw an exception during the analysis of the file, e.g. a SQL error
}
catch(SQLException err)
{
//handle exception
}
finally
{
//at the end, we close the file
if(in!=null) try { in.close();} catch(IOException err) { /* ignore */ }
}
Why do you want to avoid the additional block? Since the finally block contains "normal" operations which may throw an exception AND you want the finally block to run completely you HAVE to catch exceptions.
ReplyDeleteIf you don't expect the finally block to throw an exception and you don't know how to handle the exception anyway (you would just dump stack trace) let the exception bubble up the call stack (remove the try-catch from the finally block).
If you want to reduce typing you could implement a "global" outer try-catch block, which will catch all exceptions thrown in finally blocks:
try {
try {
...
} catch (Exception ex) {
...
} finally {
...
}
try {
...
} catch (Exception ex) {
...
} finally {
...
}
try {
...
} catch (Exception ex) {
...
} finally {
...
}
} catch (Exception ex) {
...
}
try {
ReplyDeletefinal Resource resource = acquire();
try {
use(resource);
} finally {
resource.release();
}
} catch (ResourceException exx) {
... sensible code ...
}
Job done. No null tests. Single catch, include acquire and release exceptions. Of course you can use the Execute Around idiom and only have to write it once for each resource type.
If you're using Java 7, and resource implements AutoClosable, you can do this (using InputStream as an example):
ReplyDeletetry (InputStream resource = getInputStream()) {
// Use the resource.
}
catch( Exception ex ) {
// Problem with the resource.
}
If you can you should test to avoid the error condition to begin with.
ReplyDeletetry{...}
catch(NullArgumentException nae){...}
finally
{
//or if resource had some useful function that tells you its open use that
if (resource != null)
{
resource.Close();
resource = null;//just to be explicit about it was closed
}
}
Also you should probably only be catching exceptions that you can recover from, if you can't recover then let it propagate to the top level of your program. If you can't test for an error condition that you will have to surround your code with a try catch block like you already have done (although I would recommend still catching specific, expected errors).
You could refactor this into another method ...
ReplyDeletepublic void RealDoSuff()
{
try
{ DoStuff(); }
catch
{ // resource.close failed or something really weird is going on
// like an OutOfMemoryException
}
}
private void DoStuff()
{
try
{}
catch
{
}
finally
{
if (resource != null)
{
resource.close();
}
}
}
I usually do this:
ReplyDeleteMyResource r = null;
try {
// use resource
} finally {
if( r != null ) try {
r.close();
} catch( ThatSpecificExceptionOnClose teoc ){}
}
Rationale: If I'm done with the resource and the only problem I have is closing it, there is not much I can do about it. It doesn't make sense either to kill the whole thread if I'm done with the resource anyway.
This is one of the cases when at least for me, it is safe to ignore that checked exception.
To this day I haven't had any problem using this idiom.
After lots of consideration, I find the following code best:
ReplyDeleteMyResource resource = null;
try {
resource = new MyResource();
resource.doSomethingFancy();
resource.close();
resource = null;
} finally {
closeQuietly(resource)
}
void closeQuietly(MyResource a) {
if (a!=null)
try {
a.close();
} catch (Exception e) {
//ignore
}
}
That code guarantees following:
The resource is freed when the code finished
Exceptions thrown when closing the resource are not consumed without processing them.
The code does not try to close the resource twice, no unnecessary exception will be created.
Ignoring exceptions which occur in a 'finally' block is generally a bad idea unless one knows what those exceptions will be and what conditions they will represent. In the normal try/finally usage pattern, the try block places things into a state the outside code won't be expecting, and the finally block restores those things' state to what the outside code expects. Outside code which catches an exception will generally expect that, despite the exception, everything has been restored to a normal state. For example, suppose some code starts a transaction and then tries to add two records; the "finally" block performs a "rollback if not committed" operation. A caller might be prepared for an exception to occur during the execution of the second "add" operation, and may expect that if it catches such an exception, the database will be in the state it was before either operation was attempted. If, however, a second exception occurs during the rollback, bad things could happen if the caller makes any assumptions about the database state. The rollback failure represents a major crisis--one which should not be caught by code expecting a mere "Failed to add record" exception.
ReplyDeleteMy personal inclination would be to have a finally method catch exceptions that occur and wrap them in a "CleanupFailedException", recognizing that such failure represents a major problem and such an exception should not be caught lightly.