Re: [DBCP] Back pointers
Sounds good to me. I attached my fix to DBCP-11. This patch assures that all all returned Statements, PreparedStatements, CallableStatements and ResultSets are wrapped with a delegating object, which already properly handle the back pointers for Connection and Statement. This patch includes an extensive test to assure that the *same* object used to create the statement or result set is returned from either the getConnection() or getStatementMethod(). Note we must make sure to update this test for each release of the JDBC spec we support as new ways of obtaining these objects tend to be added over time. As per your suggestion below, this patch does not fix back pointers in connections obtained from a InstanceKeyDataSource using the cpdsadapter. The cpdsadapter does not use the delegating wrapper internally, and will need a different solution. DBCP-217 should also be closed. -dain On Jul 17, 2007, at 9:39 PM, Phil Steitz wrote: On 7/17/07, Dain Sundstrom [EMAIL PROTECTED] wrote: I'm working on a fix for the back pointers bugs DBCP-11 and DBCP-217 where the Statement.getConnection() and ResultSet.getStatement() return the wrong objects. The fix is pretty simple; we just need to make sure we wrap Statements and ResultSets returned from DelegatingConnection with the matching delegating type. +1 Anyway, I have the fix mostly complete with a bunch of test cases, but there is one problem... The PerUserPoolDataSource and SharedPoolDataSource classes return the ConnectionImpl class directly. This class is a wrapper around the real connection so we need to wrap returned Statements which is easy enough. The problem is these datasources use the CPDSConnectionFactory which does not call passivate on the delegating connection when the connection is returned to the pool so the Statements owned by the DelegatingConnection aren't closed. To make matters worse, CPDSConnectionFactory can't call passivate anyway because it is in a different package and the method is protected :( At this point I'm not sure what to do. I could fix the problem for all DataSources except for these two, and in the future we could rework these two to subclass PoolingDataSource. Alternative, we could move CPDSConnectionFactory to same package as DelegatingConnection or make is a sublcass of some ConnectionFactory with access to the passivate method. I really do think these datasources should be brought in line with the main abstractions used by the other classes, but I don't think that is something for this release (maybe for 2.0?). I think we should leave this alone for now and consider refactoring for 2.0, but there is a semantic difference that we need to keep in mind. InstanceKeyDataSource (parent of PerUser and SharedPool) sources connections from a ConnectionPoolDataSource. These datasources return connection *handles* (PooledConnection impls), which are not the same as DelegatingConnections. The cpdsadapter package is just there for older jdbc drivers that do not provide ConnectionPoolDataSource implementations. See the javadoc for InstanceKeyDataSource and also the implementation of makeObject there. The key difference in the contract is that InstanceKeyDataSource implements ConnectionEventListener, so when used with a driver that correctly supports ConnectionPoolDataSource, the connection handles handed out to users notify the pool (actually the factory in dbcp) when they are closed by the user. See connectionClosed in CPDSConnectionFactory. Phil - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
[DBCP] Back pointers
I'm working on a fix for the back pointers bugs DBCP-11 and DBCP-217 where the Statement.getConnection() and ResultSet.getStatement() return the wrong objects. The fix is pretty simple; we just need to make sure we wrap Statements and ResultSets returned from DelegatingConnection with the matching delegating type. Anyway, I have the fix mostly complete with a bunch of test cases, but there is one problem... The PerUserPoolDataSource and SharedPoolDataSource classes return the ConnectionImpl class directly. This class is a wrapper around the real connection so we need to wrap returned Statements which is easy enough. The problem is these datasources use the CPDSConnectionFactory which does not call passivate on the delegating connection when the connection is returned to the pool so the Statements owned by the DelegatingConnection aren't closed. To make matters worse, CPDSConnectionFactory can't call passivate anyway because it is in a different package and the method is protected :( At this point I'm not sure what to do. I could fix the problem for all DataSources except for these two, and in the future we could rework these two to subclass PoolingDataSource. Alternative, we could move CPDSConnectionFactory to same package as DelegatingConnection or make is a sublcass of some ConnectionFactory with access to the passivate method. I really do think these datasources should be brought in line with the main abstractions used by the other classes, but I don't think that is something for this release (maybe for 2.0?). Suggestion? -dain - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [DBCP] Back pointers
BTW the reason we are getting ConnectionImpls instead of raw connections from the PooledDataSource is because PooledConnectionImpl.getConnection():159 wraps the raw connection with a ConnectionImpl. The line of code is commented with the spec requires that this return a new Connection instance. If we didn't wrap, the back pointers would work, but we'd be violating the JDBC spec. Once we wrap, we need to wrap all the statements and result sets so the back pointers are valid. Another bug, is we're not tracking statements and result sets, so when the logical connection is closed the statements and result sets are not closed as required buy the spec. -dain On Jul 17, 2007, at 2:42 PM, Dain Sundstrom wrote: I'm working on a fix for the back pointers bugs DBCP-11 and DBCP-217 where the Statement.getConnection() and ResultSet.getStatement() return the wrong objects. The fix is pretty simple; we just need to make sure we wrap Statements and ResultSets returned from DelegatingConnection with the matching delegating type. Anyway, I have the fix mostly complete with a bunch of test cases, but there is one problem... The PerUserPoolDataSource and SharedPoolDataSource classes return the ConnectionImpl class directly. This class is a wrapper around the real connection so we need to wrap returned Statements which is easy enough. The problem is these datasources use the CPDSConnectionFactory which does not call passivate on the delegating connection when the connection is returned to the pool so the Statements owned by the DelegatingConnection aren't closed. To make matters worse, CPDSConnectionFactory can't call passivate anyway because it is in a different package and the method is protected :( At this point I'm not sure what to do. I could fix the problem for all DataSources except for these two, and in the future we could rework these two to subclass PoolingDataSource. Alternative, we could move CPDSConnectionFactory to same package as DelegatingConnection or make is a sublcass of some ConnectionFactory with access to the passivate method. I really do think these datasources should be brought in line with the main abstractions used by the other classes, but I don't think that is something for this release (maybe for 2.0?). Suggestion? -dain - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [DBCP] Back pointers
On 7/17/07, Dain Sundstrom [EMAIL PROTECTED] wrote: I'm working on a fix for the back pointers bugs DBCP-11 and DBCP-217 where the Statement.getConnection() and ResultSet.getStatement() return the wrong objects. The fix is pretty simple; we just need to make sure we wrap Statements and ResultSets returned from DelegatingConnection with the matching delegating type. +1 Anyway, I have the fix mostly complete with a bunch of test cases, but there is one problem... The PerUserPoolDataSource and SharedPoolDataSource classes return the ConnectionImpl class directly. This class is a wrapper around the real connection so we need to wrap returned Statements which is easy enough. The problem is these datasources use the CPDSConnectionFactory which does not call passivate on the delegating connection when the connection is returned to the pool so the Statements owned by the DelegatingConnection aren't closed. To make matters worse, CPDSConnectionFactory can't call passivate anyway because it is in a different package and the method is protected :( At this point I'm not sure what to do. I could fix the problem for all DataSources except for these two, and in the future we could rework these two to subclass PoolingDataSource. Alternative, we could move CPDSConnectionFactory to same package as DelegatingConnection or make is a sublcass of some ConnectionFactory with access to the passivate method. I really do think these datasources should be brought in line with the main abstractions used by the other classes, but I don't think that is something for this release (maybe for 2.0?). I think we should leave this alone for now and consider refactoring for 2.0, but there is a semantic difference that we need to keep in mind. InstanceKeyDataSource (parent of PerUser and SharedPool) sources connections from a ConnectionPoolDataSource. These datasources return connection *handles* (PooledConnection impls), which are not the same as DelegatingConnections. The cpdsadapter package is just there for older jdbc drivers that do not provide ConnectionPoolDataSource implementations. See the javadoc for InstanceKeyDataSource and also the implementation of makeObject there. The key difference in the contract is that InstanceKeyDataSource implements ConnectionEventListener, so when used with a driver that correctly supports ConnectionPoolDataSource, the connection handles handed out to users notify the pool (actually the factory in dbcp) when they are closed by the user. See connectionClosed in CPDSConnectionFactory. Phil - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
