Common try-with-resources idiom won't clean up properly

So I was re-reading some blogs on the JDK 7 language changes and I noticed the following idiom seems popular:

//
try (BufferedReader br = new BufferedReader(
        new FileReader("/tmp/click.xml"))) {
   System.out.println(br.readLine());
}

That seems all well and good; but consider what happens should something goes wrong constructing the BufferedReader. The contract for try-with-resources will only try to close and tidy up the declared field, if something goes wrong then FileReader is never closed.

So consider this example where the constructor of BufferedReader throws an Error, it could well be an OutOfMemoryError or any number of other failure modes.

//
package client;

import java.io.*;

public class BufferedFileExample {

    public static void main(String[] args) throws FileNotFoundException, 
                                                  IOException {

        try (BufferedReader br = new MyBufferedReader(
            new MyFileReader("/tmp/click.xml"))) {
            System.out.println(br.readLine());
        }
    }
    
    public static class MyFileReader extends FileReader 
    {

        public MyFileReader(String in) throws FileNotFoundException {
            super(in);
        }
        
        public void close() throws IOException {
            super.close();
            System.out.println("Close called");
        }
    }
    
    
    
    public static class MyBufferedReader extends BufferedReader 
    {

        public MyBufferedReader(Reader in) {
            super(in);
            throw new Error();
        }
    }
}

This gets us the following output and as such the close is not performed on the FileReader which until the finaliser is called. You are stuck until the gc wakes up in the future to tidy it up.

Exception in thread "main" java.lang.Error
 at client.BufferedFileExample$MyBufferedReader.(BufferedFileExample.java:38)
 at client.BufferedFileExample.main(BufferedFileExample.java:12)
Process exited with exit code 1.

Luckily there is an easy fix for this, the language change supports multiple entries in try statement, so you can modify this code to be:

//
try (FileReader fr = new MyFileReader("/tmp/click.xml");
     BufferedReader br = new MyBufferedReader(
        fr)) {
   System.out.println(br.readLine());
}

So if you run the code with the modification above then you find that close is now called:

Close called
Exception in thread "main" java.lang.Error
 at client.BufferedFileExample$MyBufferedReader.(BufferedFileExample.java:38)
 at client.BufferedFileExample.main(BufferedFileExample.java:12)
Process exited with exit code 1.

So problem solved, well unfortunately there is another wrinkle here, if we remove the code that throws the Error then we will see the following output:

Close called
Close called
Process exited with exit code 0.

So this is fine for classes that implement the existing java.io.Closeble interface because calling close for a second time is fine; but the more generic interface that is used for other closeable resources unfortunately doesn't have this restriction, so quote the spec

Note that unlike the close method of Closeable, this close method is not required to be idempotent. In other words, calling this close method more than once may have some visible side effect, unlike Closeable.close which is required to have no effect if called more than once. However, implementers of this interface are strongly encouraged to make their close methods idempotent.

So in order to using a split-try-with-resources idiom you first need to make sure that the resources have a idempotent close method. I suspect that most API will do this; but you need to check first and perhaps need to further wrap your objects to make this the case.

One final point with try-with-resource is the handling of exceptions when the close and construction fails. JDK 7 introduced the concept of suppressed exceptions, so you it make the split version of the code throw an exception both in the constructor or the MyBufferedReader and in the close of MyFileReader you get to see the following novel stack trace.

Close called
Exception in thread "main" java.lang.Error
 at client.BufferedFileExample$MyBufferedReader.(BufferedFileExample.java:38)
 at client.BufferedFileExample.main(BufferedFileExample.java:12)
 Suppressed: java.lang.Error: close
  at client.BufferedFileExample$MyFileReader.close(BufferedFileExample.java:27)
  at client.BufferedFileExample.main(BufferedFileExample.java:14)
Process exited with exit code 1.

This might not be what you calling code is expecting - certainly it might differ slightly from what the try/finally code you are replacing would produce, worth know thing that exceptions are handled slightly differently. For example if there is just an Error thrown during the close method would would see this odd self-referential stack trace.

Close called
Close called
Exception in thread "main" java.lang.Error: close
 at client.BufferedFileExample$MyFileReader.close(BufferedFileExample.java:28)
 at java.io.BufferedReader.close(BufferedReader.java:517)
 at client.BufferedFileExample.main(BufferedFileExample.java:15)
 Suppressed: java.lang.Error: close
  at client.BufferedFileExample$MyFileReader.close(BufferedFileExample.java:28)
  ... 1 more
Process exited with exit code 1.

So the lesson here is that the impact of some of the new JDK 7 constructs are going to a little bit subtler than we might expect. Many thanks to my valued colleague Mark Warner for talking this one through with me.