Mailing-List: contact issues-help@commons.apache.org; run by ezmlm
Precedence: bulk
Reply-To: issues@commons.apache.org
Date: Mon, 14 Nov 2016 07:23:58 +0000 (UTC)
From: "KeiichiFujino (JIRA)" <jira@apache.org>
To: issues@commons.apache.org
Message-ID: <JIRA.13013167.1476795796000.266545.1479108238353@Atlassian.JIRA>
In-Reply-To: <JIRA.13013167.1476795796000@Atlassian.JIRA>
References: <JIRA.13013167.1476795796000@Atlassian.JIRA> <JIRA.13013167.1476795796527@arcas>
Subject: [jira] [Updated] (POOL-315) GenericObjectPool close() does not wait
 for the current eviction task
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
archived-at: Mon, 14 Nov 2016 07:24:00 -0000


     [ https://issues.apache.org/jira/browse/POOL-315?page=3Dcom.atlassian.=
jira.plugin.system.issuetabpanels:all-tabpanel ]

KeiichiFujino updated POOL-315:
-------------------------------
    Attachment: BaseGenericObjectPool.txt

I think this fix is not completed.
It seems that the evictorShutdownTimeoutMillis has not been passed to the E=
victionTimer.
I attached 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=C4=9Fra Gedik
>             Fix For: 2.4.3
>
>         Attachments: BaseGenericObjectPool.txt
>
>
> The {{close}} method is implemented as follows:
> {code}
> public void close() {
>         if(!this.isClosed()) {
>             Object var1 =3D this.closeLock;
>             synchronized(this.closeLock) {
>                 if(!this.isClosed()) {
>                     this.startEvictor(-1L);
>                     this.closed =3D true;
>                     this.clear();
>                     this.jmxUnregister();
>                     this.idleObjects.interuptTakeWaiters();
>                 }
>             }
>         }
>     }
> {code}
> The line {{this.startEvictor(-1L);}} calls {{EvictionTimer.cancel(this.ev=
ictor);}} from {{BaseGenericObjectPool}} and that in turn calls the followi=
ng method in {{EvictionTimer}}:
> {code}
>     static synchronized void cancel(TimerTask task) {
>         task.cancel();
>         --_usageCount;
>         if(_usageCount =3D=3D 0) {
>             _timer.cancel();
>             _timer =3D null;
>         }
>     }
> {code}
> Here, {{_timer}} is a {{java.util.TimerTask}}. If you look at the documen=
tation 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, b=
ut in some rare cases it takes a bit of time to go away.
> When running code under Tomcat, this results in the {{"commons-pool-Evict=
ionTimer"}} 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 thr=
eads.
> In my opinion a fix can be put into {{startEvictor}} in {{BaseGenericObje=
ctPool}}:
> {code}
>     final void startEvictor(long delay) {
>         synchronized (evictionLock) {
>             if (null !=3D evictor) {
>                 EvictionTimer.cancel(evictor);
>                 // HERE: evictor.waitForCompletion();
>                 evictor =3D null;
>                 evictionIterator =3D null;
>             }
>             if (delay > 0) {
>                 evictor =3D new Evictor();
>                 EvictionTimer.schedule(evictor, delay, delay);
>             }
>         }
>     }
> {code}
> Here is an example that illustrates the problem:
> {code:collapsible=3Dfalse}
> 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 =3D
>         "commons-pool-EvictionTimer";
>     private static final long EVICTION_PERIOD_IN_MILLIS =3D 100;
>     private static class Foo
>     {
>     }
>     private static class PooledFooFactory implements PooledObjectFactory<=
Foo>
>     {
>         private static final long VALIDATION_WAIT_IN_MILLIS =3D 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 =3D
>             new GenericObjectPoolConfig();
>         poolConfig.setTestWhileIdle(true /* testWhileIdle */);
>         final PooledFooFactory pooledFooFactory =3D new PooledFooFactory(=
);
>         GenericObjectPool<Foo> pool =3D null;
>         try
>         {
>             pool =3D new GenericObjectPool<>(pooledFooFactory, poolConfig=
);
>             pool.setTimeBetweenEvictionRunsMillis(EVICTION_PERIOD_IN_MILL=
IS);
>             pool.addObject();
>             try
>             {
>                 Thread.sleep(EVICTION_PERIOD_IN_MILLIS);
>             }
>             catch (final InterruptedException e)
>             {
>                Thread.interrupted();
>             }
>         }
>         finally
>         {
>             if (pool !=3D null)
>             {
>                 pool.close();
>             }
>         }
>         final Thread[] threads =3D new Thread[Thread.activeCount()];
>         Thread.enumerate(threads);
>         for (final Thread thread : threads)
>         {
>             if (thread =3D=3D null)
>             {
>                 continue;
>             }
>             assertFalse(thread.getName()
>                 .contains(COMMONS_POOL_EVICTIONS_TIMER_THREAD_NAME));
>         }
>     }
> }
> {code}


--
This message was sent by Atlassian JIRA
(v6.3.4#6332)