throws Exception in finally blocks

Question

Is there an elegant way to handle exceptions that are thrown in 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?

Solution 1

I usually do it like this:

try {
  // 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 );
  }
}

Solution 2

I typically use one of the closeQuietly methods in org.apache.commons.io.IOUtils:

public static void closeQuietly(OutputStream output) {
    try {
        if (output != null) {
            output.close();
        }
    } catch (IOException ioe) {
        // ignore
    }
}

Solution 3

If you're using Java 7, and resource implements AutoClosable, you can do this (using InputStream as an example):

try (InputStream resource = getInputStream()) {
  // Use the resource.
}
catch( Exception ex ) {
  // Problem with the resource.
}

Solution 4

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):

Resource 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.

Solution 5

As of Java 7 you no longer need to explicitly close resources in a finally block instead you can use try-with-resources syntax. The try-with-resources statement is a try statement that declares one or more resources. A resource is an object that must be closed after the program is finished with it. The try-with-resources statement ensures that each resource is closed at the end of the statement. Any object that implements java.lang.AutoCloseable, which includes all objects which implement java.io.Closeable, can be used as a resource.

Assume the following code:

try( Connection con = null;
     Statement stmt = con.createStatement();
     Result rs= stmt.executeQuery(QUERY);)
{  
     count = rs.getInt(1);
}

If any exception happens the close method will be called on each of these three resources in opposite order in which they were created. It means the close method would be called first for ResultSetm then the Statement and at the end for the Connection object.

It's also important to know that any exceptions that occur when the close methods is automatically called are suppressed. These suppressed exceptions can be retrieved by getsuppressed() method defined in the Throwable class.

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

View more solutions

Comments

[Chuck Conway]

In your case if you used a "using" statement, it should clean up the resource.

[Chuck Conway]

My bad, I'm assuming it's C#.

[Dirk Vollmar]

Testing error conditions is in general a good practice, simply because exceptions are expensive.

[Sharon]

"Defensive Programming" is an outmoded paradigm. The bloated code which results from testing for all error conditions eventually causes more problems than it solves. TDD and handling exceptions is that modern approach IMHO

[Ken Henderson]

@Joe - I don't disagree you on testing for all error conditions, but sometimes it makes sense, especially in light of the difference (normally) in cost of a simple check to avoid the exception versus the exception itself.

[Tim Frey]

-1 Here, resource.Close() can throw an exception. If you need to close additional resources, the exception would cause the function to return and they will remain open. That's the purpose of the second try/catch in the OP.

[Tim Frey]

-1 For this one, too. What if you're trying to close multiple resources in a single finally block? If closing the first resource fails, the others will remain open once the exception is thrown.

[Ranger]

This is why I told Paul that you HAVE to catch exceptions if you want to make sure the finally block completes. Please read the WHOLE answer!

[OscarRyz]

Yeap, I use a very similar idiom. But I don't create a function for that.

[Egwor]

I'd log it, just in case you find some leaks in the future. That way you'd know where they might (not) be coming from

[OscarRyz]

@Egwor. I agree with you. This was just some quick smippet. I log it too and probaly use a catch is something could be done with the exception :)

[Vishy]

You can make this method more general with Closeable public static void closeQuietly(Closeable closeable) {

[Darron]

What if use(resource) throws Exception A and then resource.release() throws exception B? Exception A is lost...

[Darron]

Yes, Closeable is nice. It's a shame that many things (like JDBC resources) don't implement it.

[Ken Henderson]

@Outlaw - you're are missing my point if Close throws an exception, and the resource is open then by capturing and suppressing the exception how do I correct the problem? Hence why I let it propagate (its fairly rare that I can recover with it still open).

[Dave Jarvis]

The check for null is redundant. If the resource was null, then the calling method is broken should be fixed. Also, if the resource is null, that should probably be logged. Otherwise it results in a potential exception being silently ignored.

[Dave Jarvis]

Logging is a cross-cutting concern and best left out of code directly. Indirectly apply it using aspects.

[Darron]

The check for null is not always redundant. Think of "resource = new FileInputStream("file.txt")" as the first line of the try. Also, this question was not about aspect oriented programing which many people do not use. However, the concept that the Exception should not be just ignored was most compactly handled by showing a log statement.

[Triynko]

@Egwor: It's not always pointless to set it to null. i.e. A huge bitmap would hang out in memory until you exit the scope, even after disposing it, because the lingering reference to the bitmap would prevent it and its managed resources from being garbage collected. It's a kind of temporary memory leak, where you're done using something, but can't reclaim its memory for use until that reference is gone and the GC is able to collect it. In this case, it's probably pointless... but the faster you nullify references, the sooner the (possibly scarce) resources can be collected and reused.

[Triynko]

See stackoverflow.com/questions/680550/explicit-nulling . There's mentioning that the C# GC is smart enough to consider "last possible reference" too, which would tend to make explicit-nulling pointless.

[Denys Kniazhev-Support Ukraine]

+1 for the blog link. Additionally, I would at least log the ignore exception

[Paul]

You could also avoid calling resource.close(); resource = null in the try block, that is what finally blocks are for. Also note that you do not handle any exceptions thrown while "doing something fancy", which actually, I think I prefer better, to handle infrastructural exceptions at a higher application level down the stack.

[Grogi]

The resource.close() might throw and exception as well - i.e. when the buffer flush fails. This exception should never be consumed. However, if closing the stream as a result of previously raised exception, the resource should be quietly closed ignoring the exception and preserving the root cause.

[Nathan Hughes]

It seems incomplete that this answer doesn't mention the difference in behavior between this approach and how the OP's posted example code works.

[Nathan Hughes]

using try-with-resources throws an exception on close if the part in the try block completes normally but the close method doesn't, unlike what the OP code does. recommending it as a replacement without acknowledging the change in behavior seems potentially misleading.

[Soroosh]

It doesn't throws an exception , close method is automatically called are suppressed.

[Nathan Hughes]

try the case i described. try block completes normally, close throws something. and reread the page you posted the link to, suppression only applies when the try block throws something.

[Dmitry Ginzburg]

Resource => Closeable?

[abhisheknirmal]

I would recommend using try-with-resources whenever possible. With this, if some exception is thrown while closing the resource, it will be marked as Suppressed exception and you will not lose the main exception thrown from the try block. Also, i think you get these suppressed exceptions in your stack trace, so they are not lost too.