DO NOT REPLY [Bug 33945] New: - DelegatingConnection.close() throws exception

[bu...@apache.org]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=33945>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=33945

           Summary: DelegatingConnection.close() throws exception
           Product: Commons
           Version: Nightly Builds
          Platform: Macintosh
        OS/Version: Mac OS X 10.3
            Status: NEW
          Severity: critical
          Priority: P1
         Component: Dbcp
        AssignedTo: commons-dev@jakarta.apache.org
        ReportedBy: hlapp@gnf.org


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 
understand.

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();
            }
        }
        finally {
            _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. 
BUMMER.

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.

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

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