You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by ma...@apache.org on 2017/03/12 19:48:11 UTC
[41/50] [abbrv] commons-pool git commit: Fix POOL-351 Add a
configurable delay (default 10 seconds) to wait when shutting down an Evictor
to allow the associated thread time to complete and current evictions and to
terminate.
Fix POOL-351
Add a configurable delay (default 10 seconds) to wait when shutting down an Evictor to allow the associated thread time to complete and current evictions and to terminate.
git-svn-id: https://svn.apache.org/repos/asf/commons/proper/pool/trunk@1767782 13f79535-47bb-0310-9956-ffa450edef68
Project: http://git-wip-us.apache.org/repos/asf/commons-pool/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-pool/commit/4a20cdca
Tree: http://git-wip-us.apache.org/repos/asf/commons-pool/tree/4a20cdca
Diff: http://git-wip-us.apache.org/repos/asf/commons-pool/diff/4a20cdca
Branch: refs/heads/master
Commit: 4a20cdca923bd342360f821d7020538e985d9ec2
Parents: b15bc63
Author: Mark Thomas <ma...@apache.org>
Authored: Wed Nov 2 20:53:11 2016 +0000
Committer: Mark Thomas <ma...@apache.org>
Committed: Wed Nov 2 20:53:11 2016 +0000
----------------------------------------------------------------------
src/changes/changes.xml | 5 +
.../pool2/impl/BaseGenericObjectPool.java | 30 +++-
.../pool2/impl/BaseObjectPoolConfig.java | 48 ++++++-
.../commons/pool2/impl/EvictionTimer.java | 144 +++++++------------
.../pool2/impl/GenericKeyedObjectPool.java | 1 +
.../commons/pool2/impl/GenericObjectPool.java | 1 +
.../pool2/impl/TestGenericObjectPool.java | 1 +
7 files changed, 132 insertions(+), 98 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/commons-pool/blob/4a20cdca/src/changes/changes.xml
----------------------------------------------------------------------
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 5cca806..dca51b3 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -71,6 +71,11 @@ The <action> type attribute can be add,update,fix,remove.
Ensure that any class name used for evictionPolicyClassName represents a
class that implements EvictionPolicy.
</action>
+ <action dev="markt" issue="POOL-351" type="fix">
+ Add a configurable delay (default 10 seconds) to wait when shutting down
+ an Evictor to allow the associated thread time to complete and current
+ evictions and to terminate.
+ </action>
</release>
<release version="2.4.2" date="2015-08-01" description=
"This is a patch release, including bug fixes only.">
http://git-wip-us.apache.org/repos/asf/commons-pool/blob/4a20cdca/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java b/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java
index 8afa8f1..1f46811 100644
--- a/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java
+++ b/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java
@@ -25,6 +25,7 @@ import java.util.Arrays;
import java.util.Deque;
import java.util.Iterator;
import java.util.TimerTask;
+import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicLong;
import javax.management.InstanceAlreadyExistsException;
@@ -87,6 +88,8 @@ public abstract class BaseGenericObjectPool<T> extends BaseObject {
private volatile long softMinEvictableIdleTimeMillis =
BaseObjectPoolConfig.DEFAULT_SOFT_MIN_EVICTABLE_IDLE_TIME_MILLIS;
private volatile EvictionPolicy<T> evictionPolicy;
+ private long evictorShutdownTimeoutMillis =
+ BaseObjectPoolConfig.DEFAULT_EVICTOR_SHUTDOWN_TIMEOUT_MILLIS;
// Internal (primarily state) attributes
@@ -632,6 +635,31 @@ public abstract class BaseGenericObjectPool<T> extends BaseObject {
}
}
+ /**
+ * Gets the timeout that will be used when waiting for the Evictor to
+ * shutdown if this pool is closed and it is the only pool still using the
+ * the value for the Evictor.
+ *
+ * @return The timeout in milliseconds that will be used while waiting for
+ * the Evictor to shut down.
+ */
+ public long getEvictorShutdownTimeoutMillis() {
+ return evictorShutdownTimeoutMillis;
+ }
+
+ /**
+ * Sets the timeout that will be used when waiting for the Evictor to
+ * shutdown if this pool is closed and it is the only pool still using the
+ * the value for the Evictor.
+ *
+ * @param evictorShutdownTimeoutMillis the timeout in milliseconds that
+ * will be used while waiting for the
+ * Evictor to shut down.
+ */
+ public void setEvictorShutdownTimeoutMillis(
+ final long evictorShutdownTimeoutMillis) {
+ this.evictorShutdownTimeoutMillis = evictorShutdownTimeoutMillis;
+ }
/**
* Closes the pool, destroys the remaining idle objects and, if registered
@@ -692,7 +720,7 @@ public abstract class BaseGenericObjectPool<T> extends BaseObject {
final void startEvictor(final long delay) {
synchronized (evictionLock) {
if (null != evictor) {
- EvictionTimer.cancel(evictor);
+ EvictionTimer.cancel(evictor, 10, TimeUnit.SECONDS);
evictor = null;
evictionIterator = null;
}
http://git-wip-us.apache.org/repos/asf/commons-pool/blob/4a20cdca/src/main/java/org/apache/commons/pool2/impl/BaseObjectPoolConfig.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/pool2/impl/BaseObjectPoolConfig.java b/src/main/java/org/apache/commons/pool2/impl/BaseObjectPoolConfig.java
index 2f1a595..d635227 100644
--- a/src/main/java/org/apache/commons/pool2/impl/BaseObjectPoolConfig.java
+++ b/src/main/java/org/apache/commons/pool2/impl/BaseObjectPoolConfig.java
@@ -70,6 +70,15 @@ public abstract class BaseObjectPoolConfig extends BaseObject implements Cloneab
public static final long DEFAULT_SOFT_MIN_EVICTABLE_IDLE_TIME_MILLIS = -1;
/**
+ * The default value for {@code evictorShutdownTimeoutMillis} configuration
+ * attribute.
+ * @see GenericObjectPool#getEvictorShutdownTimeoutMillis()
+ * @see GenericKeyedObjectPool#getEvictorShutdownTimeoutMillis()
+ */
+ public static final long DEFAULT_EVICTOR_SHUTDOWN_TIMEOUT_MILLIS =
+ 10L * 1000L;
+
+ /**
* The default value for the {@code numTestsPerEvictionRun} configuration
* attribute.
* @see GenericObjectPool#getNumTestsPerEvictionRun()
@@ -163,13 +172,16 @@ public abstract class BaseObjectPoolConfig extends BaseObject implements Cloneab
private long maxWaitMillis = DEFAULT_MAX_WAIT_MILLIS;
private long minEvictableIdleTimeMillis =
- DEFAULT_MIN_EVICTABLE_IDLE_TIME_MILLIS;
+ DEFAULT_MIN_EVICTABLE_IDLE_TIME_MILLIS;
+
+ private long evictorShutdownTimeoutMillis =
+ DEFAULT_EVICTOR_SHUTDOWN_TIMEOUT_MILLIS;
private long softMinEvictableIdleTimeMillis =
DEFAULT_MIN_EVICTABLE_IDLE_TIME_MILLIS;
private int numTestsPerEvictionRun =
- DEFAULT_NUM_TESTS_PER_EVICTION_RUN;
+ DEFAULT_NUM_TESTS_PER_EVICTION_RUN;
private String evictionPolicyClassName = DEFAULT_EVICTION_POLICY_CLASS_NAME;
@@ -182,7 +194,7 @@ public abstract class BaseObjectPoolConfig extends BaseObject implements Cloneab
private boolean testWhileIdle = DEFAULT_TEST_WHILE_IDLE;
private long timeBetweenEvictionRunsMillis =
- DEFAULT_TIME_BETWEEN_EVICTION_RUNS_MILLIS;
+ DEFAULT_TIME_BETWEEN_EVICTION_RUNS_MILLIS;
private boolean blockWhenExhausted = DEFAULT_BLOCK_WHEN_EXHAUSTED;
@@ -367,6 +379,36 @@ public abstract class BaseObjectPoolConfig extends BaseObject implements Cloneab
}
/**
+ * Get the value for the {@code evictorShutdownTimeoutMillis} configuration
+ * attribute for pools created with this configuration instance.
+ *
+ * @return The current setting of {@code evictorShutdownTimeoutMillis} for
+ * this configuration instance
+ *
+ * @see GenericObjectPool#getEvictorShutdownTimeoutMillis()
+ * @see GenericKeyedObjectPool#getEvictorShutdownTimeoutMillis()
+ */
+ public long getEvictorShutdownTimeoutMillis() {
+ return evictorShutdownTimeoutMillis;
+ }
+
+ /**
+ * Set the value for the {@code evictorShutdownTimeoutMillis} configuration
+ * attribute for pools created with this configuration instance.
+ *
+ * @param evictorShutdownTimeoutMillis The new setting of
+ * {@code evictorShutdownTimeoutMillis} for this configuration
+ * instance
+ *
+ * @see GenericObjectPool#getEvictorShutdownTimeoutMillis()
+ * @see GenericKeyedObjectPool#getEvictorShutdownTimeoutMillis()
+ */
+ public void setEvictorShutdownTimeoutMillis(
+ final long evictorShutdownTimeoutMillis) {
+ this.evictorShutdownTimeoutMillis = evictorShutdownTimeoutMillis;
+ }
+
+ /**
* Get the value for the {@code testOnCreate} configuration attribute for
* pools created with this configuration instance.
*
http://git-wip-us.apache.org/repos/asf/commons-pool/blob/4a20cdca/src/main/java/org/apache/commons/pool2/impl/EvictionTimer.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/pool2/impl/EvictionTimer.java b/src/main/java/org/apache/commons/pool2/impl/EvictionTimer.java
index 191dc86..b448141 100644
--- a/src/main/java/org/apache/commons/pool2/impl/EvictionTimer.java
+++ b/src/main/java/org/apache/commons/pool2/impl/EvictionTimer.java
@@ -18,16 +18,19 @@ package org.apache.commons.pool2.impl;
import java.security.AccessController;
import java.security.PrivilegedAction;
-import java.util.Timer;
import java.util.TimerTask;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.TimeUnit;
/**
- * Provides a shared idle object eviction timer for all pools. This class wraps
- * the standard {@link Timer} and keeps track of how many pools are using it.
- * If no pools are using the timer, it is canceled. 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. 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 canceled.
+ * 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>
* This class has package scope to prevent its inclusion in the pool public API.
* The class declaration below should *not* be changed to public.
@@ -38,11 +41,11 @@ import java.util.TimerTask;
*/
class EvictionTimer {
- /** Timer instance */
- private static Timer _timer; //@GuardedBy("EvictionTimer.class")
+ /** Executor instance */
+ private static ScheduledThreadPoolExecutor executor; //@GuardedBy("EvictionTimer.class")
/** Static usage count tracker */
- private static int _usageCount; //@GuardedBy("EvictionTimer.class")
+ private static int usageCount; //@GuardedBy("EvictionTimer.class")
/** Prevent instantiation */
private EvictionTimer() {
@@ -70,102 +73,55 @@ class EvictionTimer {
* @param delay Delay in milliseconds before task is executed
* @param period Time in milliseconds between executions
*/
- static synchronized void schedule(final TimerTask task, final long delay, final long period) {
- if (null == _timer) {
- // Force the new Timer thread to be created with a context class
- // loader set to the class loader that loaded this library
- final ClassLoader ccl = AccessController.doPrivileged(
- new PrivilegedGetTccl());
- try {
- AccessController.doPrivileged(new PrivilegedSetTccl(
- EvictionTimer.class.getClassLoader()));
- _timer = AccessController.doPrivileged(new PrivilegedNewEvictionTimer());
- } finally {
- AccessController.doPrivileged(new PrivilegedSetTccl(ccl));
- }
+ static synchronized void schedule(final Runnable task, final long delay, final long period) {
+ if (null == executor) {
+ executor = new ScheduledThreadPoolExecutor(1, new EvictorThreadFactory());
}
- _usageCount++;
- _timer.schedule(task, delay, period);
+ usageCount++;
+ executor.scheduleWithFixedDelay(task, delay, period, TimeUnit.MILLISECONDS);
}
/**
* Remove the specified eviction task from the timer.
- * @param task Task to be scheduled
+ *
+ * @param task 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 TimerTask task) {
+ static synchronized void cancel(final TimerTask task, long timeout, TimeUnit unit) {
task.cancel();
- _usageCount--;
- if (_usageCount == 0) {
- _timer.cancel();
- _timer = null;
- }
- }
-
- /**
- * {@link PrivilegedAction} used to get the ContextClassLoader
- */
- private static class PrivilegedGetTccl implements PrivilegedAction<ClassLoader> {
-
- /**
- * {@inheritDoc}
- */
- @Override
- public ClassLoader run() {
- return Thread.currentThread().getContextClassLoader();
- }
- }
-
- /**
- * {@link PrivilegedAction} used to set the ContextClassLoader
- */
- private static class PrivilegedSetTccl implements PrivilegedAction<Void> {
-
- /** ClassLoader */
- private final ClassLoader classLoader;
-
- /**
- * Create a new PrivilegedSetTccl using the given classloader
- * @param classLoader ClassLoader to use
- */
- PrivilegedSetTccl(final ClassLoader cl) {
- this.classLoader = cl;
- }
-
- /**
- * {@inheritDoc}
- */
- @Override
- public Void run() {
- Thread.currentThread().setContextClassLoader(classLoader);
- return null;
- }
-
- @Override
- public String toString() {
- final StringBuilder builder = new StringBuilder();
- builder.append("PrivilegedSetTccl [classLoader=");
- builder.append(classLoader);
- builder.append("]");
- return builder.toString();
+ usageCount--;
+ if (usageCount == 0) {
+ executor.shutdown();
+ try {
+ executor.awaitTermination(timeout, unit);
+ } catch (InterruptedException e) {
+ // Swallow
+ // Significant API changes would be required to propagate this
+ }
+ executor.setCorePoolSize(0);
+ executor = null;
}
}
- /**
- * {@link PrivilegedAction} used to create a new Timer. Creating the timer
- * with a privileged action means the associated Thread does not inherit the
- * current access control context. In a container environment, inheriting
- * the current access control context is likely to result in retaining a
- * reference to the thread context class loader which would be a memory
- * leak.
- */
- private static class PrivilegedNewEvictionTimer implements PrivilegedAction<Timer> {
+ private static class EvictorThreadFactory implements ThreadFactory {
- /**
- * {@inheritDoc}
- */
@Override
- public Timer run() {
- return new Timer("commons-pool-EvictionTimer", true);
+ public Thread newThread(final Runnable r) {
+ final Thread t = new Thread(null, r, "commons-pool-evictor-thrreads");
+ t.setName("commons-pool-evictor");
+
+ AccessController.doPrivileged(new PrivilegedAction<Void>() {
+ @Override
+ public Void run() {
+ t.setContextClassLoader(EvictorThreadFactory.class.getClassLoader());
+ return null;
+ }
+ });
+
+ return t;
}
}
}
http://git-wip-us.apache.org/repos/asf/commons-pool/blob/4a20cdca/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java b/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
index 6976f2a..7fa21b5 100644
--- a/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
+++ b/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
@@ -255,6 +255,7 @@ public class GenericKeyedObjectPool<K,T> extends BaseGenericObjectPool<T>
setTimeBetweenEvictionRunsMillis(
conf.getTimeBetweenEvictionRunsMillis());
setEvictionPolicyClassName(conf.getEvictionPolicyClassName());
+ setEvictorShutdownTimeoutMillis(conf.getEvictorShutdownTimeoutMillis());
}
/**
http://git-wip-us.apache.org/repos/asf/commons-pool/blob/4a20cdca/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java b/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
index 487eec2..be0f508 100644
--- a/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
+++ b/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
@@ -318,6 +318,7 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
setSoftMinEvictableIdleTimeMillis(
conf.getSoftMinEvictableIdleTimeMillis());
setEvictionPolicyClassName(conf.getEvictionPolicyClassName());
+ setEvictorShutdownTimeoutMillis(conf.getEvictorShutdownTimeoutMillis());
}
/**
http://git-wip-us.apache.org/repos/asf/commons-pool/blob/4a20cdca/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java b/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
index c9014ac..838aab4 100644
--- a/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
+++ b/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
@@ -1705,6 +1705,7 @@ public class TestGenericObjectPool extends TestBaseObjectPool {
assertEquals("maxWait",expected.getMaxWaitMillis(),actual.getMaxWaitMillis());
assertEquals("minEvictableIdleTimeMillis",expected.getMinEvictableIdleTimeMillis(),actual.getMinEvictableIdleTimeMillis());
assertEquals("numTestsPerEvictionRun",expected.getNumTestsPerEvictionRun(),actual.getNumTestsPerEvictionRun());
+ assertEquals("evictorShutdownTimeoutMillis",expected.getEvictorShutdownTimeoutMillis(),actual.getEvictorShutdownTimeoutMillis());
assertEquals("timeBetweenEvictionRunsMillis",expected.getTimeBetweenEvictionRunsMillis(),actual.getTimeBetweenEvictionRunsMillis());
}