Closing resources - revisited

In a previous blog I described some ways to make sure your resources are closed properly. The examples I gave contained a bug and may cause that resources are still not properly closed (as pointed out by Anonymous and my colleague here on my project). Here is what was wrong with it..

try {
  // do something here
} finally {
  try {
    reader.close();
  } catch(IOException e) {
    log.info("closing fails");
  }
}

Like Anonymous pointed out the problem is that when everything in the try succeeds the closing of the reader could fail. This could mean that on some operating systems not everything is written to the file. And this of course needs to be reported back to the caller of our method. But throwing an Exception in finally can be very dangerous for reasons of Exception swallowing. So that led me to the conclusion you shouldn't close resources in the finally block.

Here is an implementation I think that does handle resources properly:

try {
  // do something here
  
  // close reader as last statement.
  reader.close();
} catch(Exception e) {
  try {
    reader.close();
  } catch(IOException closeFailed) {
    log.warn("now it is save to just log the exception as there is another exception that is more important than this one.", closeFailed);
  }
  throw new CustomRuntimeException(e);
}

So handling readers is even more complex than I initially thought. In my opinion this justifies the use of some template pattern to make sure that possible exceptions are handled properly... of course you need to make sure you don't implement bugs like I did in the template example of my previous blog 😉

Comments (2)

  1. Erwin Bolwidt - Reply

    October 3, 2007 at 5:20 pm

    There is another idiom that is commonly used with custom transaction management (to decide whether to commit or rollback), namely the "success indicator flag" idiom.
    The advantage is that you can still have common handling in the finally block, you only use an extra if-statement where you need to differentiate between failure and success in the try block.

    That would turn your example into:

    boolean success = false;
    try {
    // do something here
    success = true;
    } finally {
    try {
    reader.close();
    } catch(IOException e) {
    if (!success) {
    log.info("closing fails", e);
    } else {
    throw e;
    }
    }
    }

  2. Lars Vonk - Reply

    October 4, 2007 at 11:01 pm

    Hi Erwin, I think the "success indicator flag" is indeed a better solution. Thanks for your comment.

Add a Comment