Author: markt Date: Tue Sep 23 13:43:21 2014 New Revision: 1627027 URL: http://svn.apache.org/r1627027 Log: Update Pool2 to latest trunk to pick up fixes/updates for: - memory leak via the Evictor - potential although unlikely threading issue - application provided eviction policies - POOL-270, POOL-276
Modified: tomcat/trunk/java/org/apache/tomcat/dbcp/pool2/ (props changed) tomcat/trunk/java/org/apache/tomcat/dbcp/pool2/impl/BaseGenericObjectPool.java tomcat/trunk/java/org/apache/tomcat/dbcp/pool2/impl/GenericKeyedObjectPool.java tomcat/trunk/java/org/apache/tomcat/dbcp/pool2/impl/GenericObjectPool.java Propchange: tomcat/trunk/java/org/apache/tomcat/dbcp/pool2/ ------------------------------------------------------------------------------ Merged /commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2:r1609324-1627022 Modified: tomcat/trunk/java/org/apache/tomcat/dbcp/pool2/impl/BaseGenericObjectPool.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/dbcp/pool2/impl/BaseGenericObjectPool.java?rev=1627027&r1=1627026&r2=1627027&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/dbcp/pool2/impl/BaseGenericObjectPool.java (original) +++ tomcat/trunk/java/org/apache/tomcat/dbcp/pool2/impl/BaseGenericObjectPool.java Tue Sep 23 13:43:21 2014 @@ -20,6 +20,7 @@ import java.io.PrintWriter; import java.io.StringWriter; import java.io.Writer; import java.lang.management.ManagementFactory; +import java.lang.ref.WeakReference; import java.util.Iterator; import java.util.TimerTask; import java.util.concurrent.atomic.AtomicLong; @@ -92,9 +93,10 @@ public abstract class BaseGenericObjectP /* * Class loader for evictor thread to use since in a J2EE or similar * environment the context class loader for the evictor thread may have - * visibility of the correct factory. See POOL-161. + * visibility of the correct factory. See POOL-161. Uses a weak reference to + * avoid potential memory leaks if the Pool is discarded rather than closed. */ - private final ClassLoader factoryClassLoader; + private final WeakReference<ClassLoader> factoryClassLoader; // Monitoring (primarily JMX) attributes @@ -111,7 +113,7 @@ public abstract class BaseGenericObjectP private final StatsStore waitTimes = new StatsStore(MEAN_TIMING_STATS_CACHE_SIZE); private final Object maxBorrowWaitTimeMillisLock = new Object(); private volatile long maxBorrowWaitTimeMillis = 0; // @GuardedBy("maxBorrowWaitTimeMillisLock") - private SwallowedExceptionListener swallowedExceptionListener = null; + private volatile SwallowedExceptionListener swallowedExceptionListener = null; /** @@ -135,7 +137,8 @@ public abstract class BaseGenericObjectP this.creationStackTrace = getStackTrace(new Exception()); // save the current CCL to be used later by the evictor Thread - factoryClassLoader = Thread.currentThread().getContextClassLoader(); + factoryClassLoader = + new WeakReference<>(Thread.currentThread().getContextClassLoader()); fairness = config.getFairness(); } @@ -586,7 +589,8 @@ public abstract class BaseGenericObjectP public final void setEvictionPolicyClassName( String evictionPolicyClassName) { try { - Class<?> clazz = Class.forName(evictionPolicyClassName); + Class<?> clazz = Class.forName(evictionPolicyClassName, true, + Thread.currentThread().getContextClassLoader()); Object policy = clazz.newInstance(); if (policy instanceof EvictionPolicy<?>) { @SuppressWarnings("unchecked") // safe, because we just checked the class @@ -995,8 +999,14 @@ public abstract class BaseGenericObjectP Thread.currentThread().getContextClassLoader(); try { // Set the class loader for the factory - Thread.currentThread().setContextClassLoader( - factoryClassLoader); + ClassLoader cl = factoryClassLoader.get(); + if (cl == null) { + // The pool has been dereferenced and the class loader GC'd. + // Cancel this timer so the pool can be GC'd as well. + cancel(); + return; + } + Thread.currentThread().setContextClassLoader(cl); // Evict from the pool try { Modified: tomcat/trunk/java/org/apache/tomcat/dbcp/pool2/impl/GenericKeyedObjectPool.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/dbcp/pool2/impl/GenericKeyedObjectPool.java?rev=1627027&r1=1627026&r2=1627027&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/dbcp/pool2/impl/GenericKeyedObjectPool.java (original) +++ tomcat/trunk/java/org/apache/tomcat/dbcp/pool2/impl/GenericKeyedObjectPool.java Tue Sep 23 13:43:21 2014 @@ -107,7 +107,7 @@ public class GenericKeyedObjectPool<K,T> setConfig(config); - startEvictor(getMinEvictableIdleTimeMillis()); + startEvictor(getTimeBetweenEvictionRunsMillis()); } /** @@ -350,8 +350,10 @@ public class GenericKeyedObjectPool<K,T> if (blockWhenExhausted) { p = objectDeque.getIdleObjects().pollFirst(); if (p == null) { - create = true; p = create(key); + if (p != null) { + create = true; + } } if (p == null) { if (borrowMaxWaitMillis < 0) { @@ -371,8 +373,10 @@ public class GenericKeyedObjectPool<K,T> } else { p = objectDeque.getIdleObjects().pollFirst(); if (p == null) { - create = true; p = create(key); + if (p != null) { + create = true; + } } if (p == null) { throw new NoSuchElementException("Pool exhausted"); @@ -930,8 +934,23 @@ public class GenericKeyedObjectPool<K,T> continue; } - if (evictionPolicy.evict(evictionConfig, underTest, - poolMap.get(evictionKey).getIdleObjects().size())) { + // User provided eviction policy could throw all sorts of crazy + // exceptions. Protect against such an exception killing the + // eviction thread. + boolean evict; + try { + evict = evictionPolicy.evict(evictionConfig, underTest, + poolMap.get(evictionKey).getIdleObjects().size()); + } catch (Throwable t) { + // Slightly convoluted as SwallowedExceptionListener uses + // Exception rather than Throwable + PoolUtils.checkRethrow(t); + swallowException(new Exception(t)); + // Don't evict on error conditions + evict = false; + } + + if (evict) { destroy(evictionKey, underTest, true); destroyedByEvictorCount.incrementAndGet(); } else { Modified: tomcat/trunk/java/org/apache/tomcat/dbcp/pool2/impl/GenericObjectPool.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/dbcp/pool2/impl/GenericObjectPool.java?rev=1627027&r1=1627026&r2=1627027&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/dbcp/pool2/impl/GenericObjectPool.java (original) +++ tomcat/trunk/java/org/apache/tomcat/dbcp/pool2/impl/GenericObjectPool.java Tue Sep 23 13:43:21 2014 @@ -429,8 +429,10 @@ public class GenericObjectPool<T> extend if (blockWhenExhausted) { p = idleObjects.pollFirst(); if (p == null) { - create = true; p = create(); + if (p != null) { + create = true; + } } if (p == null) { if (borrowMaxWaitMillis < 0) { @@ -450,8 +452,10 @@ public class GenericObjectPool<T> extend } else { p = idleObjects.pollFirst(); if (p == null) { - create = true; p = create(); + if (p != null) { + create = true; + } } if (p == null) { throw new NoSuchElementException("Pool exhausted"); @@ -775,8 +779,23 @@ public class GenericObjectPool<T> extend continue; } - if (evictionPolicy.evict(evictionConfig, underTest, - idleObjects.size())) { + // User provided eviction policy could throw all sorts of crazy + // exceptions. Protect against such an exception killing the + // eviction thread. + boolean evict; + try { + evict = evictionPolicy.evict(evictionConfig, underTest, + idleObjects.size()); + } catch (Throwable t) { + // Slightly convoluted as SwallowedExceptionListener uses + // Exception rather than Throwable + PoolUtils.checkRethrow(t); + swallowException(new Exception(t)); + // Don't evict on error conditions + evict = false; + } + + if (evict) { destroy(underTest); destroyedByEvictorCount.incrementAndGet(); } else { --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org