[
https://issues.apache.org/jira/browse/POOL-315?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15594241#comment-15594241
]
Buğra Gedik commented on POOL-315:
----------------------------------
waitForCompletion() was just a suggestion in terms of what needs to be done. I
know that there is not such method. I don't know if I will have time in the
near feature to create a patch.
> GenericObjectPool close() does not wait for the current eviction task
> ---------------------------------------------------------------------
>
> Key: POOL-315
> URL: https://issues.apache.org/jira/browse/POOL-315
> Project: Commons Pool
> Issue Type: Bug
> Reporter: Buğra Gedik
>
> The {{close}} method is implemented as follows:
> {code}
> public void close() {
> if(!this.isClosed()) {
> Object var1 = this.closeLock;
> synchronized(this.closeLock) {
> if(!this.isClosed()) {
> this.startEvictor(-1L);
> this.closed = true;
> this.clear();
> this.jmxUnregister();
> this.idleObjects.interuptTakeWaiters();
> }
> }
> }
> }
> {code}
> The line {{this.startEvictor(-1L);}} calls
> {{EvictionTimer.cancel(this.evictor);}} from {{BaseGenericObjectPool}} and
> that in turn calls the following method in {{EvictionTimer}}:
> {code}
> static synchronized void cancel(TimerTask task) {
> task.cancel();
> --_usageCount;
> if(_usageCount == 0) {
> _timer.cancel();
> _timer = null;
> }
> }
> {code}
> Here, {{_timer}} is a {{java.util.TimerTask}}. If you look at the
> documentation of it, you'll see that it does not block on the currently
> executing task. Even though {{task.cancel()}} is called, there is no code to
> wait for it to complete. The end result is that, even if you close the pool,
> there may still be an eviction thread running. Yes, it will eventually go
> away, but in some rare cases it takes a bit of time to go away.
> When running code under Tomcat, this results in the
> {{"commons-pool-EvictionTimer"}} thread (created in the class
> {{EvictionTimer}}) to be reported as leaking, despite the pool being closed.
> This happens rarely, since most of the time the timer thread goes away before
> Tomcat checks for leaking threads.
> In my opinion a fix can be put into {{startEvictor}} in
> {{BaseGenericObjectPool}}:
> {code}
> final void startEvictor(long delay) {
> synchronized (evictionLock) {
> if (null != evictor) {
> EvictionTimer.cancel(evictor);
> // HERE: evictor.waitForCompletion();
> evictor = null;
> evictionIterator = null;
> }
> if (delay > 0) {
> evictor = new Evictor();
> EvictionTimer.schedule(evictor, delay, delay);
> }
> }
> }
> {code}
> Here is an example that illustrates the problem:
> {code:collapsible=false}
> import static org.junit.Assert.assertFalse;
> import org.apache.commons.pool2.PooledObject;
> import org.apache.commons.pool2.PooledObjectFactory;
> import org.apache.commons.pool2.impl.DefaultPooledObject;
> import org.apache.commons.pool2.impl.GenericObjectPool;
> import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
> import org.junit.Test;
> public class PoolTest
> {
> private static final CharSequence
> COMMONS_POOL_EVICTIONS_TIMER_THREAD_NAME =
> "commons-pool-EvictionTimer";
> private static final long EVICTION_PERIOD_IN_MILLIS = 100;
> private static class Foo
> {
> }
> private static class PooledFooFactory implements PooledObjectFactory<Foo>
> {
> private static final long VALIDATION_WAIT_IN_MILLIS = 1000;
> @Override
> public PooledObject<Foo> makeObject() throws Exception
> {
> return new DefaultPooledObject<>(new Foo());
> }
> @Override
> public void destroyObject(
> PooledObject<Foo> pooledObject) throws Exception
> {
> }
> @Override
> public boolean validateObject(
> PooledObject<Foo> pooledObject)
> {
> try
> {
> Thread.sleep(VALIDATION_WAIT_IN_MILLIS);
> }
> catch (final InterruptedException e)
> {
> Thread.interrupted();
> }
> return false;
> }
> @Override
> public void activateObject(
> PooledObject<Foo> pooledObject) throws Exception
> {
> }
> @Override
> public void passivateObject(
> PooledObject<Foo> pooledObject) throws Exception
> {
> }
> }
> @Test
> public void testPool() throws Exception
> {
> final GenericObjectPoolConfig poolConfig =
> new GenericObjectPoolConfig();
> poolConfig.setTestWhileIdle(true /* testWhileIdle */);
> final PooledFooFactory pooledFooFactory = new PooledFooFactory();
> GenericObjectPool<Foo> pool = null;
> try
> {
> pool = new GenericObjectPool<>(pooledFooFactory, poolConfig);
> pool.setTimeBetweenEvictionRunsMillis(EVICTION_PERIOD_IN_MILLIS);
> pool.addObject();
> try
> {
> Thread.sleep(EVICTION_PERIOD_IN_MILLIS);
> }
> catch (final InterruptedException e)
> {
> Thread.interrupted();
> }
> }
> finally
> {
> if (pool != null)
> {
> pool.close();
> }
> }
> final Thread[] threads = new Thread[Thread.activeCount()];
> Thread.enumerate(threads);
> for (final Thread thread : threads)
> {
> if (thread == null)
> {
> continue;
> }
> assertFalse(thread.getName()
> .contains(COMMONS_POOL_EVICTIONS_TIMER_THREAD_NAME));
> }
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
