POOL-356 fix deadlock on massive concurrent requests

Happens when blockWhenExhausted is active.
The idleObjects.takeFirst and .pollFirst operations perform a lock.
So if the pool is empty and the MaxTotal is hit we end up having multiple
Threads perform a lock internally via idleObjects.takeFirst().

But if a condition occurs to destroy the object instead of putting it back into 
the pool
then nobody notifies the Lock Condition.

The proposed solution will simply create a new object in that case and puts
it back into the idle queue IF there are active threads waiting for
some idle objects.


Project: http://git-wip-us.apache.org/repos/asf/commons-pool/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-pool/commit/016a1f67
Tree: http://git-wip-us.apache.org/repos/asf/commons-pool/tree/016a1f67
Diff: http://git-wip-us.apache.org/repos/asf/commons-pool/diff/016a1f67

Branch: refs/heads/master
Commit: 016a1f67263fe1cde1d910dc7002d972811951c5
Parents: 60a041d
Author: Mark Struberg <strub...@apache.org>
Authored: Wed Oct 24 21:23:20 2018 +0200
Committer: Mark Struberg <strub...@apache.org>
Committed: Wed Oct 24 21:23:20 2018 +0200

----------------------------------------------------------------------
 .../org/apache/commons/pool2/impl/GenericObjectPool.java    | 9 +++++++++
 .../apache/commons/pool2/impl/TestGenericObjectPool.java    | 3 ++-
 2 files changed, 11 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/commons-pool/blob/016a1f67/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java 
b/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
index abdf7e1..0575f7e 100644
--- a/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
+++ b/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
@@ -914,6 +914,15 @@ public class GenericObjectPool<T> extends 
BaseGenericObjectPool<T>
             destroyedCount.incrementAndGet();
             createCount.decrementAndGet();
         }
+
+        if (idleObjects.isEmpty() && idleObjects.hasTakeWaiters()) {
+            // POOL-356.
+            // In case there are already threads waiting on something in the 
pool
+            // (e.g. idleObjects.takeFirst(); then we need to provide them a 
fresh instance.
+            // Otherwise they will be stuck forever (or until timeout)
+            PooledObject<T> freshPooled = create();
+            idleObjects.put(freshPooled);
+        }
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/commons-pool/blob/016a1f67/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
----------------------------------------------------------------------
diff --git 
a/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java 
b/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
index 3e897f7..fd5a871 100644
--- a/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
+++ b/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
@@ -777,7 +777,7 @@ public class TestGenericObjectPool extends 
TestBaseObjectPool {
         }
         // Give the threads a chance to start doing some work
         try {
-            Thread.sleep(5000);
+            Thread.sleep(100);
         } catch(final InterruptedException e) {
             // ignored
         }
@@ -824,6 +824,7 @@ public class TestGenericObjectPool extends 
TestBaseObjectPool {
                 }
             }
             if(threads[i].failed()) {
+                threads[i]._error.printStackTrace();
                 fail("Thread "+i+" failed: "+threads[i]._error.toString());
             }
         }

Reply via email to