Avoid Method Chaining When Using Multiple AutoCloseable Instances

The try-with-resources statement introduced in Java 7 allows us to adopt an approach where certain objects could be opened, being visible in the try block, and immediately closed when the execution reaches the end of that scope. This was a huge improvement for managing scarce resources, in comparison with the old approach of overriding the finalize() method (which we never exactly know when  will be executed.)

Over the years, I was repeatedly seeing the following approach on some applications, which motivated me to write this post. Consider this code snippet for executing an SQL statement using JDBC:

xxxxxxxxxx

import javax.sql.DataSource;

import java.sql.*;

...

  public BigDecimal getSum(long empId) throws SQLException {

    String sql = "select sum(salary) from payroll where empId=?";

    try(PreparedStatement stmt = myDataSource.getConnection().prepareStatement(sql)) {

      stmt.setLong(1, empId);

      try(ResultSet rs = stmt.executeQuery()) {

        if (rs.next())

          return rs.getBigDecimal(1);

        return BigDecimal.ZERO;

      }

    }

  }

...

What the code does is unimportant, but the connection handling is very important. The outer try-with-resources ensures that at the end of its block, the PreparedStatement will be closed. The same thing goes with the ResultSet in the inner try-with-resources.

Here is the catch: the Connection object returned from the DataSource will never be closed. Since it was not bound to any variable inside the try-with-resources, according to the JLS (see References), the resource in the outer try-with-resources is of type PreparedStatement. As a result, the Connection instance is not participating in the statement, and so its close() method is never called.

The solution is easy: declare a variable referencing the resource you want to be auto-closed.

The correct code snippet becomes the following:

xxxxxxxxxx

import javax.sql.DataSource;

import java.sql.*;

...

  public BigDecimal getSum(long empId) throws SQLException {

    String sql = "select sum(salary) from payroll where empId=?";

    try(Connection connection = myDataSource.getConnection();

        PreparedStatement stmt = connection.prepareStatement(sql)) {

      stmt.setLong(1, empId);

      try(ResultSet rs = stmt.executeQuery()) {

        if (rs.next())

          return rs.getBigDecimal(1);

        return BigDecimal.ZERO;

      }

    }

  }

...

Java Has Different API Designs

Unfortunately, the JDBC API is different in design than the I/O streams API (e.g. InputStream, OutputStream, Reader, Writer, etc.)

On the one hand, the I/O streams API is designed for composability, so it is customary to see code that instances several streams in a delegation chain (following a Decorator design pattern), where the read, write, and close operations are in sequence calling the same method on the chain. As a result, a close() method call on the outermost referenced variable will, in turn, call the close() methods on the delegated objects: here a unique try-with-resources statement works as expected and will close all the resources.

The JDBC API, on the other hand, is not designed this way. Connections are created from DataSources, and Statements are created from Connections, and ResultSets are created from Statements, but there is no instance composition here, but pure method chaining: all those created objects have a different type, and they must be explicitly declared in the try-with-resources statement.

My Rule of Thumb

When in doubt declare all variables participating in the try-with-resources statement.

It is true that we need to watch for different close() semantics, as defined by the AutoCloseable and Closeable interfaces. But, provided they were designed in a sound fashion, when possible, the close() method will be idempotent, meaning that calling it multiple times (coming from multiple try-with-resources variables, and maybe executing in a different order) won't change the resource closing semantics.

Considerations on API Design

Taking the JDBC API as an example, care must be taken when designing new APIs, thinking about how they will interact with the current language constructs. This is especially true with modern fluent APIs, where the processing is done in long method call chains.

JDBC was designed many years before the introduction of the try-with-resources statement in Java 7, but nevertheless, it could be improved to play more nicely with it. Maybe enabling the creation of "smart jdbc resources" (in the same sense of a C++ smart pointer) where, whenever thr reach out of scope they are auto-closed.

My own rule of thumb is: avoid creating APIs that return or create AutoCloseable resources from other AutoCloseable resources. Enforce getting the resource by calling a different object (like a Factory).

For example, under the assumption we were designing from zero the JDBC API, instead of allowing us to get one auto-closeable resource from another, like it is now:

xxxxxxxxxx

myDataSource.getConnection().prepareStatement("select ...");

Instead, enforce the acquisition of the resource from another non AutoCloseable object, e.g. a JDBCFactory:

xxxxxxxxxx

JDBCFactory factory = new JDBCFactory(myDataSource);

Connection conn = factory.getConnection();

PreparedStatement = factory.prepareStatement(connection, "select ...");

Having a unique source from where to get resources avoids the method chaining problem inside a try-with-resources statement.

Final Thoughts

It is difficult to predict how an API will interact in the presence of different language constructs. This is the case with the JDBC API, and maybe others.

Returning AutoCloseable objects from other AutoCloseable objects suffers from this specific problem with the try-with-resources statement.

I think it would be somewhat easy for some experienced FindBugs or PMD developer to create a rule that could warn us about this situation.