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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
