49

I've been doing code review (mostly using tools like FindBugs) of one of our pet projects and FindBugs marked following code as erroneous (pseudocode):

Connection conn = dataSource.getConnection();

try{
    PreparedStatement stmt = conn.prepareStatement();
    //initialize the statement
    stmt.execute();
    ResultSet rs =  stmt.getResultSet();
    //get data
}finally{
    conn.close();
}

The error was that this code might not release resources. I figured out that the ResultSet and Statement were not closed, so I closed them in finally:

finally{
    try{
        rs.close()
    }catch(SqlException se){
        //log it
    }
    try{
        stmt.close();
    }catch(SqlException se){
        //log it
    }
    conn.close();
}

But I encountered the above pattern in many projects (from quite a few companies), and no one was closing ResultSets or Statements.

Did you have troubles with ResultSets and Statements not being closed when the Connection is closed?

I found only this and it refers to Oracle having problems with closing ResultSets when closing Connections (we use Oracle db, hence my corrections). java.sql.api says nothing in Connection.close() javadoc.

Machavity
  • 30,841
  • 27
  • 92
  • 100
jb.
  • 23,300
  • 18
  • 98
  • 136
  • I highly recommend using Apache commons-dbutils (http://commons.apache.org/dbutils/) It's a lightweight JDBC library that really cleans up a lot of boilerplate JDBC code. – Ken Liu Feb 25 '10 at 00:24
  • 2
    This is the kind of errors one gets when they don't close the relevant objects - "ORA-01000: maximum open cursors exceeded" - http://stackoverflow.com/questions/12192592/java-sql-sqlexception-ora-01000-maximum-open-cursors-exceeded – david blaine Apr 25 '13 at 09:13
  • database cursor - http://stackoverflow.com/questions/3861558/what-are-the-benefits-of-using-database-cursor A cursor is a tool that allows you to iterate the records in a set. It has concepts of order and current record. – david blaine Apr 25 '13 at 09:20

8 Answers8

52

One problem with ONLY closing the connection and not the result set, is that if your connection management code is using connection pooling, the connection.close() would just put the connection back in the pool. Additionally, some database have a cursor resource on the server that will not be freed properly unless it is explicitly closed.

bluish
  • 26,356
  • 27
  • 122
  • 180
Aaron
  • 19,151
  • 4
  • 28
  • 23
  • 14
    The javadocs for Connection#close method says that "releases this Connection object's database and JDBC resources immediately". I think the problem is that some bad implementations don't do the right job. When pools don't close related resources, are they doing the wrong thing? – marcospereira Sep 19 '08 at 21:36
  • 2
    Most major vendor's JDBC drivers correspond to spec. However, most (if not all) application servers maintain a pool of connections. They wrap the native connection and reimplement methods like close() so that the connections can be 'pooled'. This means that if you work in these environments, you MUST close resources like Statements and ResultSets yourself. – Ryan Fernandes Jul 06 '09 at 08:49
  • 4
    @Ryan Fernandes: Well, some pool just give you a connectionProxy object that saves all opened statements on it. When it gets returned in the pool, it closes all opened statements. – Tim Büthe Jan 20 '11 at 09:55
  • this is NOT true. Pooled connections will clean before returning to the pool = closing all their objects except socket connection to RDBMS. – Enerccio Jul 01 '19 at 19:12
29

I've had problems with unclosed ResultSets in Oracle, even though the connection was closed. The error I got was

"ORA-01000: maximum open cursors exceeded"

So: Always close your ResultSet!

neu242
  • 15,796
  • 20
  • 79
  • 114
19

You should always close all JDBC resources explicitly. As Aaron and John already said, closing a connection will often only return it to a pool and not all JDBC drivers are implemented exact the same way.

Here is a utility method that can be used from a finally block:

public static void closeEverything(ResultSet rs, Statement stmt,
        Connection con) {
    if (rs != null) {
        try {
            rs.close();
        } catch (SQLException e) {
        }
    }
    if (stmt != null) {
        try {
            stmt.close();
        } catch (SQLException e) {
        }
    }
    if (con != null) {
        try {
            con.close();
        } catch (SQLException e) {
        }
    }
}
  • 6
    Man, maybe we need a Closable interface, huh? – marcospereira Sep 19 '08 at 21:44
  • 2
    ResultSet are closed automatically when you close the Statement. (See JavaDoc http://download.oracle.com/javase/1.4.2/docs/api/java/sql/Statement.html) – Tim Büthe Jan 20 '11 at 09:52
  • Thanks for posting. I like this idea. – Abboq Apr 04 '11 at 14:31
  • From this post's http://stackoverflow.com/questions/11160557/java-jdbc-best-design-pattern-to-close-database-connection-when-exception-occ#11160807 response from Brunoss (see Miserable Variable comment), you may want to reverse the order of closing rs and stmt (close stmt before rs). – ggkmath Jun 22 '12 at 17:54
  • Connection, Stamement and ResultSet all implement AutoCloseable since Java 7. – Natix Apr 11 '14 at 09:58
9

Oracle will give you errors about open cursors in this case.

According to: http://java.sun.com/javase/6/docs/api/java/sql/Statement.html

it looks like reusing a statement will close any open resultsets, and closing a statement will close any resultsets, but i don't see anything about closing a connection will close any of the resources it created.

All of those details are left to the JDBC driver provider.

Its always safest to close everything explicitly. We wrote a util class that wraps everything with try{ xxx } catch (Throwable {} so that you can just call Utils.close(rs) and Utils.close(stmt), etc without having to worry about exceptions that close scan supposedly throw.

John Gardner
  • 24,225
  • 5
  • 58
  • 76
  • _but i don't see anything about closing a connection will close any of the resources it created._ http://docs.oracle.com/javase/6/docs/api/java/sql/Connection.html#close() _releases this Connection object's database **and JDBC resources** immediately_ But yeah, always better to close everything. – user454322 Jan 22 '13 at 06:34
8

The ODBC Bridge can produce a memory leak with some ODBC drivers.

If you use a good JDBC driver then you should does not have any problems with closing the connection. But there are 2 problems:

  • Does you know if you have a good driver?
  • Will you use other JDBC drivers in the future?

That the best practice is to close it all.

Horcrux7
  • 23,758
  • 21
  • 98
  • 156
8

I work in a large J2EE web environment. We have several databases that may be connected to in a single request. We began getting logical deadlocks in some of our applications. The issue was that as follows:

  1. User would request page
  2. Server connects to DB 1
  3. Server Selects on DB 1
  4. Server "closes" connection to DB 1
  5. Server connects to DB 2
  6. Deadlocked!

This occurred for 2 reasons, we were experiencing far higher volume of traffic than normal and the J2EE Spec by default does not actually close your connection until the thread finishes execution. So, in the above example step 4 never actually closed the connection even though they were closed properly in finally .

To fix this, you you have to use resource references in the web.xml for your Database Connections and you have to set the res-sharing-scope to unsharable.

Example:

<resource-ref>
    <description>My Database</description>
    <res-ref-name>jdbc/jndi/pathtodatasource</res-ref-name>
    <res-type>javax.sql.DataSource</res-type>
    <res-auth>Container</res-auth>
    <res-sharing-scope>Unshareable</res-sharing-scope>
</resource-ref>
Konrad
  • 3,593
  • 3
  • 19
  • 17
4

I've definitely seen problems with unclosed ResultSets, and what can it hurt to close them all the time, right? The unreliability of needing to remembering to do this is one of the best reasons to move to frameworks that manage these details for you. It might not be feasible in your development environment, but I've had great luck using Spring to manage JPA transactions. The messy details of opening connections, statements, result sets, and writing over-complicated try/catch/finally blocks (with try/catch blocks in the finally block!) to close them again just disappears, leaving you to actually get some work done. I'd highly recommend migrating to that kind of a solution.

JavadocMD
  • 4,397
  • 2
  • 25
  • 23
  • 1
    Its one point in this app. We use EJB, and this part was sort of plugin - our client would deploy a datasource on serwer, and then is able to query it through this plugin. – jb. Sep 19 '08 at 18:05
4

In Java, Statements (not Resultsets) correlate to Cursors in Oracle. It is best to close the resources that you open as unexpected behavior can occur in regards to the JVM and system resources.

Additionally, some JDBC pooling frameworks pool Statements and Connections, so not closing them might not mark those objects as free in the pool, and cause performance issues in the framework.

In general, if there is a close() or destroy() method on an object, there's a reason to call it, and to ignore it is done so at your own peril.

Spencer Kormos
  • 8,381
  • 3
  • 28
  • 45