How to properly clean up JDBC resources in Java?

Question

What is considered best practices when cleaning up JDBC resources and why? I kept the example short, thus just the cleaning up of the ResultSet.

finally
{
  if(rs != null)
    try{ rs.close(); } catch(SQLException ignored) {}
}

versus

finally
{
  try{ rs.close(); } catch(Exception ignored) {}
}

Personally I favour the second option since it is a bit shorter. Any input on this is much appreciated.

Answer 1

Nowadays JDK 7 gives you the easiest option to clean up resources:

String query = "select COF_NAME, PRICE from COFFEES";
try (Statement stmt = con.createStatement()) {
    ResultSet rs = stmt.executeQuery(query);
    while (rs.next()) {
        String coffeeName = rs.getString("COF_NAME");
        float price = rs.getFloat("PRICE");
        System.out.println(coffeeName + ", "  + price);
    }
}

The try statement ensures that each resource is closed at the end of the statement. See http://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html

Answer 2

As others have pointed out, JDBC resources (statements, result sets, etc...) are rarely null. If they are, you have bigger issues on your hands than NullPointerExceptions. In that regard, the NullPointerExceptions will help alert you to severe problems with your JDBC driver. The typical checking for null before calling close() would silently hide the problem if your JDBC driver was, in fact, providing you with null references.

As well, not all JDBC drivers follow the specification precisely. For example, some drivers will not automatically close a ResultSet when it's associated Statement is closed. Therefore, you have to ensure that you explicitly close both the ResultSet and its Statement (sigh).

In practice, I have found this technique useful (although its not the prettiest):

PreparedStatement statement = connection.prepareStatement("...");
try {
    ResultSet results = statement.executeQuery();
    try {
        while (results.next()) {
            // ...
        }
    } finally {
        results.close();
    }
} finally {
    statement.close();
}

This technique guarantees that every close() statement is executed, starting with the ResultSet and working its way outward. NullPointerExceptions are still thrown should the driver provide you with null references, but I allow this for the reasons explained at the beginning. SQLExceptions are still thrown if any of the close() statements fail (I consider this a good thing - I want to know if something is going wrong).

Answer 3

I see no problem with your second (uncommon) version.

• usually, rs will not be null, so an NPE will occur in rare cases. So I see no performance problem here.
• both version behave exactly the same in case of rs = null

The only disadvantage - if we have more then one resource to close, then we'd have to add one try/catch for each resource, if we want to close as many resources as possible. Otherwise, we'd enter the catch clause with the first null and that could cause undiscored leaks.

So it would look like that:

finally {
   try{rs.close();  }catch(Exception ignored){}
   try{stmt.close();}catch(Exception ignored){}
   try{conn.close();}catch(Exception ignored){}
}

... which is still readable and understandable. But, according to never change a common pattern - I'd stick to the old-fashioned way of testing null first and catching SQLException while closing.