final jdbc connection never closed --> number of connections grows
------------------------------------------------------------------
Key: DBCP-269
URL: https://issues.apache.org/jira/browse/DBCP-269
Project: Commons Dbcp
Issue Type: Bug
Affects Versions: 1.3, 1.4, Nightly Builds
Environment: Windows, JDK 5
Reporter: Jean-Louis MONTEIRO
Using DBCP parameters:
JdbcDriver com.mysql.jdbc.Driver
JdbcUrl jdbc:mysql://localhost:3307/dbcp?cacheResultsetMetadata=true
UserName dbcp
Password dbcp
InitialSize 5
MaxActive 8
MaxWait -1
TestOnBorrow false
TestOnReturn false
PoolPreparedStatements true
MaxOpenPreparedStatements 10
# TimeBetweenEvictionRunsMillis 20000
# MinEvictableIdleTimeMillis 30000
# NumTestsPerEvictionRun 3
# TestWhileIdle false
MinIdle 2
MaxIdle 5
DefaultTransactionIsolation 1
provides strange behaviour.
The testcase is simple:
@Test
public void testMaxIdle() throws Exception {
int MAX_ACTIVE = 8;
List<Connection> connectionList = new
ArrayList<Connection>();
int i = 0;
DataSource dataSource = ...; // retrieve the
datasource
Assert.assertNotNull(dataSource);
System.out.println("*** Getting all connections
...");
for (i = 0; i < MAX_ACTIVE; i++) {
System.out.println("get
connection " + (i + 1));
Connection con =
dataSource.getConnection();
Assert.assertNotNull(con);
connectionList.add(con);
}
i = 0;
System.out.println("*** Releasing all
connections ...");
for (Iterator<Connection> iterator =
connectionList.iterator(); iterator.hasNext();) {
Connection connection =
(Connection) iterator.next();
System.out.println("close
connection " + (++i));
connection.close();
iterator.remove();
}
i = 0;
System.out.println("*** Getting all + X
connections ...");
for (i = 0; i < (MAX_ACTIVE + 1); i++) {
System.out.println("get
connection " + (i + 1));
Connection con =
dataSource.getConnection();
Assert.assertNotNull(con);
connectionList.add(con);
}
}
First of all, after initialization, the number of real database connections
established is 6 instead of 5. This due to the "protected synchronized
DataSource createDataSource" method of the BasicDataSource object because of
the validation of the ConnectionFactory. The "validateConnectionFactory" method
creates a new jdbc connection and destroy it at the end. Nonetheless, the
underlying database connection is not really closed.
At the end of the above test case, we should not have more than 8 connections
but we have in fact 12 of them (8 + 1 for validation + 3 because DBCP wants to
close extra connection MAX_ACTIVE - MAX_IDLE). In fact, those 3 connections are
not really closed.
Please find attached a patch correcting this bug. This is due to the method
isClosed() of the DelegatingConnection. From my point of view, the
implementation of the method should be:
public boolean isClosed() throws SQLException {
if(_closed && _conn.isClosed()) { // Instead of if(_closed ||
_conn.isClosed())
return true;
}
return false;
}
>From my understanding, a connection is considered as closed if and only if one
>of the connection chain is closed (OR in the test). But, if I'm write the
>_closed flag of the delegating connection is set to true during the
>passivation. So, after returning to the pool, a connection can not be closed.
>The isClosed method is really important because it's used indirectly by the
>reallyClose method of a PoolableConnection.
The behaviour is much more impressive if the evict thread of the configuration
is activated because, with a testcase that simply wait after getting the
datasource initialized (Thread.currentThread().sleep(600000);) you will see
database connections growing.
Finally, after applying the given patch, I faced some problems regarding DBCP
TestCases:
In some test cases, you have a test method
public void testClose() throws Exception {
ds.setAccessToUnderlyingConnectionAllowed(true);
[...]
// raw idle connection should now be closed
// FIXME should be assertTrue
assertFalse(rawIdleConnection.isClosed());
[...]
// both wrapper and raw active connection should be closed
assertTrue(activeConnection.isClosed());
assertTrue(rawActiveConnection.isClosed());
}
>From my point of view and regarding the comment the
>assertFalse(rawIdleConnection.isClosed()); sould become
>assertTrue(rawIdleConnection.isClosed());.
Moreover, the next test method fails.
// Bugzilla Bug 28251: Returning dead database connections to
BasicDataSource
// isClosed() failure blocks returning a connection to the pool
public void testIsClosedFailure() throws SQLException {
ds.setAccessToUnderlyingConnectionAllowed(true);
Connection conn = ds.getConnection();
assertNotNull(conn);
assertEquals(1, ds.getNumActive());
// set an IO failure causing the isClosed mathod to fail
TesterConnection tconn = (TesterConnection)
((DelegatingConnection)conn).getInnermostDelegate();
tconn.setFailure(new IOException("network error"));
try {
conn.close();
fail("Expected SQLException");
}
catch(SQLException ex) { }
assertEquals(0, ds.getNumActive());
}
But again, looking at the AddObjectToPool of the GenericObjectPool
(commons-pool), the IOException("Network error") is catch during the
passivation. It implies that the underlying connection (in error) is not
returned to the pool. It will be re-created by the evict thread if minIdle
property is set.
Am I wrong ?
Sorry for the little long post.
Kind regards,
Jean-Louis MONTEIRO
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
