Mailing-List: contact issues-help@commons.apache.org; run by ezmlm
Precedence: bulk
Reply-To: issues@commons.apache.org
Message-ID: <759872200.1204258731131.JavaMail.jira@brutus>
Date: Thu, 28 Feb 2008 20:18:51 -0800 (PST)
From: "David Rees (JIRA)" <jira@apache.org>
To: issues@commons.apache.org
Subject: [jira] Commented: (DBCP-44) [dbcp] Evictor thread in
 GenericObjectPool has potential for deadlock
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


    [ https://issues.apache.org/jira/browse/DBCP-44?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12573584#action_12573584 ] 

David Rees commented on DBCP-44:
--------------------------------

I've been commenting on the wrong bug it appears, my deadlock case appears to be the same as DBCP-65 (which is "fixed"?)

> [dbcp] Evictor thread in GenericObjectPool has potential for deadlock
> ---------------------------------------------------------------------
>
>                 Key: DBCP-44
>                 URL: https://issues.apache.org/jira/browse/DBCP-44
>             Project: Commons Dbcp
>          Issue Type: Bug
>    Affects Versions: 1.2.1
>         Environment: Operating System: All
> Platform: All
>            Reporter: Eric Layton
>             Fix For: 1.3
>
>         Attachments: DBCP-44.patch, deadlock.txt, deadlock_post_patch.txt, testConcurrency.java
>
>
> 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.
-
You can reply to this email to add a comment to the issue online.