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 😉