A nice feature of the Java 7 try-with-resources statement and the AutoCloseable type that was introduced to work with this statement is the fact that static code analysis tools can detect resource leaks. For instance, Eclipse:
When you have the above configuration and you try running the following program, you’ll get three warnings:
public static void main(String[] args)
throws Exception {
Connection c = DriverManager.getConnection(
"jdbc:h2:~/test", "sa", "");
Statement s = c.createStatement();
ResultSet r = s.executeQuery("SELECT 1 + 1");
r.next();
System.out.println(r.getInt(1));
}
The output is, trivially
2
The warnings are issued on all of c, s, r. A quick fix (don’t do this!) is to suppress the warning using an Eclipse-specific SuppressWarnings parameter:
After all, WeKnowWhatWeReDoing™ and this is just a simple example, right?
Wrong!
The right way to fix this, even for simple examples (at least after Java 7) is to use the effortless try-with-resources statement.
public static void main(String[] args)
throws Exception {
try (Connection c = DriverManager.getConnection(
"jdbc:h2:~/test", "sa", "");
Statement s = c.createStatement();
ResultSet r = s.executeQuery("SELECT 1 + 1")) {
r.next();
System.out.println(r.getInt(1));
}
}
In fact, it would be great if Eclipse could auto-fix this warning and wrap all the individual statements in a try-with-resources statement. Upvote this feature request, please!
Great, we know this. What’s the deal with Java 8?
In Java 8, the contract on AutoCloseable has changed very subtly (or bluntly, depending on your point of view).
Java 7 version
A resource that must be closed when it is no longer needed.
An object that may hold resources (such as file or socket handles) until it is closed. The close() method of an AutoCloseable object is called automatically when exiting a try-with-resources block for which the object has been declared in the resource specification header. This construction ensures prompt release, avoiding resource exhaustion exceptions and errors that may otherwise occur.
API Note:
It is possible, and in fact common, for a base class to implement AutoCloseable even though not all of its subclasses or instances will hold releasable resources. For code that must operate in complete generality, or when it is known that the AutoCloseable instance requires resource release, it is recommended to use try-with-resources constructions. However, when using facilities such as Stream that support both I/O-based and non-I/O-based forms, try-with-resources blocks are in general unnecessary when using non-I/O-based forms.
In short, from Java 8 onwards, AutoCloseable is more of a hint saying that you might be using a resource that needs to be closed, but this isn’t necessarily the case.
This is similar to the Iterable contract, which doesn’t say whether you can iterate only once, or several times over the Iterable, but it imposes a contract that is required for the foreach loop.
When do we have “optionally closeable” resources?
Take jOOQ for instance. Unlike in JDBC, a jOOQ Query (which was made AutoCloseable in jOOQ 3.7) may or may not represent a resource, depending on how you execute it. By default, it is not a resource:
try (Connection c = DriverManager.getConnection(
"jdbc:h2:~/test", "sa", "")) {
// No new resources created here:
ResultQuery<Record> query =
DSL.using(c).resultQuery("SELECT 1 + 1");
// Resources created and closed immediately
System.out.println(query.fetch());
}
The output is again:
+----+
| 2|
+----+
| 2|
+----+
But now, we have again an Eclipse warning on the query variable, saying that there is a resource that needs to be closed, even if by using jOOQ this way, we know that this isn’t true. The only resource in the above code is the JDBC Connection, and it is properly handled. The jOOQ-internal PreparedStatement and ResultSet are completely handled and eagerly closed by jOOQ.
Then, why implement AutoCloseable in the first place?
jOOQ inverses JDBC’s default behaviour.
In JDBC, everything is done lazily by default, and resources have to be closed explicitly.
In jOOQ, everything is done eagerly by default, and optionally, resources can be kept alive explicitly.
For instance, the following code will keep an open PreparedStatement and ResultSet:
try (Connection c = DriverManager.getConnection(
"jdbc:h2:~/test", "sa", "");
// We "keep" the statement open in the ResultQuery
ResultQuery<Record> query =
DSL.using(c)
.resultQuery("SELECT 1 + 1")
.keepStatement(true)) {
// We keep the ResultSet open in the Cursor
try (Cursor<Record> cursor = query.fetchLazy()) {
System.out.println(cursor.fetchOne());
}
}
With this version, we no longer have any warnings in Eclipse, but the above version is really the exception when using the jOOQ API.
The same thing is true for Java 8’s Stream API. Interestingly, Eclipse doesn’t issue any warnings here:
Resource leak detection seems to be a nice IDE / compiler feature at first. But avoiding false positives is hard. Specifically, because Java 8 changed contracts on AutoCloseable, implementors are allowed to implement the AutoCloseable contract for mere convenience, not as a clear indicator of a resource being present that MUST be closed.
This makes it very hard, if not impossible, for an IDE to detect resource leaks of third party APIs (non-JDK APIs), where these contracts aren’t generally well-known. The solution is, as ever so often with static code analysis tools, to simply turn off potential resource leak detection:
For more insight, see also this Stack Overflow answer by Stuart Marks, linking to the EG’s discussions on lambda-dev
8 thoughts on “A Subtle AutoCloseable Contract Change Between Java 7 and Java 8”
How do we interpret it…
I create an API with AutoClosable classes and then I declare that the caller MUST close the resource (instance of my class) calling ‘close’ because I said that.
I may not actually hold a resource myself that I need to close. That is implementation detail. I may suspect that later releases, or some subclasses may later use such resource and thus the proper functioning of my class will need invocation of ‘close’. I want to have a durable API so, because it seems logical, require and state the caller MUST close the resource.
The Java 8 verbiage, in my uinderstanding, is simply a more elaborate and more precise expression of what the short sentence containing the word “must” was saying in spec 7.
That would be a reasonable way to interpret it, but it’s not correct. Take Stream for instance. Every Stream extends AutoCloseable, yet you do not find any examples of:
There are some streams, however, that need to be closed, and the producing Javadoc clearly says so, e.g. Files.list(Path). Its Javadoc says:
The returned stream encapsulates a DirectoryStream. If timely disposal of file system resources is required, the try-with-resources construct should be used to ensure that the stream’s close method is invoked after the stream operations are completed.
I agree, this may be interpreted either way, but the fact is, no one calls Stream.close() on an “ordinary” in-memory collection backed stream.
Thanks for the explanation; good to know that these warnings can – in most cases – be ignored.
For people using IntelliJ IDEA: you can completely disable this warning (AutoCloseable used without ‘try’-with-resources), but I prefer to disable it for specific classes like SelectSelectStep, SelectJoinStep, etc. (see Settings | Editor | Inspections | Resource management issues).
It seems like I can safely tell IntelliJ to ignore the inspection “AutoCloseable used without ‘try’-with-resources” for subclasses of `org.jooq.Query`, with the following exceptions:
– instances of Query which have `keepStatement(true)` called on them
– instances of ResultQuery which have any of these calls:
– fetchLazy
– fetchResultSet
– fetchStream
– fetchStreamInto
– stream
Are there any others to watch out for (eg in code reviews)?
I could spend time trying to answer this question here on the blog, but few people would profit from this answer. What would be a much better way to answer this, including helping IDEs get this right, automatically? The solution can’t be for people to internalise such explicit knowledge, nor to document this in a blog post comment. There must be some automated solution to this.
An automated solution would be ideal of course, perhaps guided by annotations on the appropriate methods. I suppose this would have something in common with the motivation behind @java.lang.SafeVarargs.
However, I think it would also make sense for the javadocs of org.jooq.Query and org.jooq.ResultQuery to document, at the class level, the right way to interpret their AutoCloseable contract/interface. I found the above methods by skimming the method-level javadocs, but the wording varies in some cases, so I might well have missed something.
Thanks for your additional feedback. I guess the only method that really makes a query a resource is Query::keepStatement. Perhaps, that particular method should return a Query subtype that extends AutoCloseable, and we move the contract away from the Query itself.
Note that ResultQuery instances don’t become resourceful if you call fetchLazy(), fetchResultSet(), fetchStream(), etc. on it. Only the return value is a resource, not the ResultQuery.
I’ve created a change request for this: https://github.com/jOOQ/jOOQ/issues/13149. Just like with a previous change of that sort (https://github.com/jOOQ/jOOQ/issues/10512), I’m guessing that numerous existing jOOQ users will get a compilation error as they try to close a query that didn’t need closing, so while this would be a breaking change, it would be for the better.
When you have the above configuration and you try running the following program, you’ll get three warnings:
For more insight, see also this Stack Overflow answer by Stuart Marks, linking to the EG’s discussions on lambda-dev
How do we interpret it…
I create an API with AutoClosable classes and then I declare that the caller MUST close the resource (instance of my class) calling ‘close’ because I said that.
I may not actually hold a resource myself that I need to close. That is implementation detail. I may suspect that later releases, or some subclasses may later use such resource and thus the proper functioning of my class will need invocation of ‘close’. I want to have a durable API so, because it seems logical, require and state the caller MUST close the resource.
The Java 8 verbiage, in my uinderstanding, is simply a more elaborate and more precise expression of what the short sentence containing the word “must” was saying in spec 7.
That would be a reasonable way to interpret it, but it’s not correct. Take
Streamfor instance. EveryStreamextendsAutoCloseable, yet you do not find any examples of:try (Stream stream : Stream.of(1, 2, 3)) { stream.filter(...) .forEach(...); }That would be very annoying to write.
There are some streams, however, that need to be closed, and the producing Javadoc clearly says so, e.g.
Files.list(Path). Its Javadoc says:I agree, this may be interpreted either way, but the fact is, no one calls
Stream.close()on an “ordinary” in-memory collection backed stream.There is always the quick fix for the false positive to suppress the warming, if indeed you do know better.
Thanks for the explanation; good to know that these warnings can – in most cases – be ignored.
For people using IntelliJ IDEA: you can completely disable this warning (AutoCloseable used without ‘try’-with-resources), but I prefer to disable it for specific classes like SelectSelectStep, SelectJoinStep, etc. (see Settings | Editor | Inspections | Resource management issues).
It seems like I can safely tell IntelliJ to ignore the inspection “AutoCloseable used without ‘try’-with-resources” for subclasses of `org.jooq.Query`, with the following exceptions:
– instances of Query which have `keepStatement(true)` called on them
– instances of ResultQuery which have any of these calls:
– fetchLazy
– fetchResultSet
– fetchStream
– fetchStreamInto
– stream
Are there any others to watch out for (eg in code reviews)?
I could spend time trying to answer this question here on the blog, but few people would profit from this answer. What would be a much better way to answer this, including helping IDEs get this right, automatically? The solution can’t be for people to internalise such explicit knowledge, nor to document this in a blog post comment. There must be some automated solution to this.
An automated solution would be ideal of course, perhaps guided by annotations on the appropriate methods. I suppose this would have something in common with the motivation behind @java.lang.SafeVarargs.
However, I think it would also make sense for the javadocs of org.jooq.Query and org.jooq.ResultQuery to document, at the class level, the right way to interpret their AutoCloseable contract/interface. I found the above methods by skimming the method-level javadocs, but the wording varies in some cases, so I might well have missed something.
Thanks for your additional feedback. I guess the only method that really makes a query a resource is Query::keepStatement. Perhaps, that particular method should return a Query subtype that extends AutoCloseable, and we move the contract away from the Query itself.
Note that ResultQuery instances don’t become resourceful if you call fetchLazy(), fetchResultSet(), fetchStream(), etc. on it. Only the return value is a resource, not the ResultQuery.
I’ve created a change request for this: https://github.com/jOOQ/jOOQ/issues/13149. Just like with a previous change of that sort (https://github.com/jOOQ/jOOQ/issues/10512), I’m guessing that numerous existing jOOQ users will get a compilation error as they try to close a query that didn’t need closing, so while this would be a breaking change, it would be for the better.