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