tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject svn commit: r1743721 [2/3] - in /tomcat/tc8.5.x/trunk: ./ java/org/apache/tomcat/dbcp/pool2/ java/org/apache/tomcat/dbcp/pool2/impl/ webapps/docs/
Date Fri, 13 May 2016 18:50:09 GMT
Modified: tomcat/tc8.5.x/trunk/java/org/apache/tomcat/dbcp/pool2/impl/GenericKeyedObjectPool.java
URL: http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/java/org/apache/tomcat/dbcp/pool2/impl/GenericKeyedObjectPool.java?rev=1743721&r1=1743720&r2=1743721&view=diff
==============================================================================
--- tomcat/tc8.5.x/trunk/java/org/apache/tomcat/dbcp/pool2/impl/GenericKeyedObjectPool.java (original)
+++ tomcat/tc8.5.x/trunk/java/org/apache/tomcat/dbcp/pool2/impl/GenericKeyedObjectPool.java Fri May 13 18:50:08 2016
@@ -80,7 +80,7 @@ public class GenericKeyedObjectPool<K,T>
      * {@link GenericKeyedObjectPoolConfig}.
      * @param factory the factory to be used to create entries
      */
-    public GenericKeyedObjectPool(KeyedPooledObjectFactory<K,T> factory) {
+    public GenericKeyedObjectPool(final KeyedPooledObjectFactory<K,T> factory) {
         this(factory, new GenericKeyedObjectPoolConfig());
     }
 
@@ -94,8 +94,8 @@ public class GenericKeyedObjectPool<K,T>
      *                  the configuration object will not be reflected in the
      *                  pool.
      */
-    public GenericKeyedObjectPool(KeyedPooledObjectFactory<K,T> factory,
-            GenericKeyedObjectPoolConfig config) {
+    public GenericKeyedObjectPool(final KeyedPooledObjectFactory<K,T> factory,
+            final GenericKeyedObjectPoolConfig config) {
 
         super(config, ONAME_BASE, config.getJmxNamePrefix());
 
@@ -134,7 +134,7 @@ public class GenericKeyedObjectPool<K,T>
      *
      * @see #getMaxTotalPerKey
      */
-    public void setMaxTotalPerKey(int maxTotalPerKey) {
+    public void setMaxTotalPerKey(final int maxTotalPerKey) {
         this.maxTotalPerKey = maxTotalPerKey;
     }
 
@@ -175,7 +175,7 @@ public class GenericKeyedObjectPool<K,T>
      *
      * @see #getMaxIdlePerKey
      */
-    public void setMaxIdlePerKey(int maxIdlePerKey) {
+    public void setMaxIdlePerKey(final int maxIdlePerKey) {
         this.maxIdlePerKey = maxIdlePerKey;
     }
 
@@ -197,7 +197,7 @@ public class GenericKeyedObjectPool<K,T>
      * @see #getMaxIdlePerKey()
      * @see #setTimeBetweenEvictionRunsMillis
      */
-    public void setMinIdlePerKey(int minIdlePerKey) {
+    public void setMinIdlePerKey(final int minIdlePerKey) {
         this.minIdlePerKey = minIdlePerKey;
     }
 
@@ -219,7 +219,7 @@ public class GenericKeyedObjectPool<K,T>
      */
     @Override
     public int getMinIdlePerKey() {
-        int maxIdlePerKeySave = getMaxIdlePerKey();
+        final int maxIdlePerKeySave = getMaxIdlePerKey();
         if (this.minIdlePerKey > maxIdlePerKeySave) {
             return maxIdlePerKeySave;
         }
@@ -233,7 +233,7 @@ public class GenericKeyedObjectPool<K,T>
      *
      * @see GenericKeyedObjectPoolConfig
      */
-    public void setConfig(GenericKeyedObjectPoolConfig conf) {
+    public void setConfig(final GenericKeyedObjectPoolConfig conf) {
         setLifo(conf.getLifo());
         setMaxIdlePerKey(conf.getMaxIdlePerKey());
         setMaxTotalPerKey(conf.getMaxTotalPerKey());
@@ -271,7 +271,7 @@ public class GenericKeyedObjectPool<K,T>
      * {@inheritDoc}
      */
     @Override
-    public T borrowObject(K key) throws Exception {
+    public T borrowObject(final K key) throws Exception {
         return borrowObject(key, getMaxWaitMillis());
     }
 
@@ -331,30 +331,30 @@ public class GenericKeyedObjectPool<K,T>
      * @throws Exception if a keyed object instance cannot be returned due to an
      *                   error
      */
-    public T borrowObject(K key, long borrowMaxWaitMillis) throws Exception {
+    public T borrowObject(final K key, final long borrowMaxWaitMillis) throws Exception {
         assertOpen();
 
         PooledObject<T> p = null;
 
         // Get local copy of current config so it is consistent for entire
         // method execution
-        boolean blockWhenExhausted = getBlockWhenExhausted();
+        final boolean blockWhenExhausted = getBlockWhenExhausted();
 
         boolean create;
-        long waitTime = System.currentTimeMillis();
-        ObjectDeque<T> objectDeque = register(key);
+        final long waitTime = System.currentTimeMillis();
+        final ObjectDeque<T> objectDeque = register(key);
 
         try {
             while (p == null) {
                 create = false;
-                if (blockWhenExhausted) {
-                    p = objectDeque.getIdleObjects().pollFirst();
-                    if (p == null) {
-                        p = create(key);
-                        if (p != null) {
-                            create = true;
-                        }
+                p = objectDeque.getIdleObjects().pollFirst();
+                if (p == null) {
+                    p = create(key);
+                    if (p != null) {
+                        create = true;
                     }
+                }
+                if (blockWhenExhausted) {
                     if (p == null) {
                         if (borrowMaxWaitMillis < 0) {
                             p = objectDeque.getIdleObjects().takeFirst();
@@ -367,37 +367,27 @@ public class GenericKeyedObjectPool<K,T>
                         throw new NoSuchElementException(
                                 "Timeout waiting for idle object");
                     }
-                    if (!p.allocate()) {
-                        p = null;
-                    }
                 } else {
-                    p = objectDeque.getIdleObjects().pollFirst();
-                    if (p == null) {
-                        p = create(key);
-                        if (p != null) {
-                            create = true;
-                        }
-                    }
                     if (p == null) {
                         throw new NoSuchElementException("Pool exhausted");
                     }
-                    if (!p.allocate()) {
-                        p = null;
-                    }
+                }
+                if (!p.allocate()) {
+                    p = null;
                 }
 
                 if (p != null) {
                     try {
                         factory.activateObject(key, p);
-                    } catch (Exception e) {
+                    } catch (final Exception e) {
                         try {
                             destroy(key, p, true);
-                        } catch (Exception e1) {
+                        } catch (final Exception e1) {
                             // Ignore - activation failure is more important
                         }
                         p = null;
                         if (create) {
-                            NoSuchElementException nsee = new NoSuchElementException(
+                            final NoSuchElementException nsee = new NoSuchElementException(
                                     "Unable to activate object");
                             nsee.initCause(e);
                             throw nsee;
@@ -408,7 +398,7 @@ public class GenericKeyedObjectPool<K,T>
                         Throwable validationThrowable = null;
                         try {
                             validate = factory.validateObject(key, p);
-                        } catch (Throwable t) {
+                        } catch (final Throwable t) {
                             PoolUtils.checkRethrow(t);
                             validationThrowable = t;
                         }
@@ -416,12 +406,12 @@ public class GenericKeyedObjectPool<K,T>
                             try {
                                 destroy(key, p, true);
                                 destroyedByBorrowValidationCount.incrementAndGet();
-                            } catch (Exception e) {
+                            } catch (final Exception e) {
                                 // Ignore - validation failure is more important
                             }
                             p = null;
                             if (create) {
-                                NoSuchElementException nsee = new NoSuchElementException(
+                                final NoSuchElementException nsee = new NoSuchElementException(
                                         "Unable to validate object");
                                 nsee.initCause(validationThrowable);
                                 throw nsee;
@@ -464,11 +454,11 @@ public class GenericKeyedObjectPool<K,T>
      *                               returned to the pool multiple times
      */
     @Override
-    public void returnObject(K key, T obj) {
+    public void returnObject(final K key, final T obj) {
 
-        ObjectDeque<T> objectDeque = poolMap.get(key);
+        final ObjectDeque<T> objectDeque = poolMap.get(key);
 
-        PooledObject<T> p = objectDeque.getAllObjects().get(new IdentityWrapper<>(obj));
+        final PooledObject<T> p = objectDeque.getAllObjects().get(new IdentityWrapper<>(obj));
 
         if (p == null) {
             throw new IllegalStateException(
@@ -484,81 +474,80 @@ public class GenericKeyedObjectPool<K,T>
             p.markReturning(); // Keep from being marked abandoned (once GKOP does this)
         }
 
-        long activeTime = p.getActiveTimeMillis();
+        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 (Exception e) {
+                } catch (final Exception e) {
                     swallowException(e);
                 }
                 if (objectDeque.idleObjects.hasTakeWaiters()) {
                     try {
                         addObject(key);
-                    } catch (Exception e) {
+                    } catch (final Exception e) {
                         swallowException(e);
                     }
                 }
-                updateStatsReturn(activeTime);
                 return;
             }
-        }
 
-        try {
-            factory.passivateObject(key, p);
-        } catch (Exception e1) {
-            swallowException(e1);
-            try {
-                destroy(key, p, true);
-            } catch (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);
-                } catch (Exception e) {
+                    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");
-        }
-
-        int maxIdle = getMaxIdlePerKey();
-        LinkedBlockingDeque<PooledObject<T>> idleObjects =
-            objectDeque.getIdleObjects();
-
-        if (isClosed() || maxIdle > -1 && maxIdle <= idleObjects.size()) {
-            try {
-                destroy(key, p, true);
-            } catch (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);
     }
 
 
@@ -577,11 +566,11 @@ public class GenericKeyedObjectPool<K,T>
      *                               under the given key
      */
     @Override
-    public void invalidateObject(K key, T obj) throws Exception {
+    public void invalidateObject(final K key, final T obj) throws Exception {
 
-        ObjectDeque<T> objectDeque = poolMap.get(key);
+        final ObjectDeque<T> objectDeque = poolMap.get(key);
 
-        PooledObject<T> p = objectDeque.getAllObjects().get(new IdentityWrapper<>(obj));
+        final PooledObject<T> p = objectDeque.getAllObjects().get(new IdentityWrapper<>(obj));
         if (p == null) {
             throw new IllegalStateException(
                     "Object not currently part of this pool");
@@ -618,7 +607,7 @@ public class GenericKeyedObjectPool<K,T>
      */
     @Override
     public void clear() {
-        Iterator<K> iter = poolMap.keySet().iterator();
+        final Iterator<K> iter = poolMap.keySet().iterator();
 
         while (iter.hasNext()) {
             clear(iter.next());
@@ -635,12 +624,12 @@ public class GenericKeyedObjectPool<K,T>
      * @param key the key to clear
      */
     @Override
-    public void clear(K key) {
+    public void clear(final K key) {
 
-        ObjectDeque<T> objectDeque = register(key);
+        final ObjectDeque<T> objectDeque = register(key);
 
         try {
-            LinkedBlockingDeque<PooledObject<T>> idleObjects =
+            final LinkedBlockingDeque<PooledObject<T>> idleObjects =
                     objectDeque.getIdleObjects();
 
             PooledObject<T> p = idleObjects.poll();
@@ -648,7 +637,7 @@ public class GenericKeyedObjectPool<K,T>
             while (p != null) {
                 try {
                     destroy(key, p, true);
-                } catch (Exception e) {
+                } catch (final Exception e) {
                     swallowException(e);
                 }
                 p = idleObjects.poll();
@@ -667,7 +656,7 @@ public class GenericKeyedObjectPool<K,T>
 
     @Override
     public int getNumIdle() {
-        Iterator<ObjectDeque<T>> iter = poolMap.values().iterator();
+        final Iterator<ObjectDeque<T>> iter = poolMap.values().iterator();
         int result = 0;
 
         while (iter.hasNext()) {
@@ -679,7 +668,7 @@ public class GenericKeyedObjectPool<K,T>
 
 
     @Override
-    public int getNumActive(K key) {
+    public int getNumActive(final K key) {
         final ObjectDeque<T> objectDeque = poolMap.get(key);
         if (objectDeque != null) {
             return objectDeque.getAllObjects().size() -
@@ -690,7 +679,7 @@ public class GenericKeyedObjectPool<K,T>
 
 
     @Override
-    public int getNumIdle(K key) {
+    public int getNumIdle(final K key) {
         final ObjectDeque<T> objectDeque = poolMap.get(key);
         return objectDeque != null ? objectDeque.getIdleObjects().size() : 0;
     }
@@ -727,7 +716,7 @@ public class GenericKeyedObjectPool<K,T>
             jmxUnregister();
 
             // Release any threads that were waiting for an object
-            Iterator<ObjectDeque<T>> iter = poolMap.values().iterator();
+            final Iterator<ObjectDeque<T>> iter = poolMap.values().iterator();
             while (iter.hasNext()) {
                 iter.next().getIdleObjects().interuptTakeWaiters();
             }
@@ -747,14 +736,15 @@ public class GenericKeyedObjectPool<K,T>
         // build sorted map of idle objects
         final Map<PooledObject<T>, K> map = new TreeMap<>();
 
-        for (K k : poolMap.keySet()) {
-            ObjectDeque<T> queue = poolMap.get(k);
+        for (Map.Entry<K, ObjectDeque<T>> entry : poolMap.entrySet()) {
+            final K k = entry.getKey();
+            final ObjectDeque<T> deque = entry.getValue();
             // Protect against possible NPE if key has been removed in another
             // thread. Not worth locking the keys while this loop completes.
-            if (queue != null) {
+            if (deque != null) {
                 final LinkedBlockingDeque<PooledObject<T>> idleObjects =
-                    queue.getIdleObjects();
-                for (PooledObject<T> p : idleObjects) {
+                        deque.getIdleObjects();
+                for (final PooledObject<T> p : idleObjects) {
                     // each item into the map using the PooledObject object as the
                     // key. It then gets sorted based on the idle time
                     map.put(p, k);
@@ -765,22 +755,22 @@ public class GenericKeyedObjectPool<K,T>
         // Now iterate created map and kill the first 15% plus one to account
         // for zero
         int itemsToRemove = ((int) (map.size() * 0.15)) + 1;
-        Iterator<Map.Entry<PooledObject<T>, K>> iter =
+        final Iterator<Map.Entry<PooledObject<T>, K>> iter =
             map.entrySet().iterator();
 
         while (iter.hasNext() && itemsToRemove > 0) {
-            Map.Entry<PooledObject<T>, K> entry = iter.next();
+            final Map.Entry<PooledObject<T>, K> entry = iter.next();
             // kind of backwards on naming.  In the map, each key is the
             // PooledObject because it has the ordering with the timestamp
             // value.  Each value that the key references is the key of the
             // list it belongs to.
-            K key = entry.getValue();
-            PooledObject<T> p = entry.getKey();
+            final K key = entry.getValue();
+            final PooledObject<T> p = entry.getKey();
             // Assume the destruction succeeds
             boolean destroyed = true;
             try {
                 destroyed = destroy(key, p, false);
-            } catch (Exception e) {
+            } catch (final Exception e) {
                 swallowException(e);
             }
             if (destroyed) {
@@ -809,8 +799,9 @@ public class GenericKeyedObjectPool<K,T>
         int maxQueueLength = 0;
         LinkedBlockingDeque<PooledObject<T>> mostLoaded = null;
         K loadedKey = null;
-        for (K k : poolMap.keySet()) {
-            final ObjectDeque<T> deque = poolMap.get(k);
+        for (Map.Entry<K, ObjectDeque<T>> entry : poolMap.entrySet()) {
+            final K k = entry.getKey();
+            final ObjectDeque<T> deque = entry.getValue();
             if (deque != null) {
                 final LinkedBlockingDeque<PooledObject<T>> pool = deque.getIdleObjects();
                 final int queueLength = pool.getTakeQueueLength();
@@ -826,11 +817,11 @@ public class GenericKeyedObjectPool<K,T>
         if (mostLoaded != null) {
             register(loadedKey);
             try {
-                PooledObject<T> p = create(loadedKey);
+                final PooledObject<T> p = create(loadedKey);
                 if (p != null) {
                     addIdleObject(loadedKey, p);
                 }
-            } catch (Exception e) {
+            } catch (final Exception e) {
                 swallowException(e);
             } finally {
                 deregister(loadedKey);
@@ -846,8 +837,8 @@ public class GenericKeyedObjectPool<K,T>
      *         {@code false}
      */
     private boolean hasBorrowWaiters() {
-        for (K k : poolMap.keySet()) {
-            final ObjectDeque<T> deque = poolMap.get(k);
+        for (Map.Entry<K, ObjectDeque<T>> entry : poolMap.entrySet()) {
+            final ObjectDeque<T> deque = entry.getValue();
             if (deque != null) {
                 final LinkedBlockingDeque<PooledObject<T>> pool =
                     deque.getIdleObjects();
@@ -876,22 +867,22 @@ public class GenericKeyedObjectPool<K,T>
         }
 
         PooledObject<T> underTest = null;
-        EvictionPolicy<T> evictionPolicy = getEvictionPolicy();
+        final EvictionPolicy<T> evictionPolicy = getEvictionPolicy();
 
         synchronized (evictionLock) {
-            EvictionConfig evictionConfig = new EvictionConfig(
+            final EvictionConfig evictionConfig = new EvictionConfig(
                     getMinEvictableIdleTimeMillis(),
                     getSoftMinEvictableIdleTimeMillis(),
                     getMinIdlePerKey());
 
-            boolean testWhileIdle = getTestWhileIdle();
+            final boolean testWhileIdle = getTestWhileIdle();
 
             for (int i = 0, m = getNumTests(); i < m; i++) {
                 if(evictionIterator == null || !evictionIterator.hasNext()) {
                     if (evictionKeyIterator == null ||
                             !evictionKeyIterator.hasNext()) {
-                        List<K> keyCopy = new ArrayList<>();
-                        Lock readLock = keyLock.readLock();
+                        final List<K> keyCopy = new ArrayList<>();
+                        final Lock readLock = keyLock.readLock();
                         readLock.lock();
                         try {
                             keyCopy.addAll(poolKeyList);
@@ -902,7 +893,7 @@ public class GenericKeyedObjectPool<K,T>
                     }
                     while (evictionKeyIterator.hasNext()) {
                         evictionKey = evictionKeyIterator.next();
-                        ObjectDeque<T> objectDeque = poolMap.get(evictionKey);
+                        final ObjectDeque<T> objectDeque = poolMap.get(evictionKey);
                         if (objectDeque == null) {
                             continue;
                         }
@@ -923,7 +914,7 @@ public class GenericKeyedObjectPool<K,T>
                 try {
                     underTest = evictionIterator.next();
                     idleObjects = evictionIterator.getIdleObjects();
-                } catch (NoSuchElementException nsee) {
+                } catch (final NoSuchElementException nsee) {
                     // Object was borrowed in another thread
                     // Don't count this as an eviction test so reduce i;
                     i--;
@@ -945,7 +936,7 @@ public class GenericKeyedObjectPool<K,T>
                 try {
                     evict = evictionPolicy.evict(evictionConfig, underTest,
                             poolMap.get(evictionKey).getIdleObjects().size());
-                } catch (Throwable t) {
+                } catch (final Throwable t) {
                     // Slightly convoluted as SwallowedExceptionListener uses
                     // Exception rather than Throwable
                     PoolUtils.checkRethrow(t);
@@ -963,7 +954,7 @@ public class GenericKeyedObjectPool<K,T>
                         try {
                             factory.activateObject(evictionKey, underTest);
                             active = true;
-                        } catch (Exception e) {
+                        } catch (final Exception e) {
                             destroy(evictionKey, underTest, true);
                             destroyedByEvictorCount.incrementAndGet();
                         }
@@ -974,7 +965,7 @@ public class GenericKeyedObjectPool<K,T>
                             } else {
                                 try {
                                     factory.passivateObject(evictionKey, underTest);
-                                } catch (Exception e) {
+                                } catch (final Exception e) {
                                     destroy(evictionKey, underTest, true);
                                     destroyedByEvictorCount.incrementAndGet();
                                 }
@@ -999,15 +990,20 @@ public class GenericKeyedObjectPool<K,T>
      *
      * @throws Exception If the objection creation fails
      */
-    private PooledObject<T> create(K key) throws Exception {
+    private PooledObject<T> create(final K key) throws Exception {
         int maxTotalPerKeySave = getMaxTotalPerKey(); // Per key
-        int maxTotal = getMaxTotal();   // All keys
+        if (maxTotalPerKeySave < 0) {
+            maxTotalPerKeySave = Integer.MAX_VALUE;
+        }
+        final int maxTotal = getMaxTotal();   // All keys
+
+        final ObjectDeque<T> objectDeque = poolMap.get(key);
 
         // Check against the overall limit
         boolean loop = true;
 
         while (loop) {
-            int newNumTotal = numTotal.incrementAndGet();
+            final int newNumTotal = numTotal.incrementAndGet();
             if (maxTotal > -1 && newNumTotal > maxTotal) {
                 numTotal.decrementAndGet();
                 if (getNumIdle() == 0) {
@@ -1019,25 +1015,58 @@ public class GenericKeyedObjectPool<K,T>
             }
         }
 
-        ObjectDeque<T> objectDeque = poolMap.get(key);
-        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.
+                    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 (maxTotalPerKeySave > -1 && newCreateCount > maxTotalPerKeySave ||
-                newCreateCount > Integer.MAX_VALUE) {
+        if (!create.booleanValue()) {
             numTotal.decrementAndGet();
-            objectDeque.getCreateCount().decrementAndGet();
             return null;
         }
 
-
         PooledObject<T> p = null;
         try {
             p = factory.makeObject(key);
-        } catch (Exception e) {
+        } catch (final Exception e) {
             numTotal.decrementAndGet();
             objectDeque.getCreateCount().decrementAndGet();
             throw e;
+        } finally {
+            synchronized (objectDeque.makeObjectCountLock) {
+                objectDeque.makeObjectCount--;
+                objectDeque.makeObjectCountLock.notifyAll();
+            }
         }
 
         createdCount.incrementAndGet();
@@ -1055,13 +1084,13 @@ public class GenericKeyedObjectPool<K,T>
      * @return {@code true} if the object was destroyed, otherwise {@code false}
      * @throws Exception If the object destruction failed
      */
-    private boolean destroy(K key, PooledObject<T> toDestroy, boolean always)
+    private boolean destroy(final K key, final PooledObject<T> toDestroy, final boolean always)
             throws Exception {
 
-        ObjectDeque<T> objectDeque = register(key);
+        final ObjectDeque<T> objectDeque = register(key);
 
         try {
-            boolean isIdle = objectDeque.getIdleObjects().remove(toDestroy);
+            final boolean isIdle = objectDeque.getIdleObjects().remove(toDestroy);
 
             if (isIdle || always) {
                 objectDeque.getAllObjects().remove(new IdentityWrapper<>(toDestroy.getObject()));
@@ -1094,7 +1123,7 @@ public class GenericKeyedObjectPool<K,T>
      *         method returns without throwing an exception then it will never
      *         return null.
      */
-    private ObjectDeque<T> register(K k) {
+    private ObjectDeque<T> register(final K k) {
         Lock lock = keyLock.readLock();
         ObjectDeque<T> objectDeque = null;
         try {
@@ -1133,14 +1162,14 @@ public class GenericKeyedObjectPool<K,T>
      *
      * @param k The key to de-register
      */
-    private void deregister(K k) {
+    private void deregister(final K k) {
         ObjectDeque<T> objectDeque;
 
         objectDeque = poolMap.get(k);
-        long numInterested = objectDeque.getNumInterested().decrementAndGet();
+        final long numInterested = objectDeque.getNumInterested().decrementAndGet();
         if (numInterested == 0 && objectDeque.getCreateCount().get() == 0) {
             // Potential to remove key
-            Lock writeLock = keyLock.writeLock();
+            final Lock writeLock = keyLock.writeLock();
             writeLock.lock();
             try {
                 if (objectDeque.getCreateCount().get() == 0 &&
@@ -1159,12 +1188,12 @@ public class GenericKeyedObjectPool<K,T>
 
     @Override
     void ensureMinIdle() throws Exception {
-        int minIdlePerKeySave = getMinIdlePerKey();
+        final int minIdlePerKeySave = getMinIdlePerKey();
         if (minIdlePerKeySave < 1) {
             return;
         }
 
-        for (K k : poolMap.keySet()) {
+        for (final K k : poolMap.keySet()) {
             ensureMinIdle(k);
         }
     }
@@ -1177,9 +1206,9 @@ public class GenericKeyedObjectPool<K,T>
      *
      * @throws Exception If a new object is required and cannot be created
      */
-    private void ensureMinIdle(K key) throws Exception {
+    private void ensureMinIdle(final K key) throws Exception {
         // Calculate current pool objects
-        ObjectDeque<T> objectDeque = poolMap.get(key);
+        final ObjectDeque<T> objectDeque = poolMap.get(key);
 
         // objectDeque == null is OK here. It is handled correctly by both
         // methods called below.
@@ -1189,7 +1218,7 @@ public class GenericKeyedObjectPool<K,T>
         // as a loop limit and a second time inside the loop
         // to stop when another thread already returned the
         // needed objects
-        int deficit = calculateDeficit(objectDeque);
+        final int deficit = calculateDeficit(objectDeque);
 
         for (int i = 0; i < deficit && calculateDeficit(objectDeque) > 0; i++) {
             addObject(key);
@@ -1208,11 +1237,11 @@ public class GenericKeyedObjectPool<K,T>
      *                   fails.
      */
     @Override
-    public void addObject(K key) throws Exception {
+    public void addObject(final K key) throws Exception {
         assertOpen();
         register(key);
         try {
-            PooledObject<T> p = create(key);
+            final PooledObject<T> p = create(key);
             addIdleObject(key, p);
         } finally {
             deregister(key);
@@ -1227,11 +1256,11 @@ public class GenericKeyedObjectPool<K,T>
      *
      * @throws Exception If the associated factory fails to passivate the object
      */
-    private void addIdleObject(K key, PooledObject<T> p) throws Exception {
+    private void addIdleObject(final K key, final PooledObject<T> p) throws Exception {
 
         if (p != null) {
             factory.passivateObject(key, p);
-            LinkedBlockingDeque<PooledObject<T>> idleObjects =
+            final LinkedBlockingDeque<PooledObject<T>> idleObjects =
                     poolMap.get(key).getIdleObjects();
             if (getLifo()) {
                 idleObjects.addFirst(p);
@@ -1249,8 +1278,8 @@ public class GenericKeyedObjectPool<K,T>
      *
      * @throws Exception If the associated factory throws an exception
      */
-    public void preparePool(K key) throws Exception {
-        int minIdlePerKeySave = getMinIdlePerKey();
+    public void preparePool(final K key) throws Exception {
+        final int minIdlePerKeySave = getMinIdlePerKey();
         if (minIdlePerKeySave < 1) {
             return;
         }
@@ -1264,8 +1293,8 @@ public class GenericKeyedObjectPool<K,T>
      * @return The number of objects to test for validity
      */
     private int getNumTests() {
-        int totalIdle = getNumIdle();
-        int numTests = getNumTestsPerEvictionRun();
+        final int totalIdle = getNumIdle();
+        final int numTests = getNumTestsPerEvictionRun();
         if (numTests >= 0) {
             return Math.min(numTests, totalIdle);
         }
@@ -1281,15 +1310,15 @@ public class GenericKeyedObjectPool<K,T>
      *
      * @return The number of new objects to create
      */
-    private int calculateDeficit(ObjectDeque<T> objectDeque) {
+    private int calculateDeficit(final ObjectDeque<T> objectDeque) {
 
         if (objectDeque == null) {
             return getMinIdlePerKey();
         }
 
         // Used more than once so keep a local copy so the value is consistent
-        int maxTotal = getMaxTotal();
-        int maxTotalPerKeySave = getMaxTotalPerKey();
+        final int maxTotal = getMaxTotal();
+        final int maxTotalPerKeySave = getMaxTotalPerKey();
 
         int objectDefecit = 0;
 
@@ -1297,14 +1326,14 @@ public class GenericKeyedObjectPool<K,T>
         // the number of pooled objects < maxTotalPerKey();
         objectDefecit = getMinIdlePerKey() - objectDeque.getIdleObjects().size();
         if (maxTotalPerKeySave > 0) {
-            int growLimit = Math.max(0,
+            final int growLimit = Math.max(0,
                     maxTotalPerKeySave - objectDeque.getIdleObjects().size());
             objectDefecit = Math.min(objectDefecit, growLimit);
         }
 
         // Take the maxTotal limit into account
         if (maxTotal > 0) {
-            int growLimit = Math.max(0, maxTotal - getNumActive() - getNumIdle());
+            final int growLimit = Math.max(0, maxTotal - getNumActive() - getNumIdle());
             objectDefecit = Math.min(objectDefecit, growLimit);
         }
 
@@ -1316,14 +1345,14 @@ public class GenericKeyedObjectPool<K,T>
 
     @Override
     public Map<String,Integer> getNumActivePerKey() {
-        HashMap<String,Integer> result = new HashMap<>();
+        final HashMap<String,Integer> result = new HashMap<>();
 
-        Iterator<Entry<K,ObjectDeque<T>>> iter = poolMap.entrySet().iterator();
+        final Iterator<Entry<K,ObjectDeque<T>>> iter = poolMap.entrySet().iterator();
         while (iter.hasNext()) {
-            Entry<K,ObjectDeque<T>> entry = iter.next();
+            final Entry<K,ObjectDeque<T>> entry = iter.next();
             if (entry != null) {
-                K key = entry.getKey();
-                ObjectDeque<T> objectDequeue = entry.getValue();
+                final K key = entry.getKey();
+                final ObjectDeque<T> objectDequeue = entry.getValue();
                 if (key != null && objectDequeue != null) {
                     result.put(key.toString(), Integer.valueOf(
                             objectDequeue.getAllObjects().size() -
@@ -1347,7 +1376,7 @@ public class GenericKeyedObjectPool<K,T>
         int result = 0;
 
         if (getBlockWhenExhausted()) {
-            Iterator<ObjectDeque<T>> iter = poolMap.values().iterator();
+            final Iterator<ObjectDeque<T>> iter = poolMap.values().iterator();
 
             while (iter.hasNext()) {
                 // Assume no overflow
@@ -1368,16 +1397,17 @@ public class GenericKeyedObjectPool<K,T>
      */
     @Override
     public Map<String,Integer> getNumWaitersByKey() {
-        Map<String,Integer> result = new HashMap<>();
+        final Map<String,Integer> result = new HashMap<>();
 
-        for (K key : poolMap.keySet()) {
-            ObjectDeque<T> queue = poolMap.get(key);
-            if (queue != null) {
+        for (Map.Entry<K, ObjectDeque<T>> entry : poolMap.entrySet()) {
+            final K k = entry.getKey();
+            final ObjectDeque<T> deque = entry.getValue();
+            if (deque != null) {
                 if (getBlockWhenExhausted()) {
-                    result.put(key.toString(), Integer.valueOf(
-                            queue.getIdleObjects().getTakeQueueLength()));
+                    result.put(k.toString(), Integer.valueOf(
+                            deque.getIdleObjects().getTakeQueueLength()));
                 } else {
-                    result.put(key.toString(), Integer.valueOf(0));
+                    result.put(k.toString(), Integer.valueOf(0));
                 }
             }
         }
@@ -1397,16 +1427,17 @@ public class GenericKeyedObjectPool<K,T>
      */
     @Override
     public Map<String,List<DefaultPooledObjectInfo>> listAllObjects() {
-        Map<String,List<DefaultPooledObjectInfo>> result =
+        final Map<String,List<DefaultPooledObjectInfo>> result =
                 new HashMap<>();
 
-        for (K key : poolMap.keySet()) {
-            ObjectDeque<T> queue = poolMap.get(key);
-            if (queue != null) {
-                List<DefaultPooledObjectInfo> list =
+        for (Map.Entry<K, ObjectDeque<T>> entry : poolMap.entrySet()) {
+            final K k = entry.getKey();
+            final ObjectDeque<T> deque = entry.getValue();
+            if (deque != null) {
+                final List<DefaultPooledObjectInfo> list =
                         new ArrayList<>();
-                result.put(key.toString(), list);
-                for (PooledObject<T> p : queue.getAllObjects().values()) {
+                result.put(k.toString(), list);
+                for (final PooledObject<T> p : deque.getAllObjects().values()) {
                     list.add(new DefaultPooledObjectInfo(p));
                 }
             }
@@ -1430,6 +1461,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.
@@ -1450,7 +1484,7 @@ public class GenericKeyedObjectPool<K,T>
          * @param fairness true means client threads waiting to borrow / return instances
          * will be served as if waiting in a FIFO queue.
          */
-        public ObjectDeque(boolean fairness) {
+        public ObjectDeque(final boolean fairness) {
             idleObjects = new LinkedBlockingDeque<>(fairness);
         }
 
@@ -1493,7 +1527,7 @@ public class GenericKeyedObjectPool<K,T>
 
         @Override
         public String toString() {
-            StringBuilder builder = new StringBuilder();
+            final StringBuilder builder = new StringBuilder();
             builder.append("ObjectDeque [idleObjects=");
             builder.append(idleObjects);
             builder.append(", createCount=");
@@ -1551,7 +1585,7 @@ public class GenericKeyedObjectPool<K,T>
         "org.apache.tomcat.dbcp.pool2:type=GenericKeyedObjectPool,name=";
 
     @Override
-    protected void toStringAppendFields(StringBuilder builder) {
+    protected void toStringAppendFields(final StringBuilder builder) {
         super.toStringAppendFields(builder);
         builder.append(", maxIdlePerKey=");
         builder.append(maxIdlePerKey);

Modified: tomcat/tc8.5.x/trunk/java/org/apache/tomcat/dbcp/pool2/impl/GenericKeyedObjectPoolConfig.java
URL: http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/java/org/apache/tomcat/dbcp/pool2/impl/GenericKeyedObjectPoolConfig.java?rev=1743721&r1=1743720&r2=1743721&view=diff
==============================================================================
--- tomcat/tc8.5.x/trunk/java/org/apache/tomcat/dbcp/pool2/impl/GenericKeyedObjectPoolConfig.java (original)
+++ tomcat/tc8.5.x/trunk/java/org/apache/tomcat/dbcp/pool2/impl/GenericKeyedObjectPoolConfig.java Fri May 13 18:50:08 2016
@@ -89,7 +89,7 @@ public class GenericKeyedObjectPoolConfi
      *
      * @see GenericKeyedObjectPool#setMaxTotal(int)
      */
-    public void setMaxTotal(int maxTotal) {
+    public void setMaxTotal(final int maxTotal) {
         this.maxTotal = maxTotal;
     }
 
@@ -115,7 +115,7 @@ public class GenericKeyedObjectPoolConfi
      *
      * @see GenericKeyedObjectPool#setMaxTotalPerKey(int)
      */
-    public void setMaxTotalPerKey(int maxTotalPerKey) {
+    public void setMaxTotalPerKey(final int maxTotalPerKey) {
         this.maxTotalPerKey = maxTotalPerKey;
     }
 
@@ -141,7 +141,7 @@ public class GenericKeyedObjectPoolConfi
      *
      * @see GenericKeyedObjectPool#setMinIdlePerKey(int)
      */
-    public void setMinIdlePerKey(int minIdlePerKey) {
+    public void setMinIdlePerKey(final int minIdlePerKey) {
         this.minIdlePerKey = minIdlePerKey;
     }
 
@@ -167,7 +167,7 @@ public class GenericKeyedObjectPoolConfi
      *
      * @see GenericKeyedObjectPool#setMaxIdlePerKey(int)
      */
-    public void setMaxIdlePerKey(int maxIdlePerKey) {
+    public void setMaxIdlePerKey(final int maxIdlePerKey) {
         this.maxIdlePerKey = maxIdlePerKey;
     }
 
@@ -175,13 +175,13 @@ public class GenericKeyedObjectPoolConfi
     public GenericKeyedObjectPoolConfig clone() {
         try {
             return (GenericKeyedObjectPoolConfig) super.clone();
-        } catch (CloneNotSupportedException e) {
+        } catch (final CloneNotSupportedException e) {
             throw new AssertionError(); // Can't happen
         }
     }
 
     @Override
-    protected void toStringAppendFields(StringBuilder builder) {
+    protected void toStringAppendFields(final StringBuilder builder) {
         super.toStringAppendFields(builder);
         builder.append(", minIdlePerKey=");
         builder.append(minIdlePerKey);

Modified: tomcat/tc8.5.x/trunk/java/org/apache/tomcat/dbcp/pool2/impl/GenericObjectPool.java
URL: http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/java/org/apache/tomcat/dbcp/pool2/impl/GenericObjectPool.java?rev=1743721&r1=1743720&r2=1743721&view=diff
==============================================================================
--- tomcat/tc8.5.x/trunk/java/org/apache/tomcat/dbcp/pool2/impl/GenericObjectPool.java (original)
+++ tomcat/tc8.5.x/trunk/java/org/apache/tomcat/dbcp/pool2/impl/GenericObjectPool.java Fri May 13 18:50:08 2016
@@ -83,7 +83,7 @@ public class GenericObjectPool<T> extend
      * @param factory The object factory to be used to create object instances
      *                used by this pool
      */
-    public GenericObjectPool(PooledObjectFactory<T> factory) {
+    public GenericObjectPool(final PooledObjectFactory<T> factory) {
         this(factory, new GenericObjectPoolConfig());
     }
 
@@ -98,8 +98,8 @@ public class GenericObjectPool<T> extend
      *                  the configuration object will not be reflected in the
      *                  pool.
      */
-    public GenericObjectPool(PooledObjectFactory<T> factory,
-            GenericObjectPoolConfig config) {
+    public GenericObjectPool(final PooledObjectFactory<T> factory,
+            final GenericObjectPoolConfig config) {
 
         super(config, ONAME_BASE, config.getJmxNamePrefix());
 
@@ -129,8 +129,8 @@ public class GenericObjectPool<T> extend
      * @param abandonedConfig  Configuration for abandoned object identification
      *                         and removal.  The configuration is used by value.
      */
-    public GenericObjectPool(PooledObjectFactory<T> factory,
-            GenericObjectPoolConfig config, AbandonedConfig abandonedConfig) {
+    public GenericObjectPool(final PooledObjectFactory<T> factory,
+            final GenericObjectPoolConfig config, final AbandonedConfig abandonedConfig) {
         this(factory, config);
         setAbandonedConfig(abandonedConfig);
     }
@@ -170,7 +170,7 @@ public class GenericObjectPool<T> extend
      *
      * @see #getMaxIdle
      */
-    public void setMaxIdle(int maxIdle) {
+    public void setMaxIdle(final int maxIdle) {
         this.maxIdle = maxIdle;
     }
 
@@ -191,7 +191,7 @@ public class GenericObjectPool<T> extend
      * @see #getMaxIdle()
      * @see #getTimeBetweenEvictionRunsMillis()
      */
-    public void setMinIdle(int minIdle) {
+    public void setMinIdle(final int minIdle) {
         this.minIdle = minIdle;
     }
 
@@ -213,7 +213,7 @@ public class GenericObjectPool<T> extend
      */
     @Override
     public int getMinIdle() {
-        int maxIdleSave = getMaxIdle();
+        final int maxIdleSave = getMaxIdle();
         if (this.minIdle > maxIdleSave) {
             return maxIdleSave;
         }
@@ -241,7 +241,7 @@ public class GenericObjectPool<T> extend
      */
     @Override
     public boolean getLogAbandoned() {
-        AbandonedConfig ac = this.abandonedConfig;
+        final AbandonedConfig ac = this.abandonedConfig;
         return ac != null && ac.getLogAbandoned();
     }
 
@@ -256,7 +256,7 @@ public class GenericObjectPool<T> extend
      */
     @Override
     public boolean getRemoveAbandonedOnBorrow() {
-        AbandonedConfig ac = this.abandonedConfig;
+        final AbandonedConfig ac = this.abandonedConfig;
         return ac != null && ac.getRemoveAbandonedOnBorrow();
     }
 
@@ -270,7 +270,7 @@ public class GenericObjectPool<T> extend
      */
     @Override
     public boolean getRemoveAbandonedOnMaintenance() {
-        AbandonedConfig ac = this.abandonedConfig;
+        final AbandonedConfig ac = this.abandonedConfig;
         return ac != null && ac.getRemoveAbandonedOnMaintenance();
     }
 
@@ -285,7 +285,7 @@ public class GenericObjectPool<T> extend
      */
     @Override
     public int getRemoveAbandonedTimeout() {
-        AbandonedConfig ac = this.abandonedConfig;
+        final AbandonedConfig ac = this.abandonedConfig;
         return ac != null ? ac.getRemoveAbandonedTimeout() : Integer.MAX_VALUE;
     }
 
@@ -297,7 +297,7 @@ public class GenericObjectPool<T> extend
      *
      * @see GenericObjectPoolConfig
      */
-    public void setConfig(GenericObjectPoolConfig conf) {
+    public void setConfig(final GenericObjectPoolConfig conf) {
         setLifo(conf.getLifo());
         setMaxIdle(conf.getMaxIdle());
         setMinIdle(conf.getMinIdle());
@@ -324,7 +324,7 @@ public class GenericObjectPool<T> extend
      *
      * @see AbandonedConfig
      */
-    public void setAbandonedConfig(AbandonedConfig abandonedConfig) {
+    public void setAbandonedConfig(final AbandonedConfig abandonedConfig) {
         if (abandonedConfig == null) {
             this.abandonedConfig = null;
         } else {
@@ -404,10 +404,10 @@ public class GenericObjectPool<T> extend
      * @throws Exception if an object instance cannot be returned due to an
      *                   error
      */
-    public T borrowObject(long borrowMaxWaitMillis) throws Exception {
+    public T borrowObject(final long borrowMaxWaitMillis) throws Exception {
         assertOpen();
 
-        AbandonedConfig ac = this.abandonedConfig;
+        final AbandonedConfig ac = this.abandonedConfig;
         if (ac != null && ac.getRemoveAbandonedOnBorrow() &&
                 (getNumIdle() < 2) &&
                 (getNumActive() > getMaxTotal() - 3) ) {
@@ -418,21 +418,21 @@ public class GenericObjectPool<T> extend
 
         // Get local copy of current config so it is consistent for entire
         // method execution
-        boolean blockWhenExhausted = getBlockWhenExhausted();
+        final boolean blockWhenExhausted = getBlockWhenExhausted();
 
         boolean create;
-        long waitTime = System.currentTimeMillis();
+        final long waitTime = System.currentTimeMillis();
 
         while (p == null) {
             create = false;
-            if (blockWhenExhausted) {
-                p = idleObjects.pollFirst();
-                if (p == null) {
-                    p = create();
-                    if (p != null) {
-                        create = true;
-                    }
+            p = idleObjects.pollFirst();
+            if (p == null) {
+                p = create();
+                if (p != null) {
+                    create = true;
                 }
+            }
+            if (blockWhenExhausted) {
                 if (p == null) {
                     if (borrowMaxWaitMillis < 0) {
                         p = idleObjects.takeFirst();
@@ -445,37 +445,27 @@ public class GenericObjectPool<T> extend
                     throw new NoSuchElementException(
                             "Timeout waiting for idle object");
                 }
-                if (!p.allocate()) {
-                    p = null;
-                }
             } else {
-                p = idleObjects.pollFirst();
-                if (p == null) {
-                    p = create();
-                    if (p != null) {
-                        create = true;
-                    }
-                }
                 if (p == null) {
                     throw new NoSuchElementException("Pool exhausted");
                 }
-                if (!p.allocate()) {
-                    p = null;
-                }
+            }
+            if (!p.allocate()) {
+                p = null;
             }
 
             if (p != null) {
                 try {
                     factory.activateObject(p);
-                } catch (Exception e) {
+                } catch (final Exception e) {
                     try {
                         destroy(p);
-                    } catch (Exception e1) {
+                    } catch (final Exception e1) {
                         // Ignore - activation failure is more important
                     }
                     p = null;
                     if (create) {
-                        NoSuchElementException nsee = new NoSuchElementException(
+                        final NoSuchElementException nsee = new NoSuchElementException(
                                 "Unable to activate object");
                         nsee.initCause(e);
                         throw nsee;
@@ -486,7 +476,7 @@ public class GenericObjectPool<T> extend
                     Throwable validationThrowable = null;
                     try {
                         validate = factory.validateObject(p);
-                    } catch (Throwable t) {
+                    } catch (final Throwable t) {
                         PoolUtils.checkRethrow(t);
                         validationThrowable = t;
                     }
@@ -494,12 +484,12 @@ public class GenericObjectPool<T> extend
                         try {
                             destroy(p);
                             destroyedByBorrowValidationCount.incrementAndGet();
-                        } catch (Exception e) {
+                        } catch (final Exception e) {
                             // Ignore - validation failure is more important
                         }
                         p = null;
                         if (create) {
-                            NoSuchElementException nsee = new NoSuchElementException(
+                            final NoSuchElementException nsee = new NoSuchElementException(
                                     "Unable to validate object");
                             nsee.initCause(validationThrowable);
                             throw nsee;
@@ -530,8 +520,8 @@ public class GenericObjectPool<T> extend
      * {@link org.apache.tomcat.dbcp.pool2.SwallowedExceptionListener}.
      */
     @Override
-    public void returnObject(T obj) {
-        PooledObject<T> p = allObjects.get(new IdentityWrapper<>(obj));
+    public void returnObject(final T obj) {
+        final PooledObject<T> p = allObjects.get(new IdentityWrapper<>(obj));
 
         if (p == null) {
             if (!isAbandonedConfig()) {
@@ -550,18 +540,18 @@ public class GenericObjectPool<T> extend
             p.markReturning(); // Keep from being marked abandoned
         }
 
-        long activeTime = p.getActiveTimeMillis();
+        final long activeTime = p.getActiveTimeMillis();
 
         if (getTestOnReturn()) {
             if (!factory.validateObject(p)) {
                 try {
                     destroy(p);
-                } catch (Exception e) {
+                } catch (final Exception e) {
                     swallowException(e);
                 }
                 try {
                     ensureIdle(1, false);
-                } catch (Exception e) {
+                } catch (final Exception e) {
                     swallowException(e);
                 }
                 updateStatsReturn(activeTime);
@@ -571,16 +561,16 @@ public class GenericObjectPool<T> extend
 
         try {
             factory.passivateObject(p);
-        } catch (Exception e1) {
+        } catch (final Exception e1) {
             swallowException(e1);
             try {
                 destroy(p);
-            } catch (Exception e) {
+            } catch (final Exception e) {
                 swallowException(e);
             }
             try {
                 ensureIdle(1, false);
-            } catch (Exception e) {
+            } catch (final Exception e) {
                 swallowException(e);
             }
             updateStatsReturn(activeTime);
@@ -592,11 +582,11 @@ public class GenericObjectPool<T> extend
                     "Object has already been returned to this pool or is invalid");
         }
 
-        int maxIdleSave = getMaxIdle();
+        final int maxIdleSave = getMaxIdle();
         if (isClosed() || maxIdleSave > -1 && maxIdleSave <= idleObjects.size()) {
             try {
                 destroy(p);
-            } catch (Exception e) {
+            } catch (final Exception e) {
                 swallowException(e);
             }
         } else {
@@ -626,8 +616,8 @@ public class GenericObjectPool<T> extend
      * @throws IllegalStateException if obj does not belong to this pool
      */
     @Override
-    public void invalidateObject(T obj) throws Exception {
-        PooledObject<T> p = allObjects.get(new IdentityWrapper<>(obj));
+    public void invalidateObject(final T obj) throws Exception {
+        final PooledObject<T> p = allObjects.get(new IdentityWrapper<>(obj));
         if (p == null) {
             if (isAbandonedConfig()) {
                 return;
@@ -668,7 +658,7 @@ public class GenericObjectPool<T> extend
         while (p != null) {
             try {
                 destroy(p);
-            } catch (Exception e) {
+            } catch (final Exception e) {
                 swallowException(e);
             }
             p = idleObjects.poll();
@@ -732,15 +722,15 @@ public class GenericObjectPool<T> extend
         if (idleObjects.size() > 0) {
 
             PooledObject<T> underTest = null;
-            EvictionPolicy<T> evictionPolicy = getEvictionPolicy();
+            final EvictionPolicy<T> evictionPolicy = getEvictionPolicy();
 
             synchronized (evictionLock) {
-                EvictionConfig evictionConfig = new EvictionConfig(
+                final EvictionConfig evictionConfig = new EvictionConfig(
                         getMinEvictableIdleTimeMillis(),
                         getSoftMinEvictableIdleTimeMillis(),
                         getMinIdle());
 
-                boolean testWhileIdle = getTestWhileIdle();
+                final boolean testWhileIdle = getTestWhileIdle();
 
                 for (int i = 0, m = getNumTests(); i < m; i++) {
                     if (evictionIterator == null || !evictionIterator.hasNext()) {
@@ -753,7 +743,7 @@ public class GenericObjectPool<T> extend
 
                     try {
                         underTest = evictionIterator.next();
-                    } catch (NoSuchElementException nsee) {
+                    } catch (final NoSuchElementException nsee) {
                         // Object was borrowed in another thread
                         // Don't count this as an eviction test so reduce i;
                         i--;
@@ -775,7 +765,7 @@ public class GenericObjectPool<T> extend
                     try {
                         evict = evictionPolicy.evict(evictionConfig, underTest,
                                 idleObjects.size());
-                    } catch (Throwable t) {
+                    } catch (final Throwable t) {
                         // Slightly convoluted as SwallowedExceptionListener uses
                         // Exception rather than Throwable
                         PoolUtils.checkRethrow(t);
@@ -793,7 +783,7 @@ public class GenericObjectPool<T> extend
                             try {
                                 factory.activateObject(underTest);
                                 active = true;
-                            } catch (Exception e) {
+                            } catch (final Exception e) {
                                 destroy(underTest);
                                 destroyedByEvictorCount.incrementAndGet();
                             }
@@ -804,7 +794,7 @@ public class GenericObjectPool<T> extend
                                 } else {
                                     try {
                                         factory.passivateObject(underTest);
-                                    } catch (Exception e) {
+                                    } catch (final Exception e) {
                                         destroy(underTest);
                                         destroyedByEvictorCount.incrementAndGet();
                                     }
@@ -819,7 +809,7 @@ public class GenericObjectPool<T> extend
                 }
             }
         }
-        AbandonedConfig ac = this.abandonedConfig;
+        final AbandonedConfig ac = this.abandonedConfig;
         if (ac != null && ac.getRemoveAbandonedOnMaintenance()) {
             removeAbandoned(ac);
         }
@@ -851,10 +841,45 @@ public class GenericObjectPool<T> extend
      */
     private PooledObject<T> create() throws Exception {
         int localMaxTotal = getMaxTotal();
-        long newCreateCount = createCount.incrementAndGet();
-        if (localMaxTotal > -1 && newCreateCount > localMaxTotal ||
-                newCreateCount > Integer.MAX_VALUE) {
-            createCount.decrementAndGet();
+        // This simplifies the code later in this method
+        if (localMaxTotal < 0) {
+            localMaxTotal = Integer.MAX_VALUE;
+        }
+
+        // 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;
         }
 
@@ -864,9 +889,14 @@ public class GenericObjectPool<T> extend
         } catch (Exception e) {
             createCount.decrementAndGet();
             throw e;
+        } finally {
+            synchronized (makeObjectCountLock) {
+                makeObjectCount--;
+                makeObjectCountLock.notifyAll();
+            }
         }
 
-        AbandonedConfig ac = this.abandonedConfig;
+        final AbandonedConfig ac = this.abandonedConfig;
         if (ac != null && ac.getLogAbandoned()) {
             p.setLogAbandoned(true);
         }
@@ -879,17 +909,17 @@ public class GenericObjectPool<T> extend
     /**
      * Destroys a wrapped pooled object.
      *
-     * @param toDestory The wrapped pooled object to destroy
+     * @param toDestroy The wrapped pooled object to destroy
      *
      * @throws Exception If the factory fails to destroy the pooled object
      *                   cleanly
      */
-    private void destroy(PooledObject<T> toDestory) throws Exception {
-        toDestory.invalidate();
-        idleObjects.remove(toDestory);
-        allObjects.remove(new IdentityWrapper<>(toDestory.getObject()));
+    private void destroy(final PooledObject<T> toDestroy) throws Exception {
+        toDestroy.invalidate();
+        idleObjects.remove(toDestroy);
+        allObjects.remove(new IdentityWrapper<>(toDestroy.getObject()));
         try {
-            factory.destroyObject(toDestory);
+            factory.destroyObject(toDestroy);
         } finally {
             destroyedCount.incrementAndGet();
             createCount.decrementAndGet();
@@ -913,13 +943,13 @@ public class GenericObjectPool<T> extend
      * @param always true means create instances even if the pool has no threads waiting
      * @throws Exception if the factory's makeObject throws
      */
-    private void ensureIdle(int idleCount, boolean always) throws Exception {
+    private void ensureIdle(final int idleCount, final boolean always) throws Exception {
         if (idleCount < 1 || isClosed() || (!always && !idleObjects.hasTakeWaiters())) {
             return;
         }
 
         while (idleObjects.size() < idleCount) {
-            PooledObject<T> p = create();
+            final PooledObject<T> p = create();
             if (p == null) {
                 // Can't create objects, no reason to think another call to
                 // create will work. Give up.
@@ -953,7 +983,7 @@ public class GenericObjectPool<T> extend
             throw new IllegalStateException(
                     "Cannot add objects without a factory.");
         }
-        PooledObject<T> p = create();
+        final PooledObject<T> p = create();
         addIdleObject(p);
     }
 
@@ -966,7 +996,7 @@ public class GenericObjectPool<T> extend
      *
      * @throws Exception If the factory fails to passivate the object
      */
-    private void addIdleObject(PooledObject<T> p) throws Exception {
+    private void addIdleObject(final PooledObject<T> p) throws Exception {
         if (p != null) {
             factory.passivateObject(p);
             if (getLifo()) {
@@ -984,7 +1014,7 @@ public class GenericObjectPool<T> extend
      * @return The number of objects to test for validity
      */
     private int getNumTests() {
-        int numTestsPerEvictionRun = getNumTestsPerEvictionRun();
+        final int numTestsPerEvictionRun = getNumTestsPerEvictionRun();
         if (numTestsPerEvictionRun >= 0) {
             return Math.min(numTestsPerEvictionRun, idleObjects.size());
         }
@@ -998,15 +1028,15 @@ public class GenericObjectPool<T> extend
      *
      * @param ac The configuration to use to identify abandoned objects
      */
-    private void removeAbandoned(AbandonedConfig ac) {
+    private void removeAbandoned(final AbandonedConfig ac) {
         // Generate a list of abandoned objects to remove
         final long now = System.currentTimeMillis();
         final long timeout =
                 now - (ac.getRemoveAbandonedTimeout() * 1000L);
-        ArrayList<PooledObject<T>> remove = new ArrayList<>();
-        Iterator<PooledObject<T>> it = allObjects.values().iterator();
+        final ArrayList<PooledObject<T>> remove = new ArrayList<>();
+        final Iterator<PooledObject<T>> it = allObjects.values().iterator();
         while (it.hasNext()) {
-            PooledObject<T> pooledObject = it.next();
+            final PooledObject<T> pooledObject = it.next();
             synchronized (pooledObject) {
                 if (pooledObject.getState() == PooledObjectState.ALLOCATED &&
                         pooledObject.getLastUsedTime() <= timeout) {
@@ -1017,15 +1047,15 @@ public class GenericObjectPool<T> extend
         }
 
         // Now remove the abandoned objects
-        Iterator<PooledObject<T>> itr = remove.iterator();
+        final Iterator<PooledObject<T>> itr = remove.iterator();
         while (itr.hasNext()) {
-            PooledObject<T> pooledObject = itr.next();
+            final PooledObject<T> pooledObject = itr.next();
             if (ac.getLogAbandoned()) {
                 pooledObject.printStackTrace(ac.getLogWriter());
             }
             try {
                 invalidateObject(pooledObject.getObject());
-            } catch (Exception e) {
+            } catch (final Exception e) {
                 e.printStackTrace();
             }
         }
@@ -1035,10 +1065,10 @@ public class GenericObjectPool<T> extend
     //--- Usage tracking support -----------------------------------------------
 
     @Override
-    public void use(T pooledObject) {
-        AbandonedConfig ac = this.abandonedConfig;
+    public void use(final T pooledObject) {
+        final AbandonedConfig ac = this.abandonedConfig;
         if (ac != null && ac.getUseUsageTracking()) {
-            PooledObject<T> wrapper = allObjects.get(new IdentityWrapper<>(pooledObject));
+            final PooledObject<T> wrapper = allObjects.get(new IdentityWrapper<>(pooledObject));
             wrapper.use();
         }
     }
@@ -1074,10 +1104,10 @@ public class GenericObjectPool<T> extend
     public String getFactoryType() {
         // Not thread safe. Accept that there may be multiple evaluations.
         if (factoryType == null) {
-            StringBuilder result = new StringBuilder();
+            final StringBuilder result = new StringBuilder();
             result.append(factory.getClass().getName());
             result.append('<');
-            Class<?> pooledObjectType =
+            final Class<?> pooledObjectType =
                     PoolImplUtils.getFactoryType(factory.getClass());
             result.append(pooledObjectType.getName());
             result.append('>');
@@ -1099,9 +1129,9 @@ public class GenericObjectPool<T> extend
      */
     @Override
     public Set<DefaultPooledObjectInfo> listAllObjects() {
-        Set<DefaultPooledObjectInfo> result =
+        final Set<DefaultPooledObjectInfo> result =
                 new HashSet<>(allObjects.size());
-        for (PooledObject<T> p : allObjects.values()) {
+        for (final PooledObject<T> p : allObjects.values()) {
             result.add(new DefaultPooledObjectInfo(p));
         }
         return result;
@@ -1133,6 +1163,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
@@ -1143,7 +1175,7 @@ public class GenericObjectPool<T> extend
     private volatile AbandonedConfig abandonedConfig = null;
 
     @Override
-    protected void toStringAppendFields(StringBuilder builder) {
+    protected void toStringAppendFields(final StringBuilder builder) {
         super.toStringAppendFields(builder);
         builder.append(", factoryType=");
         builder.append(factoryType);

Modified: tomcat/tc8.5.x/trunk/java/org/apache/tomcat/dbcp/pool2/impl/GenericObjectPoolConfig.java
URL: http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/java/org/apache/tomcat/dbcp/pool2/impl/GenericObjectPoolConfig.java?rev=1743721&r1=1743720&r2=1743721&view=diff
==============================================================================
--- tomcat/tc8.5.x/trunk/java/org/apache/tomcat/dbcp/pool2/impl/GenericObjectPoolConfig.java (original)
+++ tomcat/tc8.5.x/trunk/java/org/apache/tomcat/dbcp/pool2/impl/GenericObjectPoolConfig.java Fri May 13 18:50:08 2016
@@ -75,7 +75,7 @@ public class GenericObjectPoolConfig ext
      *
      * @see GenericObjectPool#setMaxTotal(int)
      */
-    public void setMaxTotal(int maxTotal) {
+    public void setMaxTotal(final int maxTotal) {
         this.maxTotal = maxTotal;
     }
 
@@ -102,7 +102,7 @@ public class GenericObjectPoolConfig ext
      *
      * @see GenericObjectPool#setMaxIdle(int)
      */
-    public void setMaxIdle(int maxIdle) {
+    public void setMaxIdle(final int maxIdle) {
         this.maxIdle = maxIdle;
     }
 
@@ -129,7 +129,7 @@ public class GenericObjectPoolConfig ext
      *
      * @see GenericObjectPool#setMinIdle(int)
      */
-    public void setMinIdle(int minIdle) {
+    public void setMinIdle(final int minIdle) {
         this.minIdle = minIdle;
     }
 
@@ -137,13 +137,13 @@ public class GenericObjectPoolConfig ext
     public GenericObjectPoolConfig clone() {
         try {
             return (GenericObjectPoolConfig) super.clone();
-        } catch (CloneNotSupportedException e) {
+        } catch (final CloneNotSupportedException e) {
             throw new AssertionError(); // Can't happen
         }
     }
 
     @Override
-    protected void toStringAppendFields(StringBuilder builder) {
+    protected void toStringAppendFields(final StringBuilder builder) {
         super.toStringAppendFields(builder);
         builder.append(", maxTotal=");
         builder.append(maxTotal);

Modified: tomcat/tc8.5.x/trunk/java/org/apache/tomcat/dbcp/pool2/impl/InterruptibleReentrantLock.java
URL: http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/java/org/apache/tomcat/dbcp/pool2/impl/InterruptibleReentrantLock.java?rev=1743721&r1=1743720&r2=1743721&view=diff
==============================================================================
--- tomcat/tc8.5.x/trunk/java/org/apache/tomcat/dbcp/pool2/impl/InterruptibleReentrantLock.java (original)
+++ tomcat/tc8.5.x/trunk/java/org/apache/tomcat/dbcp/pool2/impl/InterruptibleReentrantLock.java Fri May 13 18:50:08 2016
@@ -39,7 +39,7 @@ class InterruptibleReentrantLock extends
      * @param fairness true means threads should acquire contended locks as if
      * waiting in a FIFO queue
      */
-    public InterruptibleReentrantLock(boolean fairness) {
+    public InterruptibleReentrantLock(final boolean fairness) {
         super(fairness);
     }
 
@@ -48,9 +48,9 @@ class InterruptibleReentrantLock extends
      *
      * @param condition the condition on which the threads are waiting.
      */
-    public void interruptWaiters(Condition condition) {
-        Collection<Thread> threads = getWaitingThreads(condition);
-        for (Thread thread : threads) {
+    public void interruptWaiters(final Condition condition) {
+        final Collection<Thread> threads = getWaitingThreads(condition);
+        for (final Thread thread : threads) {
             thread.interrupt();
         }
     }



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message