Mailing-List: contact issues-help@commons.apache.org; run by ezmlm
Precedence: bulk
Reply-To: issues@commons.apache.org
Date: Thu, 20 Oct 2016 10:19:58 +0000 (UTC)
From: =?utf-8?Q?Bu=C4=9Fra_Gedik_=28JIRA=29?= <jira@apache.org>
To: issues@commons.apache.org
Message-ID: <JIRA.13013167.1476795796000.38240.1476958798476@Atlassian.JIRA>
In-Reply-To: <JIRA.13013167.1476795796000@Atlassian.JIRA>
References: <JIRA.13013167.1476795796000@Atlassian.JIRA> <JIRA.13013167.1476795796527@arcas>
Subject: [jira] [Commented] (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: Thu, 20 Oct 2016 10:20:00 -0000


    [ https://issues.apache.org/jira/browse/POOL-315?page=3Dcom.atlassian.j=
ira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=3D155914=
33#comment-15591433 ]=20

Bu=C4=9Fra Gedik commented on POOL-315:
----------------------------------

{code}
package com.unscrambl.drive;

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_N=
AME =3D
        "commons-pool-EvictionTimer";
    private static final long EVICTION_PERIOD_IN_MILLIS =3D 100;

    private static class Foo
    {
    }

    private static class PooledFooFactory implements PooledObjectFactory<Fo=
o>
    {
        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_MILLIS=
);
            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}

> 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
>
> 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}


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