Author: fhanik Date: Fri Jul 10 21:02:43 2009 New Revision: 793108 URL: http://svn.apache.org/viewvc?rev=793108&view=rev Log: Fix a concurrency issue with connections being released and then trying to be reconnected
Added: tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/TestConcurrency.java (with props) Modified: tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/PoolProperties.java tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/PooledConnection.java tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/ConnectCountTest.java tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/driver/Connection.java tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/driver/Driver.java Modified: tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java URL: http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java?rev=793108&r1=793107&r2=793108&view=diff ============================================================================== --- tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java (original) +++ tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java Fri Jul 10 21:02:43 2009 @@ -648,6 +648,11 @@ boolean setToNull = false; try { con.lock(); + + if (con.isReleased()) { + return null; + } + if (!con.isDiscarded() && !con.isInitialized()) { //attempt to connect con.connect(); Modified: tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/PoolProperties.java URL: http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/PoolProperties.java?rev=793108&r1=793107&r2=793108&view=diff ============================================================================== --- tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/PoolProperties.java (original) +++ tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/PoolProperties.java Fri Jul 10 21:02:43 2009 @@ -72,7 +72,7 @@ protected boolean useEquals = false; protected int abandonWhenPercentageFull = 0; protected long maxAge = 0; - + protected boolean useLock = true; private InterceptorDefinition[] interceptors = null; public void setAbandonWhenPercentageFull(int percentage) { @@ -531,7 +531,15 @@ public void setMaxAge(long maxAge) { this.maxAge = maxAge; } + + public boolean getUseLock() { + return useLock; + } + + public void setUseLock(boolean useLock) { + this.useLock = useLock; + } - + } Modified: tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/PooledConnection.java URL: http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/PooledConnection.java?rev=793108&r1=793107&r2=793108&view=diff ============================================================================== --- tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/PooledConnection.java (original) +++ tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/PooledConnection.java Fri Jul 10 21:02:43 2009 @@ -112,7 +112,6 @@ private AtomicBoolean released = new AtomicBoolean(false); - public PooledConnection(PoolProperties prop, ConnectionPool parent) { instanceCount = counter.addAndGet(1); poolProperties = prop; @@ -367,7 +366,7 @@ * Otherwise this is a noop for performance */ public void lock() { - if (this.poolProperties.isPoolSweeperEnabled()) { + if (poolProperties.getUseLock() || this.poolProperties.isPoolSweeperEnabled()) { //optimized, only use a lock when there is concurrency lock.writeLock().lock(); } @@ -378,7 +377,7 @@ * Otherwise this is a noop for performance */ public void unlock() { - if (this.poolProperties.isPoolSweeperEnabled()) { + if (poolProperties.getUseLock() || this.poolProperties.isPoolSweeperEnabled()) { //optimized, only use a lock when there is concurrency lock.writeLock().unlock(); } @@ -423,5 +422,9 @@ public String toString() { return "PooledConnection["+(connection!=null?connection.toString():"null")+"]"; } + + public boolean isReleased() { + return released.get(); + } } Modified: tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/ConnectCountTest.java URL: http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/ConnectCountTest.java?rev=793108&r1=793107&r2=793108&view=diff ============================================================================== --- tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/ConnectCountTest.java (original) +++ tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/ConnectCountTest.java Fri Jul 10 21:02:43 2009 @@ -62,7 +62,7 @@ @Override protected void tearDown() throws Exception { - Driver.connectCount.set(0); + Driver.reset(); super.tearDown(); } Added: tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/TestConcurrency.java URL: http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/TestConcurrency.java?rev=793108&view=auto ============================================================================== --- tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/TestConcurrency.java (added) +++ tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/TestConcurrency.java Fri Jul 10 21:02:43 2009 @@ -0,0 +1,144 @@ +package org.apache.tomcat.jdbc.test; + +import java.sql.Connection; +import java.util.concurrent.atomic.AtomicInteger; + +import org.apache.tomcat.jdbc.pool.DataSource; +import org.apache.tomcat.jdbc.test.driver.Driver; + +public class TestConcurrency extends DefaultTestCase { + + public static final boolean debug = Boolean.getBoolean("jdbc.debug"); + + protected volatile DataSource ds = null; + + public TestConcurrency(String name) { + super(name); + } + + @Override + public void setUp() { + // TODO Auto-generated method stub + ds = createDefaultDataSource(); + ds.getPoolProperties().setDriverClassName(Driver.class.getName()); + ds.getPoolProperties().setUrl(Driver.url); + ds.getPoolProperties().setInitialSize(0); + ds.getPoolProperties().setMaxIdle(0); + ds.getPoolProperties().setMinIdle(0); + ds.getPoolProperties().setMaxActive(10); + ds.getPoolProperties().setRemoveAbandoned(true); + ds.getPoolProperties().setLogAbandoned(true); + ds.getPoolProperties().setTestWhileIdle(true); + ds.getPoolProperties().setMinEvictableIdleTimeMillis(750); + ds.getPoolProperties().setTimeBetweenEvictionRunsMillis(25); + } + + @Override + protected void tearDown() throws Exception { + Driver.reset(); + super.tearDown(); + } + + public void testSimple() throws Exception { + ds.getConnection().close(); + final int iter = 1000 * 10; + final AtomicInteger loopcount = new AtomicInteger(0); + final Runnable run = new Runnable() { + public void run() { + try { + while (loopcount.incrementAndGet() < iter) { + Connection con = ds.getConnection(); + Thread.sleep(10); + con.close(); + } + }catch (Exception x) { + loopcount.set(iter); //stops the test + x.printStackTrace(); + } + } + }; + Thread[] threads = new Thread[20]; + for (int i=0; i<threads.length; i++) { + threads[i] = new Thread(run); + } + for (int i=0; i<threads.length; i++) { + threads[i].start(); + } + try { + while (loopcount.get()<iter) { + assertEquals("Size comparison:",10, ds.getPool().getSize()); + if (debug) { + System.out.println("Size: "+ds.getPool().getSize()); + System.out.println("Used: "+ds.getPool().getActive()); + System.out.println("Idle: "+ds.getPool().getIdle()); + System.out.println("Wait: "+ds.getPool().getWaitCount()); + } + Thread.sleep(250); + } + }catch (Exception x) { + loopcount.set(iter); //stops the test + x.printStackTrace(); + } + for (int i=0; i<threads.length; i++) { + threads[i].join(); + } + assertEquals("Size comparison:",10, ds.getPool().getSize()); + assertEquals("Idle comparison:",10, ds.getPool().getIdle()); + assertEquals("Used comparison:",0, ds.getPool().getActive()); + assertEquals("Connect count",10,Driver.connectCount.get()); + + } + + public void testBrutal() throws Exception { + ds.getPoolProperties().setRemoveAbandoned(false); + ds.getPoolProperties().setMinEvictableIdleTimeMillis(-1); + ds.getPoolProperties().setTimeBetweenEvictionRunsMillis(-1); + ds.getConnection().close(); + final int iter = 1000 * 10; + final AtomicInteger loopcount = new AtomicInteger(0); + final Runnable run = new Runnable() { + public void run() { + try { + while (loopcount.incrementAndGet() < iter) { + Connection con = ds.getConnection(); + Thread.sleep(10); + con.close(); + } + }catch (Exception x) { + loopcount.set(iter); //stops the test + x.printStackTrace(); + } + } + }; + Thread[] threads = new Thread[20]; + for (int i=0; i<threads.length; i++) { + threads[i] = new Thread(run); + } + for (int i=0; i<threads.length; i++) { + threads[i].start(); + } + try { + while (loopcount.get()<iter) { + //assertEquals("Size comparison:",10, ds.getPool().getSize()); + ds.getPool().testAllIdle(); + ds.getPool().checkAbandoned(); + ds.getPool().checkIdle(); + } + }catch (Exception x) { + loopcount.set(iter); //stops the test + x.printStackTrace(); + } + for (int i=0; i<threads.length; i++) { + threads[i].join(); + } + System.out.println("Connect count:"+Driver.connectCount.get()); + System.out.println("DisConnect count:"+Driver.disconnectCount.get()); + assertEquals("Size comparison:",10, ds.getPool().getSize()); + assertEquals("Idle comparison:",10, ds.getPool().getIdle()); + assertEquals("Used comparison:",0, ds.getPool().getActive()); + assertEquals("Connect count",10,Driver.connectCount.get()); + + } + + +} Propchange: tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/TestConcurrency.java ------------------------------------------------------------------------------ svn:eol-style = native Modified: tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/driver/Connection.java URL: http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/driver/Connection.java?rev=793108&r1=793107&r2=793108&view=diff ============================================================================== --- tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/driver/Connection.java (original) +++ tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/driver/Connection.java Fri Jul 10 21:02:43 2009 @@ -39,6 +39,7 @@ } public void close() throws SQLException { + Driver.disconnectCount.incrementAndGet(); } public void commit() throws SQLException { Modified: tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/driver/Driver.java URL: http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/driver/Driver.java?rev=793108&r1=793107&r2=793108&view=diff ============================================================================== --- tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/driver/Driver.java (original) +++ tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/driver/Driver.java Fri Jul 10 21:02:43 2009 @@ -25,8 +25,14 @@ public class Driver implements java.sql.Driver { public static final String url = "jdbc:tomcat:test"; - public static AtomicInteger connectCount = new AtomicInteger(0); + public static final AtomicInteger connectCount = new AtomicInteger(0); + public static final AtomicInteger disconnectCount = new AtomicInteger(0); + public static void reset() { + connectCount.set(0); + disconnectCount.set(0); + } + static { try { DriverManager.registerDriver(new Driver()); --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org