Author: fhanik
Date: Thu Jul 24 14:00:38 2014
New Revision: 1613137

URL: http://svn.apache.org/r1613137
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=53367
Fix interrupt handling in the FairBlockingQueue

Added:
    
tomcat/tc7.0.x/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/bugs/Bug53367.java
   (with props)
Modified:
    
tomcat/tc7.0.x/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/FairBlockingQueue.java

Modified: 
tomcat/tc7.0.x/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/FairBlockingQueue.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/FairBlockingQueue.java?rev=1613137&r1=1613136&r2=1613137&view=diff
==============================================================================
--- 
tomcat/tc7.0.x/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/FairBlockingQueue.java
 (original)
+++ 
tomcat/tc7.0.x/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/FairBlockingQueue.java
 Thu Jul 24 14:00:38 2014
@@ -148,14 +148,36 @@ public class FairBlockingQueue<E> implem
                 //unlock the global lock
                 lock.unlock();
                 //wait for the specified timeout
-                if (!c.await(timeout, unit)) {
-                    //if we timed out, remove ourselves from the waitlist
+                boolean didtimeout = true;
+                InterruptedException interruptedException = null;
+                try {
+                    //wait for the specified timeout
+                    didtimeout = !c.await(timeout, unit);
+                } catch (InterruptedException ix) {
+                    interruptedException = ix;
+                }
+                if (didtimeout) {
+                    //if we timed out, or got interrupted
+                    // remove ourselves from the waitlist
                     lock.lock();
-                    waiters.remove(c);
-                    lock.unlock();
+                    try {
+                        waiters.remove(c);
+                    } finally {
+                        lock.unlock();
+                    }
                 }
                 //return the item we received, can be null if we timed out
                 result = c.getItem();
+                if (null!=interruptedException) {
+                    //we got interrupted
+                    if (null!=result) {
+                        //we got a result - clear the interrupt status
+                        //don't propagate cause we have removed a connection 
from pool
+                        Thread.interrupted();
+                    } else {
+                        throw interruptedException;
+                    }
+                }
             } else {
                 //we have an object, release
                 lock.unlock();

Added: 
tomcat/tc7.0.x/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/bugs/Bug53367.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/bugs/Bug53367.java?rev=1613137&view=auto
==============================================================================
--- 
tomcat/tc7.0.x/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/bugs/Bug53367.java
 (added)
+++ 
tomcat/tc7.0.x/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/bugs/Bug53367.java
 Thu Jul 24 14:00:38 2014
@@ -0,0 +1,163 @@
+package org.apache.tomcat.jdbc.bugs;
+
+
+
+import org.apache.tomcat.jdbc.test.DefaultProperties;
+import org.junit.Assert;
+import org.apache.tomcat.jdbc.pool.ConnectionPool;
+import org.apache.tomcat.jdbc.pool.DataSource;
+import org.apache.tomcat.jdbc.pool.PoolExhaustedException;
+import org.apache.tomcat.jdbc.pool.PoolProperties;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.ArrayBlockingQueue;
+
+@RunWith(Parameterized.class)
+public class Bug53367 {
+
+    private boolean fairQueue;
+
+    public Bug53367(boolean fair) {
+        this.fairQueue = fair;
+    }
+
+    @Parameterized.Parameters
+    public static Collection parameters() {
+        return Arrays.asList(new Object[][]{
+            new Object[] {Boolean.TRUE},
+            new Object[] {Boolean.FALSE},
+        });
+    }
+
+    @Test
+    public void testPool() throws SQLException, InterruptedException {
+        DriverManager.setLoginTimeout(1);
+        PoolProperties poolProperties = new DefaultProperties();
+        int threadsCount = 3;
+        poolProperties.setMaxActive(threadsCount);
+        poolProperties.setMaxIdle(threadsCount);
+        poolProperties.setMinIdle(0);
+        poolProperties.setMaxWait(5000);
+        poolProperties.setInitialSize(0);
+        poolProperties.setRemoveAbandoned(true);
+        poolProperties.setRemoveAbandonedTimeout(300);
+        poolProperties.setRollbackOnReturn(true);
+        poolProperties.setFairQueue(fairQueue);
+        final DataSource ds = new DataSource(poolProperties);
+
+        final CountDownLatch openedLatch = new CountDownLatch(threadsCount);
+        final CountDownLatch closedLatch = new CountDownLatch(threadsCount);
+        final CountDownLatch toCloseLatch = new CountDownLatch(1);
+
+        for (int i = 0; i < threadsCount; i++) {
+            new Thread(new Runnable() {
+                @Override
+                public void run() {
+                    try {
+                        Connection connection = ds.getConnection();
+                        openedLatch.countDown();
+
+                        toCloseLatch.await();
+                        connection.close();
+
+                        closedLatch.countDown();
+
+                    } catch (Exception e) {
+                        System.err.println("Step 1:"+e.getMessage());
+                    }
+                }
+            }).start();
+        }
+
+        openedLatch.await();
+        ConnectionPool pool = ds.getPool();
+        //Now we have 3 initialized busy connections
+        Assert.assertEquals(0, pool.getIdle());
+        Assert.assertEquals(threadsCount, pool.getActive());
+        Assert.assertEquals(threadsCount, pool.getSize());
+
+        List<Thread> threads = new ArrayList<Thread>();
+        for (int i = 0; i < threadsCount; i++) {
+            Thread thread = new Thread(new Runnable() {
+                @Override
+                public void run() {
+                    try {
+                        ds.getConnection();
+                    } catch (Exception e) {
+                        System.err.println("Step 2:"+e.getMessage());
+                    }
+                }
+            });
+            thread.start();
+            threads.add(thread);
+        }
+        for (Thread thread : threads) {
+            thread.interrupt();
+        }
+        for (Thread thread : threads) {
+            thread.join();
+        }
+        //Still 3 active connections
+        Assert.assertEquals(0, pool.getIdle());
+        Assert.assertEquals(threadsCount, pool.getActive());
+        Assert.assertEquals(threadsCount, pool.getSize());
+
+        toCloseLatch.countDown();
+        closedLatch.await();
+
+        //Here comes the bug! No more active connections and unable to 
establish new connections.
+
+        Assert.assertEquals(threadsCount, pool.getIdle()); // <-- Should be 
threadsCount (3) here
+        Assert.assertEquals(0, pool.getActive());
+        Assert.assertEquals(threadsCount, pool.getSize());
+
+        final AtomicInteger failedCount = new AtomicInteger();
+        final ArrayBlockingQueue<Connection> cons = new 
ArrayBlockingQueue<Connection>(threadsCount);
+        threads.clear();
+        for (int i = 0; i < threadsCount; i++) {
+            Thread thread = new Thread(new Runnable() {
+                @Override
+                public void run() {
+                    try {
+                        cons.add(ds.getConnection());
+                    } catch (PoolExhaustedException e) {
+                        failedCount.incrementAndGet();
+                        System.err.println("Step 3:"+e.getMessage());
+                    } catch (Exception e) {
+                        System.err.println("Step 4:"+e.getMessage());
+                        throw new RuntimeException(e);
+                    }
+                }
+            });
+            thread.start();
+            threads.add(thread);
+        }
+
+        for (Thread thread : threads) {
+            thread.join();
+        }
+        Assert.assertEquals(0, failedCount.get());
+
+        Assert.assertEquals(0, pool.getIdle());
+        Assert.assertEquals(threadsCount, pool.getActive());
+        Assert.assertEquals(threadsCount, pool.getSize());
+        for (Connection con : cons) {
+            con.close();
+        }
+        Assert.assertEquals(threadsCount, pool.getIdle());
+        Assert.assertEquals(0, pool.getActive());
+        Assert.assertEquals(threadsCount, pool.getSize());
+    }
+}
\ No newline at end of file

Propchange: 
tomcat/tc7.0.x/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/bugs/Bug53367.java
------------------------------------------------------------------------------
    svn:eol-style = native



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to