tombo_189
tombo_189

Reputation: 384

Closing JDBC-Connections only in finally-block?

Why are database connections often closed at two positions, once directly after use, and secondly additionally in a finally-block using a check for null in order to prevent them to be closed twice. Doesn't it suffice to use the finally block? The finally-block is to be executed in every case.

Here is an official example of Apache-Tomcat JNDI Datasource HOW-TO. They point out there that a connection MUST be closed under every circumstance. I wonder why it is not sufficient to use the finally-block as the close-commands within the end of the main try {}-block seem to be redundent.

    Connection conn = null;
    Statement stmt = null; // Or PreparedStatement if needed
    ResultSet rs = null;
    try
    {
        conn = ... get connection from connection pool ...
        stmt = conn.createStatement("select ...");
        rs = stmt.executeQuery();
        ... iterate through the result set ...
        rs.close ();
        rs = null;
        stmt.close ();
        stmt = null;
        conn.close (); // Return to connection pool
        conn = null; // Make sure we don't close it twice
    }
    catch (SQLException e)
    {
        ... deal with errors ...
    }
    finally
    {
        // Always make sure result sets and statements are closed,
        // and the connection is returned to the pool
        if (rs != null)
        {
            try
            {
                rs.close ();
            }
            catch (SQLException ignore)
            {
            }
            rs = null;
        }
        if (stmt != null)
        {
            try
            {
                stmt.close ();
            }
            catch (SQLException ignore)
            {
            }
            stmt = null;
        }
        if (conn != null)
        {
            try
            {
                conn.close ();
            }
            catch (SQLException ignore)
            {
            }
            conn = null;
        }
    }

I would like to write much shorter:

    Connection conn = null;
    Statement stmt = null; // Or PreparedStatement if needed
    ResultSet rs = null;
    try
    {
        conn = ... get connection from connection pool ...
        stmt = conn.createStatement ("select ...");
        rs = stmt.executeQuery();
        ... iterate through the result set ...
    }
    catch (SQLException e)
    {
        // ... deal with errors ...
    }
    finally
    {
        // Always make sure result sets and statements are closed,
        // and the connection is returned to the pool
        try
        {
            if (rs != null)
                rs.close ();
            if (stmt != null)
                stmt.close ();
            if (conn != null)
                conn.close ();
        }
        catch (SQLException ignore)
        {
        }
    }

Upvotes: 5

Views: 6274

Answers (3)

Vividh Kulshrestha
Vividh Kulshrestha

Reputation: 1

I will prefer writing a common method for closing the connection, which can be called from a finally block.

Something like this:

Connection conn = null;        
Statement stmt = null; // Or PreparedStatement if needed        
ResultSet rs = null;

try
{
    conn = ... get connection from connection pool ...            
    stmt = conn.createStatement ("select ...");         
    rs = stmt.executeQuery();

    ... iterate through the result set ...

}
catch (SQLException e)
{
    // ... deal with errors ...
}
finally
{
    CloseTheConnection(conn);        
    CloseTheStatement(stmt);         
}

public void closeTheConnection(Connection conn){             
try{
    if(conn!=null){            
     conn.close();             
  }             
}catch(Exception ex){            
}

public void closeTheStatement(Statement stmt){             
try{            
 if( stmt != null)            
      stmt.close();            
  } catch(Exception ex){                
  }
}

By creating different methods and calling them from finally will ensure that even if you get any exception from one method the other method will definitely be called. And it will be reusable as well.

Upvotes: 0

tombo_189
tombo_189

Reputation: 384

Thanks for the many comments. So summing up (especially EJP comments to my question [that closing a Connection will close underlying Statements and closing a Statement will it self close the ResultSet]) and as I consider the try-with-resource construct to be a bit difficult to read I suggest writing

Connection conn = null;
Statement stmt = null; // Or PreparedStatement if needed
ResultSet rs = null;
try
{
    conn = ... get connection from connection pool ...
    stmt = conn.createStatement ("select ...");
    rs = stmt.executeQuery();
    ... iterate through the result set ...
}
catch (SQLException e)
{
    // ... deal with errors ...
}
finally
{
    // Always make sure result sets and statements are closed,
    // and the connection is returned to the pool
    try
    {
        if (conn != null)
            conn.close ();
    }
    catch (SQLException ignore)
    {
    }
}

and closing only the main Connection while leaving the Statement and the ResultSet always untouched and to be closed by the Connection.

Right?

Upvotes: 0

eis
eis

Reputation: 53553

You have a good question - I don't understand the "official example" either. Finally block is certainly enough.

However, your code has more serious faults, namely if rs.close() would throw an exception, you'd have stmt and conn leaking out and you'd also ignore that exception silently. That is something you should not do. Since Java 7, using try-with-resources construct is the preferred way, but if you cannot go there, at least handle each possible exception (rs, stmt, conn) separately so they don't cause each other to leak.

For example Apache commons DbUtils has closeQuietly() just for this purpose, because it used to be a common scenario. Personally I would go to somewhere like Spring JDBCTemplate that does this sort of stuff behind the scenes.

Edit: try-with-resources is explained by Oracle here. In short, you'd do it something like this:

try (Connection conn = yourCodeToGetConnection();
    Statement stmt = con.createStatement();
    ResultSet rs = stmt.executeQuery(query)) {
    while (rs.next()) {
        String coffeeName = rs.getString("COF_NAME");
        int supplierID = rs.getInt("SUP_ID");
        float price = rs.getFloat("PRICE");
        int sales = rs.getInt("SALES");
        int total = rs.getInt("TOTAL");

        System.out.println(coffeeName + ", " + supplierID + ", " + 
                               price + ", " + sales + ", " + total);
    }
} catch (SQLException ex) {
    // log, report or raise
}

Where the try-statement automatically deals with conn, stmt and rs closing in all cases and in order (the reverse order in which you state them). Possible exceptions you still need to handle yourself.

Upvotes: 6

Related Questions