Blog

Please close the resource behind you.

24 Sep, 2007

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

guest
13 Comments
Oldest
Newest Most Voted
Inline Feedbacks
View all comments
Will Sargent
Will Sargent
14 years ago

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.

WarpedJavaGuy
14 years ago

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
}
}

Marc
Marc
14 years ago

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.

Anonymous
Anonymous
14 years ago

log.info(“…”, e);
isn’t really an option. It’s not better than swallowing the exception.

Gert-Jan van der Heiden
Gert-Jan van der Heiden
14 years ago

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

Gert-Jan van der Heiden
Gert-Jan van der Heiden
14 years ago

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.

Anonymous
Anonymous
14 years ago

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.

Anonymous
Anonymous
14 years ago

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
}

trackback
14 years ago

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

Explore related posts