commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jean-Louis MONTEIRO (JIRA)" <j...@apache.org>
Subject [jira] Updated: (DBCP-269) final jdbc connection never closed --> number of connections grows
Date Thu, 12 Jun 2008 09:27:45 GMT

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


Mime
View raw message