Hello Mark, I am not shure about the weak reference to the classloader. What is the idea behind asuming that the pool is dereferenced when the class loader goes? I can imagine there are cases where the class loader for the factory is no longer used (as it was not used to load the factory for example) and the pool is still referenced. Would this lead to a unexpected cancel()?
Gruss Bernd Am Tue, 23 Sep 2014 13:04:53 -0000 schrieb ma...@apache.org: > 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/changes/changes.xml > URL: > http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/changes/changes.xml?rev=1626998&r1=1626997&r2=1626998&view=diff > ============================================================================== > --- commons/proper/pool/trunk/src/changes/changes.xml (original) +++ > commons/proper/pool/trunk/src/changes/changes.xml Tue Sep 23 13:04:52 > 2014 @@ -80,6 +80,10 @@ The <action> type attribute can be add,u > <action dev="ggregory" type="update" issue="POOL-274"> Update > asm-util to 5.0.3 from 4.0. </action> > + <action dev="markt" type="fix"> > + Prevent potential memory leaks when the Pool is dereferenced > without being > + closed. > + </action> > </release> > <release version="2.2" date="2014-02-24" description= > "This is a maintenance release that adds a new testOnCreate > configuration option > > 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()); > 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 { > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
