java - Correct idiom for managing multiple chained resources in try-with-resources block?

Question

The Java 7 try-with-resources syntax (also known as ARM block (Automatic Resource Management)) is nice, short and straightforward when using only one AutoCloseable resource. However, I am not sure what is the correct idiom when I need to declare multiple resources that are dependent on each other, for example a FileWriter and a BufferedWriter that wraps it. Of course, this question concerns any case when some AutoCloseable resources are wrapped, not only these two specific classes.

I came up with the three following alternatives:

1)

The naive idiom I have seen is to declare only the top-level wrapper in the ARM-managed variable:

static void printToFile1(String text, File file) {
    try (BufferedWriter bw = new BufferedWriter(new FileWriter(file))) {
        bw.write(text);
    } catch (IOException ex) {
        // handle ex
    }
}

This is nice and short, but it is broken. Because the underlying FileWriter is not declared in a variable, it will never be closed directly in the generated finally block. It will be closed only through the close method of the wrapping BufferedWriter. The problem is, that if an exception is thrown from the bw's constructor, its close will not be called and therefore the underlying FileWriter will not be closed.

2)

static void printToFile2(String text, File file) {
    try (FileWriter fw = new FileWriter(file);
            BufferedWriter bw = new BufferedWriter(fw)) {
        bw.write(text);
    } catch (IOException ex) {
        // handle ex
    }
}

Here, both the underlying and the wrapping resource are declared in the ARM-managed variables, so both of them will certainly be closed, but the underlying fw.close() will be called twice: not only directly, but also through the wrapping bw.close().

This should not be a problem for these two specific classes that both implement Closeable (which is a subtype of AutoCloseable), whose contract states that multiple calls to close are permitted:

Closes this stream and releases any system resources associated with it. If the stream is already closed then invoking this method has no effect.

However, in a general case, I can have resources that implement only AutoCloseable (and not Closeable), which doesn't guarantee that close can be called multiple times:

Note that unlike the close method of java.io.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.

3)

static void printToFile3(String text, File file) {
    try (FileWriter fw = new FileWriter(file)) {
        BufferedWriter bw = new BufferedWriter(fw);
        bw.write(text);
    } catch (IOException ex) {
        // handle ex
    }
}

This version should be theoretically correct, because only the fw represents a real resource that needs to be cleaned up. The bw doesn't itself hold any resource, it only delegates to the fw, so it should be sufficient to only close the underlying fw.

On the other hand, the syntax is a bit irregular and also, Eclipse issues a warning, which I believe is a false alarm, but it is still a warning that one has to deal with:

Resource leak: 'bw' is never closed

---

So, which approach to go for? Or have I missed some other idiom that is the correct one?

Answer 1

Here's my take on the alternatives:

1)

try (BufferedWriter bw = new BufferedWriter(new FileWriter(file))) {
    bw.write(text);
}

For me, the best thing coming to Java from traditional C++ 15 years ago was that you could trust your program. Even if things are in the muck and going wrong, which they often do, I want the rest of the code to be on best behaviour and smelling of roses. Indeed, the BufferedWriter might throw an exception here. Running out of memory wouldn't be unusual, for instance. For other decorators, do you know which of the java.io wrapper classes throw a checked exception from their constructors? I don't. Doesn't do code understandability much good if you rely upon that sort of obscure knowledge.

Also there's the "destruction". If there is an error condition, then you probably don't want to be flushing rubbish to a file that needs deleting (code for that not shown). Although, of course, deleting the file is also another interesting operation to do as error handling.

Generally you want finally blocks to be as short and reliable as possible. Adding flushes does not help this goal. For many releases some of the buffering classes in the JDK had a bug where an exception from flush within close caused close on the decorated object not be called. Whilst that has been fixed for some time, expect it from other implementations.

2)

try (
    FileWriter fw = new FileWriter(file);
    BufferedWriter bw = new BufferedWriter(fw)
) {
    bw.write(text);
}

We're still flushing in the implicit finally block (now with repeated close - this gets worse as you add more decorators), but the construction is safe and we have to implicit finally blocks so even a failed flush doesn't prevent resource release.

3)

try (FileWriter fw = new FileWriter(file)) {
    BufferedWriter bw = new BufferedWriter(fw);
    bw.write(text);
}

There's a bug here. Should be:

try (FileWriter fw = new FileWriter(file)) {
    BufferedWriter bw = new BufferedWriter(fw);
    bw.write(text);
    bw.flush();
}

Some poorly implemented decorators are in fact resource and will need to be closed reliably. Also some streams may need to be closed in a particular way (perhaps they are doing compression and need to write bits to finish off, and can't just flush everything.

Verdict

Although 3 is a technically superior solution, software development reasons make 2 the better choice. However, try-with-resource is still an inadequate fix and you should stick with the Execute Around idiom, which should have a clearer syntax with closures in Java SE 8.