You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Jean-Louis MONTEIRO (JIRA)" <ji...@apache.org> on 2008/06/12 11:23:45 UTC
[jira] Created: (DBCP-269) final jdbc connection never closed -->
number of connections grows
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.
[jira] Updated: (DBCP-269) final jdbc connection never closed -->
number of connections grows
Posted by "Jean-Louis MONTEIRO (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DBCP-269?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jean-Louis MONTEIRO updated DBCP-269:
-------------------------------------
Attachment: patch.txt
> 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
> Attachments: patch.txt
>
>
> 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.
[jira] Closed: (DBCP-269) final jdbc connection never closed -->
number of connections grows
Posted by "Dain Sundstrom (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DBCP-269?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Dain Sundstrom closed DBCP-269.
-------------------------------
Resolution: Fixed
Fix Version/s: 1.3
I didn't use the patch as is. Instead of changing the behavior of the proxy, I changed the close logic to inspect the underlying connection to determine if the proxy should be returned to the pool or destroyed.
> 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
> Assignee: Dain Sundstrom
> Fix For: 1.3
>
> Attachments: patch.txt
>
>
> 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.