Commons Dbcp
  1. Commons Dbcp
  2. DBCP-233

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

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.3
    • Labels:
      None

      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

      1. CloseTwice.patch
        13 kB
        Dain Sundstrom

        Activity

        Hide
        Mark Thomas added a comment -

        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.

        Show
        Mark Thomas added a comment - 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.
        Hide
        Kavin Du added a comment - - edited

        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.

        Show
        Kavin Du added a comment - - edited 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.
        Hide
        Bob Bueckers added a comment -

        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

        Show
        Bob Bueckers added a comment - 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
        Hide
        Phil Steitz added a comment -

        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.

        Show
        Phil Steitz added a comment - 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.
        Hide
        Phil Steitz added a comment -

        Reopening while we investigate this.

        Show
        Phil Steitz added a comment - Reopening while we investigate this.
        Hide
        Frank Hefter added a comment -

        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.

        Show
        Frank Hefter added a comment - 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.
        Hide
        Phil Steitz added a comment -

        Patch applied. Many thanks.

        Show
        Phil Steitz added a comment - Patch applied. Many thanks.

          People

          • Assignee:
            Unassigned
            Reporter:
            Dain Sundstrom
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development