Allow connection, statement, and result set to be closed multiple times

• Key: DBCP-233
• Type: Improvement
• Status: Resolved
• Resolution: Fixed
• Priority: Major
• Assignee: Unassigned
• Reporter: Dain Sundstrom
• Fix Version/s: 1.3

Description

This patch allows Connection, Statement, PreparedStatement, CallableStatement and ResultSet to be closed multiple times. The first time close is called the resource is closed and any subsequent calls have no effect. This behavior is required as per the JavaDocs for these classes. The patch adds tests for closing all types multiple times and updates any tests that incorrectly assert that a resource can be closed more then once.

This patch fixes DBCP-134 and DBCP-3

Comments

[Phil Steitz]

Patch applied. Many thanks.

[Frank Hefter]

I patched a 1.2.2 version with the above but had ongoing problems with "already closed exceptions".
In addition I would suggest to remove in PoolableConnection line84: the "throw new ..." clause (within the isClosed area).
After this change my application is stable now.

I'm a little concerned about the comment "// XXX should be guarded to happen at most once"
But this is done by the try catch surrounding the invalidateObject(this) and the exception thrown didn't help anyway.

Sorry I had no chance to make a patch file or test this.

[Phil Steitz]

Reopening while we investigate this.

[Phil Steitz]

How does you code get connections and where are the exceptions occurring? The changes and tests committed should ensure that connection handles obtained from DBCP datasources can be closed multiple times without exceptions being generated.

[Bob Bueckers]

I'm currently using dbcp 1.2.1 but have no problem updating to a newer release. What do I need to do to get the patch for this issue if I update to the 1.2.2 release? This is a very hot, and time sensitive issue for our project!

Thanks in advance!

Bob

[Kavin Du]

One other repro case is if the underlying database connection is closed (listener bounced for example) from the database end, calling Connection.close() will result in:

java.sql.SQLException: Attempted to use Connection after closed() was called.
at org.apache.commons.dbcp.cpdsadapter.ConnectionImpl.assertOpen(ConnectionImpl.java:87)
at org.apache.commons.dbcp.cpdsadapter.ConnectionImpl.close(ConnectionImpl.java:116)

The connection will stay in the the active pool, using up MaxActive. We need to have it drop off from the pool if it's closed. This is important as we want our application to automatically recover from a database failover and still have a accurate connection count.

The standard pattern of
Connection conn = null;
try { conn = ... } finally {
if (conn!=null) {
try { conn.close(); } catch (SQLException e) { // Ignore or log }
}
}

should address all of the concerns outlined above. Note that you'll need to build DBCP from svn to get the fix that allows multiple calls to close()

As Phil notes, the test cases cover this quite extensively. If you have a scenario/test case that fails with the latest code from svn please re-open and describe your scenario so we can investigate.