commons-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject svn commit: r1735563 - in /commons/proper/pool/trunk/src: changes/changes.xml main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
Date Fri, 18 Mar 2016 09:53:38 GMT
Author: markt
Date: Fri Mar 18 09:53:38 2016
New Revision: 1735563

URL: http://svn.apache.org/viewvc?rev=1735563&view=rev
Log:
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. 

Modified:
    commons/proper/pool/trunk/src/changes/changes.xml
    commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java

Modified: commons/proper/pool/trunk/src/changes/changes.xml
URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/changes/changes.xml?rev=1735563&r1=1735562&r2=1735563&view=diff
==============================================================================
--- commons/proper/pool/trunk/src/changes/changes.xml (original)
+++ commons/proper/pool/trunk/src/changes/changes.xml Fri Mar 18 09:53:38 2016
@@ -62,6 +62,11 @@ The <action> type attribute can be add,u
     <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.">

Modified: commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java?rev=1735563&r1=1735562&r2=1735563&view=diff
==============================================================================
--- commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
(original)
+++ commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
Fri Mar 18 09:53:38 2016
@@ -478,8 +478,29 @@ public class GenericKeyedObjectPool<K,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>
                         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);
     }
 
 



Mime
View raw message