You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by ps...@apache.org on 2011/11/22 23:47:58 UTC

svn commit: r1205211 - in /commons/proper/pool/branches/POOL_1_X/src: changes/ java/org/apache/commons/pool/impl/ test/org/apache/commons/pool/impl/

Author: psteitz
Date: Tue Nov 22 22:47:57 2011
New Revision: 1205211

URL: http://svn.apache.org/viewvc?rev=1205211&view=rev
Log:
Awaken threads waiting on borrowObject when a pool has been closed and have them throw
IllegalStateException.  Prior to the fix for this issue, threads waiting in borrowObject when
close was invoked on GOP or GKOP would block indefinitely.

JIRA: POOL-189
Reported by Adrian Nistor
Patched by Bill Speirs

Modified:
    commons/proper/pool/branches/POOL_1_X/src/changes/changes.xml
    commons/proper/pool/branches/POOL_1_X/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java
    commons/proper/pool/branches/POOL_1_X/src/java/org/apache/commons/pool/impl/GenericObjectPool.java
    commons/proper/pool/branches/POOL_1_X/src/test/org/apache/commons/pool/impl/TestGenericKeyedObjectPool.java
    commons/proper/pool/branches/POOL_1_X/src/test/org/apache/commons/pool/impl/TestGenericObjectPool.java

Modified: commons/proper/pool/branches/POOL_1_X/src/changes/changes.xml
URL: http://svn.apache.org/viewvc/commons/proper/pool/branches/POOL_1_X/src/changes/changes.xml?rev=1205211&r1=1205210&r2=1205211&view=diff
==============================================================================
--- commons/proper/pool/branches/POOL_1_X/src/changes/changes.xml (original)
+++ commons/proper/pool/branches/POOL_1_X/src/changes/changes.xml Tue Nov 22 22:47:57 2011
@@ -20,6 +20,12 @@
     <title>Commons Pool Changes</title>
   </properties>
   <body>
+  <release version="1.5.7" date="TBD" "This is a patch release, including bugfixes only.">
+    <action dev="psteitz" type="fix" issue="POOL-189" due-to="Bill Speirs">
+      Awaken threads waiting on borrowObject when a pool has been closed and have them throw
+      IllegalStateException.  Prior to the fix for this issue, threads waiting in borrowObject when
+      close was invoked on GOP or GKOP would block indefinitely.
+    </action>
   <release version="1.5.6" date="2011-04-03" description="This is a patch release, including bugfixes only.">
     <action dev="markt" type="fix" issue="POOL-179" due-to="Axel Grossmann">
       Correctly handle an InterruptedException when waiting for an object from

Modified: commons/proper/pool/branches/POOL_1_X/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java
URL: http://svn.apache.org/viewvc/commons/proper/pool/branches/POOL_1_X/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java?rev=1205211&r1=1205210&r2=1205211&view=diff
==============================================================================
--- commons/proper/pool/branches/POOL_1_X/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java (original)
+++ commons/proper/pool/branches/POOL_1_X/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java Tue Nov 22 22:47:57 2011
@@ -1159,6 +1159,10 @@ public class GenericKeyedObjectPool exte
                                         break;
                                     }
                                 }
+                                // see if we were awakened by a closing pool
+                                if(isClosed() == true) {
+                                    throw new IllegalStateException("Pool closed");
+                                }
                             } catch(InterruptedException e) {
                                 boolean doAllocate = false;
                                 synchronized (this) {
@@ -1794,6 +1798,15 @@ public class GenericKeyedObjectPool exte
                 _evictionKeyCursor = null;
             }
             startEvictor(-1L);
+            
+            while(_allocationQueue.size() > 0) {
+                Latch l = (Latch) _allocationQueue.remove();
+                
+                synchronized (l) {
+                    // notify the waiting thread
+                    l.notify();
+                }
+            }
         }
     }
 

Modified: commons/proper/pool/branches/POOL_1_X/src/java/org/apache/commons/pool/impl/GenericObjectPool.java
URL: http://svn.apache.org/viewvc/commons/proper/pool/branches/POOL_1_X/src/java/org/apache/commons/pool/impl/GenericObjectPool.java?rev=1205211&r1=1205210&r2=1205211&view=diff
==============================================================================
--- commons/proper/pool/branches/POOL_1_X/src/java/org/apache/commons/pool/impl/GenericObjectPool.java (original)
+++ commons/proper/pool/branches/POOL_1_X/src/java/org/apache/commons/pool/impl/GenericObjectPool.java Tue Nov 22 22:47:57 2011
@@ -1127,6 +1127,10 @@ public class GenericObjectPool extends B
                                         break;
                                     }
                                 }
+                                // see if we were awakened by a closing pool
+                                if(isClosed() == true) {
+                                    throw new IllegalStateException("Pool closed");
+                                }
                             } catch(InterruptedException e) {
                                 boolean doAllocate = false;
                                 synchronized(this) {
@@ -1481,6 +1485,15 @@ public class GenericObjectPool extends B
         synchronized (this) {
             clear();
             startEvictor(-1L);
+
+            while(_allocationQueue.size() > 0) {
+                Latch l = (Latch) _allocationQueue.remove();
+                
+                synchronized (l) {
+                    // notify the waiting thread
+                    l.notify();
+                }
+            }
         }
     }
 

Modified: commons/proper/pool/branches/POOL_1_X/src/test/org/apache/commons/pool/impl/TestGenericKeyedObjectPool.java
URL: http://svn.apache.org/viewvc/commons/proper/pool/branches/POOL_1_X/src/test/org/apache/commons/pool/impl/TestGenericKeyedObjectPool.java?rev=1205211&r1=1205210&r2=1205211&view=diff
==============================================================================
--- commons/proper/pool/branches/POOL_1_X/src/test/org/apache/commons/pool/impl/TestGenericKeyedObjectPool.java (original)
+++ commons/proper/pool/branches/POOL_1_X/src/test/org/apache/commons/pool/impl/TestGenericKeyedObjectPool.java Tue Nov 22 22:47:57 2011
@@ -29,6 +29,7 @@ import org.apache.commons.pool.TestBaseK
 import org.apache.commons.pool.VisitTracker;
 import org.apache.commons.pool.VisitTrackerFactory;
 import org.apache.commons.pool.WaiterFactory;
+import org.apache.commons.pool.impl.TestGenericObjectPool.WaitingTestThread;
 
 /**
  * @author Rodney Waldhoff
@@ -219,6 +220,31 @@ public class TestGenericKeyedObjectPool 
         }
     }
 
+    public void testWhenExhaustedBlockClosePool() throws Exception {
+        pool.setMaxActive(1);
+        pool.setWhenExhaustedAction(GenericObjectPool.WHEN_EXHAUSTED_BLOCK);
+        pool.setMaxWait(0);
+        Object obj1 = pool.borrowObject("a");
+        
+        // Make sure an object was obtained
+        assertNotNull(obj1);
+        
+        // Create a separate thread to try and borrow another object
+        WaitingTestThread wtt = new WaitingTestThread(pool, "a", 200);
+        wtt.start();
+        // Give wtt time to start
+        Thread.sleep(200);
+        
+        // close the pool (Bug POOL-189)
+        pool.close();
+        
+        // Give interrupt time to take effect
+        Thread.sleep(200);
+        
+        // Check thread was interrupted
+        assertTrue(wtt._thrown instanceof IllegalStateException);
+    }
+
     public void testMaxTotal() throws Exception {
         pool.setMaxActive(2);
         pool.setMaxTotal(3);

Modified: commons/proper/pool/branches/POOL_1_X/src/test/org/apache/commons/pool/impl/TestGenericObjectPool.java
URL: http://svn.apache.org/viewvc/commons/proper/pool/branches/POOL_1_X/src/test/org/apache/commons/pool/impl/TestGenericObjectPool.java?rev=1205211&r1=1205210&r2=1205211&view=diff
==============================================================================
--- commons/proper/pool/branches/POOL_1_X/src/test/org/apache/commons/pool/impl/TestGenericObjectPool.java (original)
+++ commons/proper/pool/branches/POOL_1_X/src/test/org/apache/commons/pool/impl/TestGenericObjectPool.java Tue Nov 22 22:47:57 2011
@@ -116,7 +116,7 @@ public class TestGenericObjectPool exten
         pool.setMaxWait(0);
         Object obj1 = pool.borrowObject();
         
-        // Make sure on object was obtained
+        // Make sure an object was obtained
         assertNotNull(obj1);
         
         // Create a separate thread to try and borrow another object
@@ -147,7 +147,31 @@ public class TestGenericObjectPool exten
         }
         pool.returnObject(obj2);
         pool.close();
+    }
+
+    public void testWhenExhaustedBlockClosePool() throws Exception {
+        pool.setMaxActive(1);
+        pool.setWhenExhaustedAction(GenericObjectPool.WHEN_EXHAUSTED_BLOCK);
+        pool.setMaxWait(0);
+        Object obj1 = pool.borrowObject();
+        
+        // Make sure an object was obtained
+        assertNotNull(obj1);
         
+        // Create a separate thread to try and borrow another object
+        WaitingTestThread wtt = new WaitingTestThread(pool, 200);
+        wtt.start();
+        // Give wtt time to start
+        Thread.sleep(200);
+        
+        // close the pool (Bug POOL-189)
+        pool.close();
+        
+        // Give interrupt time to take effect
+        Thread.sleep(200);
+        
+        // Check thread was interrupted
+        assertTrue(wtt._thrown instanceof IllegalStateException);
     }
 
     public void testEvictWhileEmpty() throws Exception {