You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by fh...@apache.org on 2009/07/10 23:02:43 UTC

svn commit: r793108 - in /tomcat/trunk/modules/jdbc-pool: java/org/apache/tomcat/jdbc/pool/ test/org/apache/tomcat/jdbc/test/ test/org/apache/tomcat/jdbc/test/driver/

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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org