Commons Dbcp
  1. Commons Dbcp
  2. DBCP-342

Not cleaned up commons pool's evictor task when SQLException occurs after the GenericObjectPool is initialized in createDataSource.

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.3, 1.4
    • Fix Version/s: 1.3.1, 1.4.1
    • Labels:
      None
    • Environment:
      • commons-pool : 1.4
      • commons-dbcp : 1.4, 1.3 both
      • dbms : MSSQL 2007

      Description

      JIRA DBCP-93 bts is not fixed correctly
      (JIRA DBCP-93 : If SQLException occurs after the GenericObjectPool is initialized in createDataSource, the evictor task is not cleaned up)

      Only one evictor timer thread leaves but evictor timer task queued as many as createDataSource() method called.
      queued evictor will keep tries to make connection to meet min idle connection.
      if suddenly dbms become stable and can accept connection again,
      then every queued evictor will try to make connection simultaneously.

      In my case there were more then 100000 user requests while I set wrong password to database,
      when I change my password to correct in database
      more then 100000 evictor tries to make connection
      and finally mssql makes connections as many as integer limit and dbms dies.

      here is my test.

      TestBasicDataSource.java
          public void testCreateDataSourceCleanupEvictor() throws Exception {
          	ds.close();
          	ds = null;
          	ds = createDataSource();
          	ds.setDriverClassName("org.apache.commons.dbcp.TesterConnRequestCountDriver");
          	ds.setUrl("jdbc:apache:commons:testerConnRequestCountDriver");
          	ds.setValidationQuery("SELECT DUMMY FROM DUAL");
          	ds.setUsername("username");
          	
          	// Make password incorrect, so createDataSource will throw
          	ds.setPassword("wrong");
          	// Set timeBetweenEvictionRuns > 0, so evictor will be created
          	ds.setTimeBetweenEvictionRunsMillis(100);
          	// Set min idle > 0, so evictor will try to make connection as many as idle count
          	ds.setMinIdle(2);
          	
          	Class.forName("org.apache.commons.dbcp.TesterConnRequestCountDriver");
          	TesterConnRequestCountDriver testerDriver = (TesterConnRequestCountDriver) DriverManager.getDriver("jdbc:apache:commons:testerConnRequestCountDriver");
          	testerDriver.initConnRequestCount();
          	
          	// user request 10 times
          	for (int i=0; i<10; i++) {
          		try {
          			@SuppressWarnings("unused")
      				DataSource ds2 = ds.createDataSource();
          		} catch (SQLException e) {
          			// Ignore
          		}
          	}
      
          	// sleep 1000ms. evictory will invoked 10 times.
          	Thread.sleep(1000);
          	
          	// if orphan evictor is alive, connection request count will be 100 which is denoted from (ds.createDataSource() count * evictor invoke count)
          	// if DBMS connection is back to stable in 10 minutes then this test case will make 120000 connections simultaneously
          	// 		120000 connections = ds.createDataSource() count (10) * evictor invoke count (6000) * min idle (2)
          	// but if evictor is cleaned up, connection request count will be only 10 (ds.createDataSource() count )
          	assertEquals(10, testerDriver.getConnectionRequestCount());
          	
          	// fail to create datasource then clean up orphan connection pool and evictor in connnection pool
          	assertNull(ds.connectionPool);
          }
      }
      

        Issue Links

          Activity

          Hide
          byungchol.kim added a comment -

          fix patch

          Show
          byungchol.kim added a comment - fix patch
          Hide
          Phil Steitz added a comment -

          Modified version of the patch applied in r1096550 (trunk) and r1096550 (1.4.x branch). I made the following changes to the patch:
          1. Instead of letting the Evictor start and shutting it down on exception, I moved the timeBetweenEvictionRuns setter into a new startPoolMaintenance method that is not invoked until initialization has succeeded. This prevents connection requests getting fired while initialization is in progress.
          2. Separated exception management for factory and instance creation.
          3. Made closeConnectionPool swallow rather than propagate exceptions so that the main exception leading to its invocation is not lost.
          4. Changed the test class to not depend on DriverManager driver loading, which no longer happens by default.

          Thanks for the patch!

          Show
          Phil Steitz added a comment - Modified version of the patch applied in r1096550 (trunk) and r1096550 (1.4.x branch). I made the following changes to the patch: 1. Instead of letting the Evictor start and shutting it down on exception, I moved the timeBetweenEvictionRuns setter into a new startPoolMaintenance method that is not invoked until initialization has succeeded. This prevents connection requests getting fired while initialization is in progress. 2. Separated exception management for factory and instance creation. 3. Made closeConnectionPool swallow rather than propagate exceptions so that the main exception leading to its invocation is not lost. 4. Changed the test class to not depend on DriverManager driver loading, which no longer happens by default. Thanks for the patch!
          Hide
          Frederic Tardif added a comment -

          We just hit that same error with version 1.4.
          The consequence of leaking pools that spawn connections (min pooled connection per pool) can be pretty nasty.

          Great to know there is a fix.

          Any ETA about when dbcp will release a new version with this fix officially included?

          Show
          Frederic Tardif added a comment - We just hit that same error with version 1.4. The consequence of leaking pools that spawn connections (min pooled connection per pool) can be pretty nasty. Great to know there is a fix. Any ETA about when dbcp will release a new version with this fix officially included?
          Hide
          Phil Steitz added a comment -

          Should be resolved, but not closed

          Show
          Phil Steitz added a comment - Should be resolved, but not closed

            People

            • Assignee:
              Unassigned
              Reporter:
              byungchol.kim
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development