Details
-
Bug
-
Status: Closed
-
Critical
-
Resolution: Fixed
-
Nightly Builds
-
None
-
Operating System: Mac OS X 10.3
Platform: Macintosh
-
33945
Description
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)
}
finally
}
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.