commons-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject svn commit: r1735269 - in /commons/proper/pool/trunk/src: main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java main/java/org/apache/commons/pool2/impl/GenericObjectPool.java test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
Date Wed, 16 Mar 2016 17:20:41 GMT
Author: markt
Date: Wed Mar 16 17:20:41 2016
New Revision: 1735269

URL: http://svn.apache.org/viewvc?rev=1735269&view=rev
Log:
Re-working of fix for POOL-303

Modified:
    commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
    commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
    commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java

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=1735269&r1=1735268&r2=1735269&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
Wed Mar 16 17:20:41 2016
@@ -1017,26 +1017,58 @@ public class GenericKeyedObjectPool<K,T>
             }
         }
 
-        final long newCreateCount = objectDeque.getCreateCount().incrementAndGet();
+        // Flag that indicates if create should:
+        // - TRUE:  call the factory to create an object
+        // - FALSE: return null
+        // - null:  loop and re-test the condition that determines whether to
+        //          call the factory
+        Boolean create = null;
+        while (create == null) {
+            synchronized (objectDeque.makeObjectCountLock) {
+                final long newCreateCount = objectDeque.getCreateCount().incrementAndGet();
+                // Check against the per key limit
+                if (newCreateCount > maxTotalPerKeySave) {
+                    // The key is currently at capacity or in the process of
+                    // making enough new objects to take it to capacity.
+                    numTotal.decrementAndGet();
+                    objectDeque.getCreateCount().decrementAndGet();
+                    if (objectDeque.makeObjectCount == 0) {
+                        // There are no makeObject() calls in progress for this
+                        // key so the key is at capacity. Do not attempt to
+                        // create a new object. Return and wait for an object to
+                        // be returned.
+                        create = Boolean.FALSE;
+                    } else {
+                        // There are makeObject() calls in progress that might
+                        // bring the pool to capacity. Those calls might also
+                        // fail so wait until they complete and then re-test if
+                        // the pool is at capacity or not.
+                        objectDeque.makeObjectCountLock.wait();
+                    }
+                } else {
+                    // The pool is not at capacity. Create a new object.
+                    objectDeque.makeObjectCount++;
+                    create = Boolean.TRUE;
+                }
+            }
+        }
 
-        // Check against the per key limit
-        if (newCreateCount > maxTotalPerKeySave) {
-            numTotal.decrementAndGet();
-            objectDeque.getCreateCount().decrementAndGet();
+        if (!create.booleanValue()) {
             return null;
         }
 
-
         PooledObject<T> p = null;
         try {
             p = factory.makeObject(key);
         } catch (final Exception e) {
             numTotal.decrementAndGet();
             objectDeque.getCreateCount().decrementAndGet();
-            // POOL-303. There may be threads waiting on an object return that
-            // isn't going to happen. Unblock them.
-            objectDeque.idleObjects.interuptTakeWaiters();
             throw e;
+        } finally {
+            synchronized (objectDeque.makeObjectCountLock) {
+                objectDeque.makeObjectCount--;
+                objectDeque.makeObjectCountLock.notifyAll();
+            }
         }
 
         createdCount.incrementAndGet();
@@ -1431,6 +1463,9 @@ public class GenericKeyedObjectPool<K,T>
          */
         private final AtomicInteger createCount = new AtomicInteger(0);
 
+        private long makeObjectCount = 0;
+        private final Object makeObjectCountLock = new Object();
+
         /*
          * The map is keyed on pooled instances, wrapped to ensure that
          * they work properly as keys.

Modified: commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java?rev=1735269&r1=1735268&r2=1735269&view=diff
==============================================================================
--- commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
(original)
+++ commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
Wed Mar 16 17:20:41 2016
@@ -842,24 +842,59 @@ public class GenericObjectPool<T> extend
      */
     private PooledObject<T> create() throws Exception {
         int localMaxTotal = getMaxTotal();
+        // This simplifies the code later in this method
         if (localMaxTotal < 0) {
             localMaxTotal = Integer.MAX_VALUE;
         }
-        final long newCreateCount = createCount.incrementAndGet();
-        if (newCreateCount > localMaxTotal) {
-            createCount.decrementAndGet();
+
+        // Flag that indicates if create should:
+        // - TRUE:  call the factory to create an object
+        // - FALSE: return null
+        // - null:  loop and re-test the condition that determines whether to
+        //          call the factory
+        Boolean create = null;
+        while (create == null) {
+            synchronized (makeObjectCountLock) {
+                final long newCreateCount = createCount.incrementAndGet();
+                if (newCreateCount > localMaxTotal) {
+                    // The pool is currently at capacity or in the process of
+                    // making enough new objects to take it to capacity.
+                    createCount.decrementAndGet();
+                    if (makeObjectCount == 0) {
+                        // There are no makeObject() calls in progress so the
+                        // pool is at capacity. Do not attempt to create a new
+                        // object. Return and wait for an object to be returned
+                        create = Boolean.FALSE;
+                    } else {
+                        // There are makeObject() calls in progress that might
+                        // bring the pool to capacity. Those calls might also
+                        // fail so wait until they complete and then re-test if
+                        // the pool is at capacity or not.
+                        makeObjectCountLock.wait();
+                    }
+                } else {
+                    // The pool is not at capacity. Create a new object.
+                    makeObjectCount++;
+                    create = Boolean.TRUE;
+                }
+            }
+        }
+
+        if (!create.booleanValue()) {
             return null;
         }
 
         final PooledObject<T> p;
         try {
             p = factory.makeObject();
-        } catch (final Exception e) {
+        } catch (Exception e) {
             createCount.decrementAndGet();
-            // POOL-303. There may be threads waiting on an object return that
-            // isn't going to happen. Unblock them.
-            idleObjects.interuptTakeWaiters();
             throw e;
+        } finally {
+            synchronized (makeObjectCountLock) {
+                makeObjectCount--;
+                makeObjectCountLock.notifyAll();
+            }
         }
 
         final AbandonedConfig ac = this.abandonedConfig;
@@ -1129,6 +1164,8 @@ public class GenericObjectPool<T> extend
      * {@link #_maxActive} objects created at any one time.
      */
     private final AtomicLong createCount = new AtomicLong(0);
+    private long makeObjectCount = 0;
+    private final Object makeObjectCountLock = new Object();
     private final LinkedBlockingDeque<PooledObject<T>> idleObjects;
 
     // JMX specific attributes

Modified: commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java?rev=1735269&r1=1735268&r2=1735269&view=diff
==============================================================================
--- commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
(original)
+++ commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
Wed Mar 16 17:20:41 2016
@@ -25,6 +25,7 @@ import static org.junit.Assert.assertTru
 import static org.junit.Assert.fail;
 
 import java.lang.management.ManagementFactory;
+import java.nio.charset.UnsupportedCharsetException;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
@@ -2590,6 +2591,9 @@ public class TestGenericObjectPool exten
         }
         Assert.assertFalse(thread1.isAlive());
         Assert.assertFalse(thread2.isAlive());
+
+        Assert.assertTrue(thread1._thrown instanceof UnsupportedCharsetException);
+        Assert.assertTrue(thread2._thrown instanceof UnsupportedCharsetException);
     }
 
     private static class CreateFailFactory extends BasePooledObjectFactory<String>
{
@@ -2599,7 +2603,7 @@ public class TestGenericObjectPool exten
         @Override
         public String create() throws Exception {
             semaphore.acquire();
-            throw new Exception();
+            throw new UnsupportedCharsetException("wibble");
         }
 
         @Override



Mime
View raw message