On 24/09/2014 10:49, Konstantin Kolinko wrote:
> 2014-09-23 17:53 GMT+04:00 Phil Steitz <[email protected]>:
>> On 9/23/14 6:04 AM, [email protected] 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.
I did some quick checking. The only way I can get that to return null is
if I use:
Thread.currentThread().setContextClassLoader(null);
I can't think of a valid reason for doing that but I have got a patch
that will protect against this so, give the wide range of code Pool gets
used in, I'll add that protection.
Mark
>
>>> 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: [email protected]
> For additional commands, e-mail: [email protected]
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
