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?
Since your resources are nested, your try-with clauses should also be:
My solution is to do a "extract method" refactoring, as following:
printToFile
can be written eitheror
For class lib designers, I will suggest them extend the
AutoClosable
interface with an additional method to suppress the close. In this case we can then manually control the close behavior.For language designers, the lesson is that adding a new feature could mean adding a lot others. In this Java case, obviously ARM feature will work better with a resource ownership transfer mechanism.
UPDATE
Originally the code above requires
@SuppressWarning
since theBufferedWriter
inside the function requiresclose()
.As suggested by a comment, if
flush()
to be called before close the writer, we need to do so before anyreturn
(implicit or explicit) statements inside the try block. There is currently no way to ensure the caller doing this I think, so this must be documented forwriteFileWriter
.UPDATE AGAIN
The above update makes
@SuppressWarning
unnecessary since it require the function to return the resource to the caller, so itself does not necessary being closed. Unfortunately, this pull us back to the beginning of the situation: the warning is now moved back to the caller side.So to properly solve this, we need a customised
AutoClosable
that whenever it closes, the underlineBufferedWriter
shall beflush()
ed. Actually, this shows us another way to bypass the warning, since theBufferWriter
is never closed in either way.