You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by ma...@apache.org on 2017/03/12 19:48:03 UTC

[33/50] [abbrv] commons-pool git commit: Fix POOL-310 Ensure that threads using GKOP do not block indefinitely if more than maxTotal threads try to borrow objects with different keys at the same time and the factory destroys objects on return.

Fix POOL-310
Ensure that threads using GKOP do not block indefinitely if more than maxTotal threads try to borrow objects with different keys at the same time and the factory destroys objects on return. 

git-svn-id: https://svn.apache.org/repos/asf/commons/proper/pool/trunk@1735563 13f79535-47bb-0310-9956-ffa450edef68


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

Branch: refs/heads/master
Commit: 9257fb9e40526f7ad33ac83f0b7f152607510ccd
Parents: f7a0d26
Author: Mark Thomas <ma...@apache.org>
Authored: Fri Mar 18 09:53:38 2016 +0000
Committer: Mark Thomas <ma...@apache.org>
Committed: Fri Mar 18 09:53:38 2016 +0000

----------------------------------------------------------------------
 src/changes/changes.xml                         |  5 +
 .../pool2/impl/GenericKeyedObjectPool.java      | 97 ++++++++++----------
 2 files changed, 53 insertions(+), 49 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/commons-pool/blob/9257fb9e/src/changes/changes.xml
----------------------------------------------------------------------
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 6b48a1f..65ca2b8 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -62,6 +62,11 @@ The <action> type attribute can be add,update,fix,remove.
     <action dev="markt" issue="POOL-307" type="update" due-to="Anthony Whitford">
       Replace inefficient use of keySet with entrySet in GKOP.
     </action>
+    <action dev="markt" issue="POOL-310" type="fix" due-to="Ivan Iliev">
+      Ensure that threads using GKOP do not block indefinitely if more than
+      maxTotal threads try to borrow objects with different keys at the same
+      time and the factory destroys objects on return. 
+    </action>
   </release>
   <release version="2.4.2" date="2015-08-01" description=
  "This is a patch release, including bug fixes only.">

http://git-wip-us.apache.org/repos/asf/commons-pool/blob/9257fb9e/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java b/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
index 200c424..6976f2a 100644
--- a/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
+++ b/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
@@ -478,8 +478,29 @@ public class GenericKeyedObjectPool<K,T> extends BaseGenericObjectPool<T>
 
         final long activeTime = p.getActiveTimeMillis();
 
-        if (getTestOnReturn()) {
-            if (!factory.validateObject(key, p)) {
+        try {
+            if (getTestOnReturn()) {
+                if (!factory.validateObject(key, p)) {
+                    try {
+                        destroy(key, p, true);
+                    } catch (final Exception e) {
+                        swallowException(e);
+                    }
+                    if (objectDeque.idleObjects.hasTakeWaiters()) {
+                        try {
+                            addObject(key);
+                        } catch (final Exception e) {
+                            swallowException(e);
+                        }
+                    }
+                    return;
+                }
+            }
+
+            try {
+                factory.passivateObject(key, p);
+            } catch (final Exception e1) {
+                swallowException(e1);
                 try {
                     destroy(key, p, true);
                 } catch (final Exception e) {
@@ -492,65 +513,43 @@ public class GenericKeyedObjectPool<K,T> extends BaseGenericObjectPool<T>
                         swallowException(e);
                     }
                 }
-                updateStatsReturn(activeTime);
                 return;
             }
-        }
 
-        try {
-            factory.passivateObject(key, p);
-        } catch (final Exception e1) {
-            swallowException(e1);
-            try {
-                destroy(key, p, true);
-            } catch (final Exception e) {
-                swallowException(e);
+            if (!p.deallocate()) {
+                throw new IllegalStateException(
+                        "Object has already been returned to this pool");
             }
-            if (objectDeque.idleObjects.hasTakeWaiters()) {
+
+            final int maxIdle = getMaxIdlePerKey();
+            final LinkedBlockingDeque<PooledObject<T>> idleObjects =
+                objectDeque.getIdleObjects();
+
+            if (isClosed() || maxIdle > -1 && maxIdle <= idleObjects.size()) {
                 try {
-                    addObject(key);
+                    destroy(key, p, true);
                 } catch (final Exception e) {
                     swallowException(e);
                 }
-            }
-            updateStatsReturn(activeTime);
-            return;
-        }
-
-        if (!p.deallocate()) {
-            throw new IllegalStateException(
-                    "Object has already been returned to this pool");
-        }
-
-        final int maxIdle = getMaxIdlePerKey();
-        final LinkedBlockingDeque<PooledObject<T>> idleObjects =
-            objectDeque.getIdleObjects();
-
-        if (isClosed() || maxIdle > -1 && maxIdle <= idleObjects.size()) {
-            try {
-                destroy(key, p, true);
-            } catch (final Exception e) {
-                swallowException(e);
-            }
-        } else {
-            if (getLifo()) {
-                idleObjects.addFirst(p);
             } else {
-                idleObjects.addLast(p);
+                if (getLifo()) {
+                    idleObjects.addFirst(p);
+                } else {
+                    idleObjects.addLast(p);
+                }
+                if (isClosed()) {
+                    // Pool closed while object was being added to idle objects.
+                    // Make sure the returned object is destroyed rather than left
+                    // in the idle object pool (which would effectively be a leak)
+                    clear(key);
+                }
             }
-            if (isClosed()) {
-                // Pool closed while object was being added to idle objects.
-                // Make sure the returned object is destroyed rather than left
-                // in the idle object pool (which would effectively be a leak)
-                clear(key);
+        } finally {
+            if (hasBorrowWaiters()) {
+                reuseCapacity();
             }
+            updateStatsReturn(activeTime);
         }
-
-        if (hasBorrowWaiters()) {
-            reuseCapacity();
-        }
-
-        updateStatsReturn(activeTime);
     }