2014-09-23 17:53 GMT+04:00 Phil Steitz <phil.ste...@gmail.com>: > On 9/23/14 6:04 AM, ma...@apache.org wrote: >> Author: markt >> Date: Tue Sep 23 13:04:52 2014 >> New Revision: 1626998 >> >> URL: http://svn.apache.org/r1626998 >> Log: >> Prevent potential memory leaks when the Pool is dereferenced without being >> closed. >> >> Modified: >> commons/proper/pool/trunk/src/changes/changes.xml >> >> commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java
(...) >> Modified: >> commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java >> URL: >> http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java?rev=1626998&r1=1626997&r2=1626998&view=diff >> ============================================================================== >> --- >> commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java >> (original) >> +++ >> commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java >> Tue Sep 23 13:04:52 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; >> @@ -94,9 +95,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 >> @@ -137,7 +139,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<ClassLoader>( >> + Thread.currentThread().getContextClassLoader()); I think that Thread.currentThread().getContextClassLoader() may return null if the pool is used in a standalone application. This will break the "if (cl == null)" check added to Evictor.run() method below. >> fairness = config.getFairness(); >> } >> >> @@ -251,7 +254,7 @@ public abstract class BaseGenericObjectP >> public final boolean getLifo() { >> return lifo; >> } >> - >> + >> /** >> * Returns whether or not the pool serves threads waiting to borrow >> objects fairly. >> * True means that waiting threads are served as if waiting in a FIFO >> queue. >> @@ -997,8 +1000,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 { >> >> >> Best regards, Konstantin Kolinko --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
