Please close the resource behind you.

How to close resources is not rocket science. But still I see in many projects, including my own, that resources are not properly closed. Most of the time people tend to forget to close the resources in a finally block or forget that closing a resource might also throw an Exception what may cause Exception swallowing.
In Java 5 the Closable interface was introduced which enables some convenient ways to handle closing resources.

Let's take a look at an example that I will use throughout this blog to show what problems can occur when handling resources and how we can refactor the sample code so that it safely closes the used resource.

public void readFile() throws IOException {

  BufferedReader reader = null;
  try {
    reader = new BufferedReader(new FileReader("foo.txt"));
    String line = null;
    while((line = reader.readLine()) != null) {
      String formattedLine = Parser.parseLine(line);
      // do other stuff
    }
  } finally {
    reader.close();
  }
}

Although the call to reader.close() is in a finally block, there is still something wrong with this code. Let's say Parser.parseLine(line) throws some RuntimeException. As you (should) know it is also possible that the reader.close() throws an IOException. If both methods throw the Exceptions we loose the RuntimeException thrown by Parser.parseLine(line) and we will never know the true reason why our code failed. This phenomenon is called Exception swallowing, which is a bad thing (I'll not go into any further detail on this topic).

We can refactor our code so we don't swallow the original exception:

public void copyFile() throws IOException {

  BufferedReader reader = null;
  try {
    reader = new BufferedReader(new FileReader("foo.txt"));
    String line = null;
    while((line = reader.readLine()) != null) {
      String formattedLine = Parser.parseLine(line);
      // do other stuff
    }
  } finally {
    try {
      reader.close();
    } catch(IOException e) {
      log.info("closing reader failed.", e);
    }
  }

This code now is safe from Exception swallowing and closes the resources it uses, but it looks ugly to me. I don't like the nested try-catch block. If we would have to close more than one resource it even gets worse as we need as many nested try-catch as there are resources. Let's see if we can refactor some more.

Like I said before as of Java 5 all resources that are closeable implement the Closeable interface. This gives us the possibility to create one utility method that handles the closing of resources and preventing exception swallowing. After refactoring the code looks like:

public void copyFile() throws IOException {

  BufferedReader reader = null;
  try {
    reader = new BufferedReader(new FileReader("foo.txt"));
    String line = null;
    while((line = reader.readLine()) != null) {
      String formattedLine = Parser.parseLine(line);
      // do other stuff
    }
  } finally {
    close(reader);
  }
}

private void close(Closeable closeable) {
  try {
    if(closeable != null) {
      closeable.close();
    }
  } catch(IOException e) {
    log.info("closing reader failed.", e);
  }
}

We can use the close(Closeable) method for every Closeable resource we need to close throughout our code. If we would have more than one resource in the method, a simple call to close(Closeable) is sufficient in the finally block. Great advantage of this approach is that your "main" method stays readable and the implementation of closing resources is implemented in one location.

There is still one problem though with this approach; people might still forget to put the call to the close(Closeable) method in a finally block or to call the close(Closeable) method at all.

A solution for this is the use of the Template pattern. Popular frameworks like the Spring framework make frequent use of the template pattern. Examples of such are for instance the HibernateTemplate, JdbcTemplate and many more. If we use this approach we can be sure that the resources are always closed properly in a finally block.

An implementation of such a template pattern could look like this:

public class CloseableTemplate {
  public void execute(CloseableCallbackHandler callbackHandler, T closeable) {
    try {
      callbackHandler.doWithCloseable(closeable);
    } finally {
       close(closeable);
    }
  }
  private void close(Closeable closeable) {
    try {
      if(closeable != null) {
        closeable.close();
      }
    } catch(IOException e) {
      log.info("closing reader failed.", e);
    }
  }
}

public interface CloseableCallbackHandler {
  void doWithCloseable(T closeable);
}

The implementation of the copyFile method would then look like:

public void copyFile() throws IOException {
  BufferedReader reader = new BufferedReader(new FileReader("foo.txt"));
  new CloseableTemplate().execute(
     new CloseableCallbackHandler(){
       @Override
       public void doWithCloseable(BufferedReader bufferedReader) {
         String line = null;
         while((line = reader.readLine()) != null) {
           String formattedLine = Parser.parseLine(line);
           // do other stuff
         }
       }
     }, bufferedReader);
  }
}

As you can see the try-finally block is no longer necessary in the copyFile method as the CloseableTemplate takes care of this. So if you want to be sure that resources are closed I suggest you use a similar approach in your project.

Lars

Comments (13)

  1. Will Sargent - Reply

    September 25, 2007 at 12:20 am

    What about passing the exception up stream:

    public void copyFile() throws IOException
    {
    BufferedReader reader = new BufferedReader(new FileReader("foo.txt"));
    try
    {
    while ((line = reader.readLine()) != null)
    {
    String formattedLine = Parser.parseLine(line);
    // do other stuff
    }
    } finally
    {
    reader.close();
    }
    }

    or (if you want to do the logging in house):

    public void copyFile()
    {
    BufferedReader reader = new BufferedReader(new FileReader("foo.txt"));
    try
    {
    try
    {
    while ((line = reader.readLine()) != null)
    {
    String formattedLine = Parser.parseLine(line);
    // do other stuff
    }
    } finally
    {
    reader.close();
    }
    } catch (IOException e)
    {
    log.info("io exception", e);
    }
    }

    In the second case, but at least the try / catch block is on the outside, where it should be.

  2. WarpedJavaGuy - Reply

    September 25, 2007 at 8:54 am

    Can't help but think that closures (proposed for Java 7) would provide the ultimate solution:

    BufferedReader reader =
    new BufferedReader(new FileReader("foo.txt"));
    with (BufferedReader in : reader) {
    String line = null;
    while((line = in.readLine()) != null) {
    String formattedLine = Parser.parseLine(line);
    // do other stuff
    }
    }

  3. Marc - Reply

    September 25, 2007 at 9:33 am

    In reply to Wils comment: in both cases you caught or passed all of the IOExceptions (also the ones f.i. readLine could throw). I would go with Lars' approach, alltough in Java 1.4 your stuck with the 'ugly' version. Minor detail, Lars, you start off with a readLine method.

  4. Anonymous - Reply

    September 25, 2007 at 10:04 am

    log.info("...", e);
    isn't really an option. It's not better than swallowing the exception.

  5. Lars Vonk - Reply

    September 25, 2007 at 1:38 pm

    @Anonymous: You could of course log it on any log level. Or check it there is an exception thrown or not and depending on that throw the exception that occurred on the close method.
    But not handling the Exception like in my first example is worse as you are then not even aware of the original exception.

  6. Gert-Jan van der Heiden - Reply

    September 25, 2007 at 2:11 pm

    Not much of an improvement. About the same amount of code, only the solution is not really readable.

  7. Gert-Jan van der Heiden - Reply

    September 25, 2007 at 2:18 pm

    I dont think it is much of an improvement. The amount of code isn't drastically changed. Also, IMHO, the code isn't easy to read.

    In the finally block, the reader should be checked for not null.

  8. Lars Vonk - Reply

    September 25, 2007 at 2:32 pm

    Hi Gert-Jan, not much of an improvement compared to what? I am guessing you are referring to the Template-Callback approach versus the call to the close method in the finally block. I agree with you that the inline Template-Callback approach is not really readable but it does guarantee that the resources will be closed (you don't have to worry about the try-finally anymore). Maybe if you would create a separate class instead of the inlining it would be more readable...

  9. Anonymous - Reply

    September 25, 2007 at 11:32 pm

    Hello again!

    1. Your code has a severe bug. Consider: Your function succeeds, only reader.close() throws an exception. Then

    finally {
    ..try {
    ....reader.close();
    ..} catch(IOException e) {
    ....log.info("closing reader failed.", e);
    ..}
    }

    simply swallows the exception from reader.close(). This is tolerable for a Reader but certainly not in general e.g. for a Writer and other 'resources'.

    2. It is very hard to maintain the original exception when another exception occurs during stack unwinding.

    3. 'Hidden' exceptions are a mere theoretical problem. They are hardly ever seen in real world applications and it makes no sense to add extra code to handle them.

  10. Lars Vonk - Reply

    September 26, 2007 at 8:55 am

    Hi Anonymous,

    Thanks for your comments.

    @1: I don't consider logging an Exception the same as swallowing. I consider swallowing to be empty catch blocks and in case you "miss" an exception due to the reason I described in my blog.
    Like I said maybe logging on info level is not the best option but at least it will be looked at by the application maintainer (at least you hope ;-)).
    Personally I would not propagate the IOException that occurred for closing a resource all the way up to the end user. What can he do about it?

    @2: true that's why I guess you need solid finally blocks.

    @3: true, but I hope I showed that with little effort or even less effort in case of the Template approach you can save yourself a lot of problems in those "hardly" cases.
    (It is less because you only have to create the Template once and can use that throughout you application(s)).

  11. Anonymous - Reply

    September 29, 2007 at 2:04 pm

    Hey, why haven't you fixed YOUR BUGGY CODE yet?

    public void copyFile() throws IOException {
    ..BufferedReader reader = null;
    ..try {
    ....reader = new BufferedReader(new FileReader("foo.txt"));
    ....String line = null;
    ....while((line = reader.readLine()) != null) {
    ......String formattedLine = Parser.parseLine(line);
    ....}
    ....// everything fine here: NO exception!
    ..} finally {
    ....try {
    ......// now reader.close() throws an exception
    ......reader.close();
    ....} catch(IOException e) {
    ......log.info("closing reader failed and my caller will never know!", e);
    ....}
    ..}
    // return normally without any exception
    }

  12. Lars Vonk - Reply

    October 2, 2007 at 8:36 pm

    I see your point. If closing fails it can be possible that not everything is written to the file. So instead of closing it in finally it should be closed in the catch blocks. Here is another version of the template class:

    public class CloseableTemplate {
    ..public void execute(CloseableCallbackHandler
    callbackHandler, T closeable) {
    ....try {
    ......callbackHandler.doWithCloseable(closeable);
    ......// close reader normally, we are interested whether or not this fails.
    ......reader.close();
    ....} catch(Exception e) {
    ......close(closeable);
    ......// rethrow as runtime
    ......throw new CustomRuntimeException(e);
    ....}
    ..}
    ..private void close(Closeable closeable) {
    ....try {
    ......if(closeable != null) {
    ........closeable.close();
    ......}
    ....} catch(IOException e) {
    ......log.info(\\\"closing reader failed but since there is another exception that occurred first we only log this one.\\\", e);
    ....}
    ..}
    }

  13. Xebia Blog - Reply

    October 3, 2007 at 2:57 pm

    [...] 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.. [...]

Add a Comment