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 2014/09/23 15:43:21 UTC
svn commit: r1627027 - in /tomcat/trunk/java/org/apache/tomcat/dbcp/pool2:
./ impl/BaseGenericObjectPool.java impl/GenericKeyedObjectPool.java
impl/GenericObjectPool.java
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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org