Uploaded image for project: 'Commons DBCP'
  1. Commons DBCP
  2. DBCP-134

[dbcp] DelegatingConnection.close() throws exception



    • Bug
    • Status: Closed
    • Critical
    • Resolution: Fixed
    • Nightly Builds
    • 1.3
    • None
    • Operating System: Mac OS X 10.3
      Platform: Macintosh

    • 33945


      Closing connections that were obtained from PoolingDataSource und wrapped with a
      DelegatingConnection throws a 'java.sql.SQLException: Already closed' when calling close() on them in
      order to return the connection to the underlying pool.

      The reason is code in DelegatingConnection.passivate() the motivation for which I don't completely

      At any rate, here is what happens. DelegatingConnection.close() calls passivate() before actually closing
      the delegate:


      • Closes the underlying connection, and close
      • any Statements that were not explicitly closed.
        public void close() throws SQLException { passivate(); _conn.close(); }

      DelegatingConnection.passivate() in turn cleans up statements and, if the delegate is a
      DelegatingConnection too, calls passivate() on the delegate. Finally, the instance variable _closed is set
      to true:

      protected void passivate() throws SQLException {
      try {
      // ... some statement clean up work, then:
      if(_conn instanceof DelegatingConnection)

      { ((DelegatingConnection)_conn).passivate(); }


      { _closed = true; }


      When this finishes and the delegate is indeed itself a delegating connection, close() will call
      _conn.close(). If DelegatingConnection were final this would even work, but it is not (on purpose). A
      notable derived class is PoolableConnection, which overrides close() and throws an exception if it is
      called when isClosed() returns true. isClosed() returns true if the _closed instance variable is true.

      The problem surfaces as soon as one tries to wrap the connection returned by PoolingDataSource with
      another DelegatingConnections, which happens to be what I do.

      I noticed this when I upgraded from 1.1 to 1.2.1, and it's still there in the nightly snapshot.

      There are several design decisions that I think deserve a critical look:

      • why does passivate() set a variable that effectively decides whether a connection is considered
        closed or not? shouldn't only close() be doing that?
      • why does DelegatingConnection even bother to clean up statements when those statements by
        definition must have come from the delegate (or its delegate and so forth) and so are the responsibility
        of the delegate (creator) to clean up
      • by propagating passivate() in the same method to the delegate if the delegate delegates itself
        DelegatingConnection is making assumptions that may be (and in fact are) easily violated if someone
        sub-classes DelegatingConnection and the delegate is now a subclass with possibly altered behavior.
        Why does it not suffice to expect that calling close() on the delegate will give the delegate enough
        chance to clean up itself, regardless of the implementing class of the delegate.

      I'd be thrilled if this can be fixed quickly, and fixing any of the problems pinpointed above will fix the
      issue. Or so I think.




            Unassigned Unassigned
            hlapp@gnf.org Hilmar Lapp
            0 Vote for this issue
            0 Start watching this issue