[ http://issues.apache.org/jira/browse/DBCP-44?page=all ]
Phil Steitz updated DBCP-44: ---------------------------- Bugzilla Id: (was: 33912) Fix Version: 1.2.2 > [dbcp] Evictor thread in GenericObjectPool has potential for deadlock > --------------------------------------------------------------------- > > Key: DBCP-44 > URL: http://issues.apache.org/jira/browse/DBCP-44 > Project: Commons Dbcp > Type: Bug > Environment: Operating System: All > Platform: All > Reporter: Eric Layton > Fix For: 1.2.2 > > The GenericObjectPool Evictor thread can potentially cause a deadlock between > its connection factory and java.sql.DriverManager. The deadlock occurs when > the > Evictor thread is trying to make enough connections to bring the pool's idle > connections up to what's specified in minIdle, at the same time a connection > is > being requested through DriverManager.getConnection(). See the attached stack > trace dump: > Found one Java-level deadlock: > ============================= > "Thread-0": > waiting to lock monitor 0x0809a994 (object 0x698c2b48, a java.lang.Class), > which is held by "main" > "main": > waiting to lock monitor 0x0809aad4 (object 0x65df7030, a > org.apache.commons.dbcp.PoolableConnectionFactory), > which is held by "Thread-0" > > Java stack information for the threads listed above: > =================================================== > "Thread-0": > at java.sql.DriverManager.getConnection(DriverManager.java:158) > - waiting to lock <0x698c2b48> (a java.lang.Class) > at > org.apache.commons.dbcp.DriverManagerConnectionFactory.createConnection(DriverManagerConnectionFactory.java:48) > at > org.apache.commons.dbcp.PoolableConnectionFactory.makeObject(PoolableConnectionFactory.java:290) > - locked <0x65df7030> (a > org.apache.commons.dbcp.PoolableConnectionFactory) > at > org.apache.commons.pool.impl.GenericObjectPool.addObject(GenericObjectPool.java:996) > at > org.apache.commons.pool.impl.GenericObjectPool.ensureMinIdle(GenericObjectPool.java:978) > at > org.apache.commons.pool.impl.GenericObjectPool.access$000(GenericObjectPool.java:124) > at > org.apache.commons.pool.impl.GenericObjectPool$Evictor.run(GenericObjectPool.java:1090) > at java.lang.Thread.run(Thread.java:595) > "main": > at > org.apache.commons.dbcp.PoolableConnectionFactory.makeObject(PoolableConnectionFactory.java:290) > - waiting to lock <0x65df7030> (a > org.apache.commons.dbcp.PoolableConnectionFactory) > at > org.apache.commons.pool.impl.GenericObjectPool.borrowObject(GenericObjectPool.java:771) > at > org.apache.commons.dbcp.PoolingDriver.connect(PoolingDriver.java:175) > at java.sql.DriverManager.getConnection(DriverManager.java:525) > - locked <0x698c2b48> (a java.lang.Class) > at java.sql.DriverManager.getConnection(DriverManager.java:193) > - locked <0x698c2b48> (a java.lang.Class) > at Deadlock.main(Deadlock.java:83) > > Found 1 deadlock. > The deadlock occurs when GenericObjectPool.addObject() calls synchronized > method > PoolableConnectionFactory.makeObject(), meanwhile static synchronized > DriverManager.getConnection() is called. makeObject() waits for the lock on > DriverManager to be released, whereas DriverManager waits for the lock on the > PoolableConnectionFactory instance to be released. > The Java program below, based on ManualPoolingDriverExample.java provided with > DBCP, duplicates the deadlock for me within several seconds of being run: > import java.sql.*; > import org.apache.commons.pool.*; > import org.apache.commons.pool.impl.*; > import org.apache.commons.dbcp.*; > > /** > * Duplicate DBCP pool deadlocking. > * > * Compile with: > * /usr/java/jdk1.5.0/bin/javac > * -classpath > commons-collections.jar:commons-dbcp-1.2.1.jar:commons-pool-1.2.jar > * Deadlock.java > * > * Run with: > * /usr/java/jdk1.5.0/bin/java > * -classpath > commons-collections.jar:commons-dbcp-1.2.1.jar:commons-pool-1.2.jar:ojdbc14.jar:xerces.jar:. > * Deadlock > * > * Locks still occur when compiled and run with J2SDK 1.4.1_03. > */ > public class Deadlock { > > private static final int ACTIVE = 10; > > private static void init() { > System.out.println("Loading drivers"); > > try { > Class.forName("oracle.jdbc.driver.OracleDriver"); > > Class.forName("org.apache.commons.dbcp.PoolingDriver"); > } catch (ClassNotFoundException e) { > e.printStackTrace(); > } > > System.out.println("Setting up pool"); > > try { > GenericObjectPool.Config config = new > GenericObjectPool.Config(); > config.maxActive = ACTIVE; > config.minIdle = 2; // Idle limits are low to allow > more > possibility of locking. > config.maxIdle = 4; // Locking only occurs when there > are 0 idle connections in the pool. > config.maxWait = 5000L; > config.testOnBorrow = true; > config.testOnReturn = false; > config.testWhileIdle = true; > // Locking still occurs whether these tests > are > performed or not. > config.whenExhaustedAction = > GenericObjectPool.WHEN_EXHAUSTED_BLOCK; > // Locking still occurs regardless of the > exhausted action. > config.timeBetweenEvictionRunsMillis = 3000L; // The > Evictor thread is involved in the lock, so run it quite often. > config.minEvictableIdleTimeMillis = 6000L; > config.numTestsPerEvictionRun = 3; > > ObjectPool op = new GenericObjectPool(null, config); > > ConnectionFactory cf = new > DriverManagerConnectionFactory("jdbc:oracle:thin:@oracle8server:1521:sid", > "username", "password"); > PoolableConnectionFactory pcf = new > PoolableConnectionFactory(cf, op, null, "SELECT 1 FROM DUAL", false, true); > // Locking still occurs whether there is a > validation query or not. > > PoolingDriver pd = > (PoolingDriver)DriverManager.getDriver("jdbc:apache:commons:dbcp:"); > pd.registerPool("PoolName", op); > > } catch (SQLException e) { > e.printStackTrace(); > } > > System.out.println("Done"); > } > > > public static void main(String[] args) { > init(); > > Connection[] c = new Connection[ACTIVE]; > > try { > printPoolStatus(); > > // Loop forever to create a high load. > while (true) { > // Create a number of connections. > for (int i = 0; i < ACTIVE; ++i) { > c[i] = > DriverManager.getConnection("jdbc:apache:commons:dbcp:PoolName"); > printPoolStatus(); > } > // Then immmediately drop them. > for (int i = 0; i < ACTIVE; ++i) { > try { > if (c[i] != null) { > c[i].close(); > printPoolStatus(); > c[i] = null; > } > } catch (SQLException e) { > e.printStackTrace(); > } > } > } > > } catch (SQLException e) { > e.printStackTrace(); > > } finally { > // Close down any open connetions. > for (int i = 0; i < ACTIVE; ++i) { > try { > if (c[i] != null) c[i].close(); > } catch (SQLException e) { } > } > > System.out.println("Closing pool"); > try { > PoolingDriver pd = > (PoolingDriver)DriverManager.getDriver("jdbc:apache:commons:dbcp:"); > pd.closePool("PoolName"); > } catch (SQLException e) { > e.printStackTrace(); > } > System.out.println("Pool closed"); > } > > } > > > /** > * Display pool status. Locks still occur even if this method is > never > called. > */ > private static void printPoolStatus() throws SQLException { > PoolingDriver pd = > (PoolingDriver)DriverManager.getDriver("jdbc:apache:commons:dbcp:"); > ObjectPool op = pd.getConnectionPool("PoolName"); > > System.out.println("Active / idle: " + op.getNumActive() + " > / " > + op.getNumIdle()); > } > > } > The patch I initially suggest is as follows (sorry for not providing a diff): > In org.apache.commons.dbcp.PoolableConnectionFactory.java we have: > synchronized public Object makeObject() throws Exception { > Connection conn = _connFactory.createConnection(); > if(null != _stmtPoolFactory) { > KeyedObjectPool stmtpool = _stmtPoolFactory.createPool(); > conn = new PoolingConnection(conn,stmtpool); > stmtpool.setFactory((PoolingConnection)conn); > } > return new PoolableConnection(conn,_pool,_config); > } > I suggest changing that to this: > public Object makeObject() throws Exception { > Connection conn = _connFactory.createConnection(); > synchronized (this) { > if(null != _stmtPoolFactory) { > KeyedObjectPool stmtpool = _stmtPoolFactory.createPool(); > conn = new PoolingConnection(conn,stmtpool); > stmtpool.setFactory((PoolingConnection)conn); > } > return new PoolableConnection(conn,_pool,_config); > } > } > Note the move of the synchronized block from the entire method to within the > method after the connection (obtained ultimately by DriverManager) is > retrieved. > I'm afraid I don't know enough about DBCP and pooling to know the full > ramification of this change and other calls to makeObject(). -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]