You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2019/05/03 15:57:37 UTC
[tomcat] branch master updated: Update internal fork of Apache
Commons Pool 2
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push:
new d676a1e Update internal fork of Apache Commons Pool 2
d676a1e is described below
commit d676a1eb64c3d3c93abe90daa3226bfa448f68ec
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri May 3 16:57:22 2019 +0100
Update internal fork of Apache Commons Pool 2
---
MERGE.txt | 2 +-
java/org/apache/tomcat/dbcp/pool2/PoolUtils.java | 47 +++++++++++++---------
.../dbcp/pool2/impl/BaseGenericObjectPool.java | 8 ++--
.../tomcat/dbcp/pool2/impl/CallStackUtils.java | 8 +---
.../tomcat/dbcp/pool2/impl/EvictionTimer.java | 35 +++++++++-------
.../dbcp/pool2/impl/GenericKeyedObjectPool.java | 25 +++++++-----
.../tomcat/dbcp/pool2/impl/GenericObjectPool.java | 26 ++++++++++++
webapps/docs/changelog.xml | 6 ++-
8 files changed, 97 insertions(+), 60 deletions(-)
diff --git a/MERGE.txt b/MERGE.txt
index e28ca94..7d24a98 100644
--- a/MERGE.txt
+++ b/MERGE.txt
@@ -69,4 +69,4 @@ Pool2
Sub-tree
src/main/java/org/apache/commons/pool2
The SHA1 ID for the most recent commit to be merged to Tomcat is:
-d4e0e88227ad91d8c8ef36ba01d656f71c770f83
+0664f4dac9ef653703624cbe67272134cf0151cb (2019-04-30)
diff --git a/java/org/apache/tomcat/dbcp/pool2/PoolUtils.java b/java/org/apache/tomcat/dbcp/pool2/PoolUtils.java
index 4fb0aba..2494351 100644
--- a/java/org/apache/tomcat/dbcp/pool2/PoolUtils.java
+++ b/java/org/apache/tomcat/dbcp/pool2/PoolUtils.java
@@ -36,6 +36,13 @@ import java.util.concurrent.locks.ReentrantReadWriteLock.WriteLock;
*/
public final class PoolUtils {
+ private static final String MSG_FACTOR_NEGATIVE = "factor must be positive.";
+ private static final String MSG_MIN_IDLE = "minIdle must be non-negative.";
+ private static final String MSG_NULL_KEY = "key must not be null.";
+ private static final String MSG_NULL_KEYED_POOL = "keyedPool must not be null.";
+ private static final String MSG_NULL_KEYS = "keys must not be null.";
+ private static final String MSG_NULL_POOL = "pool must not be null.";
+
/**
* Timer used to periodically check pools idle object count. Because a
* {@link Timer} creates a {@link Thread}, an IODH is used.
@@ -101,10 +108,10 @@ public final class PoolUtils {
final int minIdle, final long period)
throws IllegalArgumentException {
if (pool == null) {
- throw new IllegalArgumentException("keyedPool must not be null.");
+ throw new IllegalArgumentException(MSG_NULL_KEYED_POOL);
}
if (minIdle < 0) {
- throw new IllegalArgumentException("minIdle must be non-negative.");
+ throw new IllegalArgumentException(MSG_MIN_IDLE);
}
final TimerTask task = new ObjectPoolMinIdleTimerTask<>(pool, minIdle);
getMinIdleTimer().schedule(task, 0L, period);
@@ -142,13 +149,13 @@ public final class PoolUtils {
final int minIdle, final long period)
throws IllegalArgumentException {
if (keyedPool == null) {
- throw new IllegalArgumentException("keyedPool must not be null.");
+ throw new IllegalArgumentException(MSG_NULL_KEYED_POOL);
}
if (key == null) {
- throw new IllegalArgumentException("key must not be null.");
+ throw new IllegalArgumentException(MSG_NULL_KEY);
}
if (minIdle < 0) {
- throw new IllegalArgumentException("minIdle must be non-negative.");
+ throw new IllegalArgumentException(MSG_MIN_IDLE);
}
final TimerTask task = new KeyedObjectPoolMinIdleTimerTask<>(
keyedPool, key, minIdle);
@@ -188,7 +195,7 @@ public final class PoolUtils {
final int minIdle, final long period)
throws IllegalArgumentException {
if (keys == null) {
- throw new IllegalArgumentException("keys must not be null.");
+ throw new IllegalArgumentException(MSG_NULL_KEYS);
}
final Map<K, TimerTask> tasks = new HashMap<>(keys.size());
final Iterator<K> iter = keys.iterator();
@@ -217,7 +224,7 @@ public final class PoolUtils {
public static <T> void prefill(final ObjectPool<T> pool, final int count)
throws Exception, IllegalArgumentException {
if (pool == null) {
- throw new IllegalArgumentException("pool must not be null.");
+ throw new IllegalArgumentException(MSG_NULL_POOL);
}
for (int i = 0; i < count; i++) {
pool.addObject();
@@ -246,10 +253,10 @@ public final class PoolUtils {
final K key, final int count) throws Exception,
IllegalArgumentException {
if (keyedPool == null) {
- throw new IllegalArgumentException("keyedPool must not be null.");
+ throw new IllegalArgumentException(MSG_NULL_KEYED_POOL);
}
if (key == null) {
- throw new IllegalArgumentException("key must not be null.");
+ throw new IllegalArgumentException(MSG_NULL_KEY);
}
for (int i = 0; i < count; i++) {
keyedPool.addObject(key);
@@ -281,7 +288,7 @@ public final class PoolUtils {
final Collection<K> keys, final int count) throws Exception,
IllegalArgumentException {
if (keys == null) {
- throw new IllegalArgumentException("keys must not be null.");
+ throw new IllegalArgumentException(MSG_NULL_KEYS);
}
final Iterator<K> iter = keys.iterator();
while (iter.hasNext()) {
@@ -308,7 +315,7 @@ public final class PoolUtils {
*/
public static <T> ObjectPool<T> synchronizedPool(final ObjectPool<T> pool) {
if (pool == null) {
- throw new IllegalArgumentException("pool must not be null.");
+ throw new IllegalArgumentException(MSG_NULL_POOL);
}
/*
* assert !(pool instanceof GenericObjectPool) :
@@ -436,10 +443,10 @@ public final class PoolUtils {
public static <T> ObjectPool<T> erodingPool(final ObjectPool<T> pool,
final float factor) {
if (pool == null) {
- throw new IllegalArgumentException("pool must not be null.");
+ throw new IllegalArgumentException(MSG_NULL_POOL);
}
if (factor <= 0f) {
- throw new IllegalArgumentException("factor must be positive.");
+ throw new IllegalArgumentException(MSG_FACTOR_NEGATIVE);
}
return new ErodingObjectPool<>(pool, factor);
}
@@ -538,10 +545,10 @@ public final class PoolUtils {
final KeyedObjectPool<K, V> keyedPool, final float factor,
final boolean perKey) {
if (keyedPool == null) {
- throw new IllegalArgumentException("keyedPool must not be null.");
+ throw new IllegalArgumentException(MSG_NULL_KEYED_POOL);
}
if (factor <= 0f) {
- throw new IllegalArgumentException("factor must be positive.");
+ throw new IllegalArgumentException(MSG_FACTOR_NEGATIVE);
}
if (perKey) {
return new ErodingPerKeyKeyedObjectPool<>(keyedPool, factor);
@@ -587,7 +594,7 @@ public final class PoolUtils {
ObjectPoolMinIdleTimerTask(final ObjectPool<T> pool, final int minIdle)
throws IllegalArgumentException {
if (pool == null) {
- throw new IllegalArgumentException("pool must not be null.");
+ throw new IllegalArgumentException(MSG_NULL_POOL);
}
this.pool = pool;
this.minIdle = minIdle;
@@ -665,7 +672,7 @@ public final class PoolUtils {
final K key, final int minIdle) throws IllegalArgumentException {
if (keyedPool == null) {
throw new IllegalArgumentException(
- "keyedPool must not be null.");
+ MSG_NULL_KEYED_POOL);
}
this.keyedPool = keyedPool;
this.key = key;
@@ -747,7 +754,7 @@ public final class PoolUtils {
SynchronizedObjectPool(final ObjectPool<T> pool)
throws IllegalArgumentException {
if (pool == null) {
- throw new IllegalArgumentException("pool must not be null.");
+ throw new IllegalArgumentException(MSG_NULL_POOL);
}
this.pool = pool;
}
@@ -924,7 +931,7 @@ public final class PoolUtils {
throws IllegalArgumentException {
if (keyedPool == null) {
throw new IllegalArgumentException(
- "keyedPool must not be null.");
+ MSG_NULL_KEYED_POOL);
}
this.keyedPool = keyedPool;
}
@@ -1605,7 +1612,7 @@ public final class PoolUtils {
final ErodingFactor erodingFactor) {
if (keyedPool == null) {
throw new IllegalArgumentException(
- "keyedPool must not be null.");
+ MSG_NULL_KEYED_POOL);
}
this.keyedPool = keyedPool;
this.erodingFactor = erodingFactor;
diff --git a/java/org/apache/tomcat/dbcp/pool2/impl/BaseGenericObjectPool.java b/java/org/apache/tomcat/dbcp/pool2/impl/BaseGenericObjectPool.java
index 0de61d6..448c4a7 100644
--- a/java/org/apache/tomcat/dbcp/pool2/impl/BaseGenericObjectPool.java
+++ b/java/org/apache/tomcat/dbcp/pool2/impl/BaseGenericObjectPool.java
@@ -772,11 +772,9 @@ public abstract class BaseGenericObjectPool<T> extends BaseObject {
*/
final void startEvictor(final long delay) {
synchronized (evictionLock) {
- if (null != evictor) {
- EvictionTimer.cancel(evictor, evictorShutdownTimeoutMillis, TimeUnit.MILLISECONDS);
- evictor = null;
- evictionIterator = null;
- }
+ EvictionTimer.cancel(evictor, evictorShutdownTimeoutMillis, TimeUnit.MILLISECONDS);
+ evictor = null;
+ evictionIterator = null;
if (delay > 0) {
evictor = new Evictor();
EvictionTimer.schedule(evictor, delay, delay);
diff --git a/java/org/apache/tomcat/dbcp/pool2/impl/CallStackUtils.java b/java/org/apache/tomcat/dbcp/pool2/impl/CallStackUtils.java
index d5089c3..871f311 100644
--- a/java/org/apache/tomcat/dbcp/pool2/impl/CallStackUtils.java
+++ b/java/org/apache/tomcat/dbcp/pool2/impl/CallStackUtils.java
@@ -25,12 +25,6 @@ import java.security.AccessControlException;
*/
public final class CallStackUtils {
- private static final boolean CAN_CREATE_SECURITY_MANAGER;
-
- static {
- CAN_CREATE_SECURITY_MANAGER = canCreateSecurityManager();
- }
-
/**
* @return {@code true} if it is able to create a security manager in the current environment, {@code false}
* otherwise.
@@ -76,7 +70,7 @@ public final class CallStackUtils {
public static CallStack newCallStack(final String messageFormat,
final boolean useTimestamp,
final boolean requireFullStackTrace) {
- return CAN_CREATE_SECURITY_MANAGER && !requireFullStackTrace
+ return canCreateSecurityManager() && !requireFullStackTrace
? new SecurityManagerCallStack(messageFormat, useTimestamp)
: new ThrowableCallStack(messageFormat, useTimestamp);
}
diff --git a/java/org/apache/tomcat/dbcp/pool2/impl/EvictionTimer.java b/java/org/apache/tomcat/dbcp/pool2/impl/EvictionTimer.java
index b483dc3..f034c38 100644
--- a/java/org/apache/tomcat/dbcp/pool2/impl/EvictionTimer.java
+++ b/java/org/apache/tomcat/dbcp/pool2/impl/EvictionTimer.java
@@ -24,18 +24,20 @@ import java.util.concurrent.ThreadFactory;
import java.util.concurrent.TimeUnit;
/**
- * Provides a shared idle object eviction timer for all pools. This class is
- * currently implemented using {@link ScheduledThreadPoolExecutor}. This
- * implementation may change in any future release. This class keeps track of
- * how many pools are using it. If no pools are using the timer, it is cancelled.
- * This prevents a thread being left running which, in application server
- * environments, can lead to memory leads and/or prevent applications from
- * shutting down or reloading cleanly.
+ * Provides a shared idle object eviction timer for all pools.
* <p>
- * This class has package scope to prevent its inclusion in the pool public API.
- * The class declaration below should *not* be changed to public.
+ * This class is currently implemented using {@link ScheduledThreadPoolExecutor}. This implementation may change in any
+ * future release. This class keeps track of how many pools are using it. If no pools are using the timer, it is
+ * cancelled. This prevents a thread being left running which, in application server environments, can lead to memory
+ * leads and/or prevent applications from shutting down or reloading cleanly.
+ * </p>
+ * <p>
+ * This class has package scope to prevent its inclusion in the pool public API. The class declaration below should
+ * *not* be changed to public.
+ * </p>
* <p>
* This class is intended to be thread-safe.
+ * </p>
*
* @since 2.0
*/
@@ -66,6 +68,7 @@ class EvictionTimer {
* call to this method *must* call {@link #cancel(BaseGenericObjectPool.Evictor,long,TimeUnit)}
* to cancel the task to prevent memory and/or thread leaks in application
* server environments.
+ *
* @param task Task to be scheduled
* @param delay Delay in milliseconds before task is executed
* @param period Time in milliseconds between executions
@@ -84,16 +87,18 @@ class EvictionTimer {
/**
* Remove the specified eviction task from the timer.
*
- * @param task Task to be cancelled
+ * @param evictor Task to be cancelled
* @param timeout If the associated executor is no longer required, how
* long should this thread wait for the executor to
* terminate?
* @param unit The units for the specified timeout
*/
static synchronized void cancel(
- final BaseGenericObjectPool<?>.Evictor task, final long timeout, final TimeUnit unit) {
- task.cancel();
- if (executor.getQueue().size() == 0) {
+ final BaseGenericObjectPool<?>.Evictor evictor, final long timeout, final TimeUnit unit) {
+ if (evictor != null) {
+ evictor.cancel();
+ }
+ if (executor != null && executor.getQueue().isEmpty()) {
executor.shutdown();
try {
executor.awaitTermination(timeout, unit);
@@ -107,14 +112,14 @@ class EvictionTimer {
}
/**
- * Thread factory that creates a thread, with the context class loader from this class.
+ * Thread factory that creates a daemon thread, with the context class loader from this class.
*/
private static class EvictorThreadFactory implements ThreadFactory {
@Override
public Thread newThread(final Runnable runnable) {
final Thread thread = new Thread(null, runnable, "commons-pool-evictor-thread");
-
+ thread.setDaemon(true); // POOL-363 - Required for applications using Runtime.addShutdownHook(). --joshlandin 03.27.2019
AccessController.doPrivileged(new PrivilegedAction<Void>() {
@Override
public Void run() {
diff --git a/java/org/apache/tomcat/dbcp/pool2/impl/GenericKeyedObjectPool.java b/java/org/apache/tomcat/dbcp/pool2/impl/GenericKeyedObjectPool.java
index d9c9c8f..7de46a3 100644
--- a/java/org/apache/tomcat/dbcp/pool2/impl/GenericKeyedObjectPool.java
+++ b/java/org/apache/tomcat/dbcp/pool2/impl/GenericKeyedObjectPool.java
@@ -1145,26 +1145,29 @@ public class GenericKeyedObjectPool<K, T> extends BaseGenericObjectPool<T>
* @param k The key to de-register
*/
private void deregister(final K k) {
+ Lock lock = keyLock.readLock();
ObjectDeque<T> objectDeque;
- objectDeque = poolMap.get(k);
- final long numInterested = objectDeque.getNumInterested().decrementAndGet();
- if (numInterested == 0 && objectDeque.getCreateCount().get() == 0) {
- // Potential to remove key
- final Lock writeLock = keyLock.writeLock();
- writeLock.lock();
- try {
- if (objectDeque.getCreateCount().get() == 0 &&
- objectDeque.getNumInterested().get() == 0) {
+ try {
+ lock.lock();
+ objectDeque = poolMap.get(k);
+ final long numInterested = objectDeque.getNumInterested().decrementAndGet();
+ if (numInterested == 0 && objectDeque.getCreateCount().get() == 0) {
+ // Potential to remove key
+ // Upgrade to write lock
+ lock.unlock();
+ lock = keyLock.writeLock();
+ lock.lock();
+ if (objectDeque.getCreateCount().get() == 0 && objectDeque.getNumInterested().get() == 0) {
// NOTE: Keys must always be removed from both poolMap and
// poolKeyList at the same time while protected by
// keyLock.writeLock()
poolMap.remove(k);
poolKeyList.remove(k);
}
- } finally {
- writeLock.unlock();
}
+ } finally {
+ lock.unlock();
}
}
diff --git a/java/org/apache/tomcat/dbcp/pool2/impl/GenericObjectPool.java b/java/org/apache/tomcat/dbcp/pool2/impl/GenericObjectPool.java
index 15b6d6d..04fdd79 100644
--- a/java/org/apache/tomcat/dbcp/pool2/impl/GenericObjectPool.java
+++ b/java/org/apache/tomcat/dbcp/pool2/impl/GenericObjectPool.java
@@ -181,6 +181,7 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
* <p>
* If the configured value of minIdle is greater than the configured value
* for maxIdle then the value of maxIdle will be used instead.
+ * </p>
*
* @param minIdle
* The minimum number of objects.
@@ -202,6 +203,7 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
* <p>
* If the configured value of minIdle is greater than the configured value
* for maxIdle then the value of maxIdle will be used instead.
+ * </p>
*
* @return The minimum number of objects.
*
@@ -339,6 +341,7 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
* borrowObject}({@link #getMaxWaitMillis()})</code>.
* <p>
* {@inheritDoc}
+ * </p>
*/
@Override
public T borrowObject() throws Exception {
@@ -356,6 +359,7 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
* instance is destroyed and the next available instance is examined. This
* continues until either a valid instance is returned or there are no more
* idle instances available.
+ * </p>
* <p>
* If there are no idle instances available in the pool, behavior depends on
* the {@link #getMaxTotal() maxTotal}, (if applicable)
@@ -365,6 +369,7 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
* instance is created, activated and (if applicable) validated and returned
* to the caller. If validation fails, a <code>NoSuchElementException</code>
* is thrown.
+ * </p>
* <p>
* If the pool is exhausted (no available idle instances and no capacity to
* create new ones), this method will either block (if
@@ -374,11 +379,13 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
* method will block when {@link #getBlockWhenExhausted()} is true is
* determined by the value passed in to the <code>borrowMaxWaitMillis</code>
* parameter.
+ * </p>
* <p>
* When the pool is exhausted, multiple calling threads may be
* simultaneously blocked waiting for instances to become available. A
* "fairness" algorithm has been implemented to ensure that threads receive
* available instances in request arrival order.
+ * </p>
*
* @param borrowMaxWaitMillis The time to wait in milliseconds for an object
* to become available
@@ -496,14 +503,17 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
* If {@link #getMaxIdle() maxIdle} is set to a positive value and the
* number of idle instances has reached this value, the returning instance
* is destroyed.
+ * </p>
* <p>
* If {@link #getTestOnReturn() testOnReturn} == true, the returning
* instance is validated before being returned to the idle instance pool. In
* this case, if validation fails, the instance is destroyed.
+ * </p>
* <p>
* Exceptions encountered destroying objects for any reason are swallowed
* but notified via a
* {@link org.apache.tomcat.dbcp.pool2.SwallowedExceptionListener}.
+ * </p>
*/
@Override
public void returnObject(final T obj) {
@@ -587,6 +597,7 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
* <p>
* Activation of this method decrements the active count and attempts to
* destroy the instance.
+ * </p>
*
* @throws Exception if an exception occurs destroying the
* object
@@ -617,6 +628,7 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
* idle instance.
* <p>
* Implementation notes:
+ * </p>
* <ul>
* <li>This method does not destroy or effect in any way instances that are
* checked out of the pool when it is invoked.</li>
@@ -659,6 +671,7 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
* objects destroyed on return.
* <p>
* Destroys idle instances in the pool by invoking {@link #clear()}.
+ * </p>
*/
@Override
public void close() {
@@ -691,6 +704,7 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
* <p>
* Successive activations of this method examine objects in sequence,
* cycling through objects in oldest-to-youngest order.
+ * </p>
*/
@Override
public void evict() throws Exception {
@@ -811,6 +825,7 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
* <p>
* If there are {@link #getMaxTotal()} objects already in circulation
* or in process of being created, this method returns null.
+ * </p>
*
* @return The new wrapped pooled object
*
@@ -915,6 +930,15 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
destroyedCount.incrementAndGet();
createCount.decrementAndGet();
}
+
+ if (idleObjects.isEmpty() && idleObjects.hasTakeWaiters()) {
+ // POOL-356.
+ // In case there are already threads waiting on something in the pool
+ // (e.g. idleObjects.takeFirst(); then we need to provide them a fresh instance.
+ // Otherwise they will be stuck forever (or until timeout)
+ final PooledObject<T> freshPooled = create();
+ idleObjects.put(freshPooled);
+ }
}
@Override
@@ -929,6 +953,7 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
* or the total number of objects (idle, checked out, or being created) reaches
* {@link #getMaxTotal()}. If {@code always} is false, no instances are created unless
* there are threads waiting to check out instances from the pool.
+ * </p>
*
* @param idleCount the number of idle instances desired
* @param always true means create instances even if the pool has no threads waiting
@@ -1115,6 +1140,7 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
* JMX. That means it won't be invoked unless the explicitly requested
* whereas all attributes will be automatically requested when viewing the
* attributes for an object in a tool like JConsole.
+ * </p>
*
* @return Information grouped on all the objects in the pool
*/
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 7c77575..e75f345 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -215,9 +215,13 @@
(2019-04-24) pick up some enhancements. (markt)
</update>
<update>
- Update the internal fork of Apache Commons DBCP 2 to
+ Update the internal fork of Apache Commons DBCP 2 to dcdbc72
(2019-04-24) to pick up some clean-up and enhancements. (markt)
</update>
+ <update>
+ Update the internal fork of Apache Commons Pool 2 to 0664f4d
+ (2019-04-30) to pick up some enhancements and bug fixes. (markt)
+ </update>
</changelog>
</subsection>
</section>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org