commons-user mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Nathan Smith" <nat...@labpro2000.com>
Subject [DBCP] HIGH PRIORITY. Code changes to make sure connections are closed.
Date Thu, 27 Oct 2005 21:35:43 GMT
To the DBCP development team,

A while back we had a problem with DBCP not closing connections fully. 
It went throught the motions of closing the connection, adjusting the 
active, and inuse counters correctly but the connection never actually 
was closed. It was however no longer referenced by DBCP.

I'll try to explain why this problem occured in the first place. A few 
years ago we read some documentation that said it was always a good idea 
to explicitly close any ResultSet's or Statements that you use to 
release resources as soon as you have finished with then, even though 
the javadoc states the ResultSet is closed when the Statement is closed. 
We have done this throughout the application. Our JDBC driver likes to 
throw SQLExceptions if a ResultSet or Statement have already been closed 
when the Connection is closed, as it is supposed to clean up after 
itself as per the javadoc.

This causes problems in the DelegatingConnection class when the 
connection is really closed. It calls passivate(), which closes the 
Statement's and ResultSet's that are associated with the Connection. If 
the Statement is already closed it could throw an NPE or most likely an 
SQLException because the ResultSet is already closed. This misses out 
the call _conn.close(). The single most crucial call that tells the 
database to close this connection. After a while the database can no 
longer provide connections as it has reached a specified limit but the 
connection pool numbers look as though everything is fine.

I have included the changes I have made to the code. I don't know how to 
create a patch to merge into your development tree, sorry.

Below is the modified code:

DelegatingConnection.java

    public void close() throws SQLException
    {
       * log.debug("Closes the underlying connection, and closes "
                  + "any Statements that were not explicitly closed.");
        try {
            passivate();
        } catch(SQLException sqle) {
            log.warn("Problem passivating connection. " + 
sqle.getMessage(), sqle);
            throw sqle;
        }
        finally {
            log.debug("Closing the underlying connection ("+ _conn + ')');
            if(_conn != null) {
                _conn.close();
            }
        }*
    }

    protected void passivate() throws SQLException {
        log.debug("[DelegatingConnection.passivate] Connection (" + 
_conn + ')');
        try {
            // The JDBC spec requires that a Connection close any open
            // Statement's when it is closed.
            List statements = getTrace();
            log.debug("statements: " + statements);
            if( statements != null) {
                Statement[] set = new Statement[statements.size()];
                statements.toArray(set);
                for (int i = 0; i < set.length; i++) {
                   * if(set[i] != null) {
                        set[i].close();
                    }*
                }
                clearTrace();
            }
            setLastUsed(0);
            if(_conn instanceof DelegatingConnection) {
                ((DelegatingConnection)_conn).passivate();
            }
        }
        finally {
            _closed = true;
        }
    }

DelegatingStatement.java

    public void close() throws SQLException {
        try {
            try {
                if (_conn != null) {
                    _conn.removeTrace(this);
                    _conn = null;
                }
       
                // The JDBC spec requires that a statment close any open
                // ResultSet's when it is closed.
                // FIXME The PreparedStatement we're wrapping should 
handle this for us.
                // See bug 17301 for what could happen when ResultSets 
are closed twice.
                List resultSets = getTrace();
                if( resultSets != null) {
                    ResultSet[] set = (ResultSet[]) 
resultSets.toArray(new ResultSet[resultSets.size()]);
                    for (int i = 0; i < set.length; i++) {
                       * if(set[i] != null) {
                            set[i].close();
                        }*
                    }
                    clearTrace();
                }

               * if(_stmt != null) {
                    _stmt.close();
                }*
            }
            catch (SQLException e) {
                handleException(e);
            }
        }
        finally {
            _closed = true;
        }
    }

DelegatingResultSet.java

    public void close() throws SQLException {
        try {
            if(_stmt != null) {
                ((AbandonedTrace)_stmt).removeTrace(this);
                _stmt = null;
            }
            *if(_res != null) {
                _res.close();
            }*
        }
        catch (SQLException e) {
            handleException(e);
        }

PoolableConnectionFactory.java

    public void validateConnection(Connection conn) throws SQLException {
        String query = _validationQuery;
        if(conn.isClosed()) {
            throw new SQLException("validateConnection: connection closed");
        }
        if(null != query) {
            Statement stmt = null;
            ResultSet rset = null;
            try {
                stmt = conn.createStatement();
                rset = stmt.executeQuery(query);
                if(!rset.next()) {
                    throw new SQLException("validationQuery didn't 
return a row");
                }
            } finally {
                try {
                    *if(rset != null) {
                        rset.close();
                    }*
                } catch(Exception t) {
                    // ignored
                }
                try {
                    *if(stmt != null) {
                        stmt.close();
                    }*
                } catch(Exception t) {
                    // ignored
                }
            }
        }
    }

PoolingConnection.java

    public synchronized void close() throws SQLException {
        log.debug("[PoolingConnection.close] ");
        *try {*
            if(null != _pstmtPool) {
                KeyedObjectPool oldpool = _pstmtPool;
                _pstmtPool = null;
                try {
                    oldpool.close();
                } catch(RuntimeException e) {
                    throw e;
                } catch(SQLException e) {
                    throw e;
                } catch(Exception e) {
                    throw new SQLNestedException("Cannot close 
connection", e);
                }
            }
       * } finally {
            Connection innermostDelegate = getInnermostDelegate();
            log.debug("Closing my underlying connection (" + 
innermostDelegate + ')');
            if(innermostDelegate != null) {
                innermostDelegate.close();
            }
        }*
    }

Cheers.
-- 



---------------------------------------------------------------------
To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-user-help@jakarta.apache.org


Mime
View raw message