commons-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mattsic...@apache.org
Subject [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.
Date Sun, 12 Mar 2017 19:48:11 GMT
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 <markt@apache.org>
Authored: Wed Nov 2 20:53:11 2016 +0000
Committer: Mark Thomas <markt@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());
     }
 


Mime
View raw message