[jira] [Commented] (POOL-337) EvictionTimer does not remove cancelled tasks from the executor, leading to an IllegalStateException when the evictor attempts to evict

["ASF GitHub Bot (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/POOL-337?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16385833#comment-16385833 ] 

ASF GitHub Bot commented on POOL-337:
-------------------------------------

GitHub user cdeneux opened a pull request:

    https://github.com/apache/commons-pool/pull/4

    POOL-337: EvictionTimer does not remove cancelled tasks

    EvictionTimer does not remove cancelled tasks from the executor, and so a memory leak occurs.
    
    Thanks to Reinald Verheij for the patch

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/petalslink/commons-pool POOL-337

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/commons-pool/pull/4.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #4
    
----
commit ec8b7769585bcb86a42a53a2d939c271194ebe9d
Author: cdeneux <ch...@...>
Date:   2018-02-28T17:25:06Z

    POOL-337: EvictionTimer does not remove cancelled tasks from the executor

----


> EvictionTimer does not remove cancelled tasks from the executor, leading to an IllegalStateException when the evictor attempts to evict
> ---------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: POOL-337
>                 URL: https://issues.apache.org/jira/browse/POOL-337
>             Project: Commons Pool
>          Issue Type: Bug
>    Affects Versions: 2.5.0
>            Reporter: Reinald Verheij
>            Priority: Major
>         Attachments: EvictionTimer.java, EvictionTimer.java.original-from-2.5.0.java
>
>
> EvictionTimer does not remove cancelled tasks from the executor, leading to an IllegalStateException when the evictor attempts to evict.
>  
> EvictionTimer::schedule() adds eviction tasks to the executor, but the cancel does not remove it. This is asymmetric and leads to the following exception:
> {noformat}
> java.lang.IllegalStateException: Pool not open
> 	at org.apache.commons.pool2.impl.BaseGenericObjectPool.assertOpen(BaseGenericObjectPool.java:713)
> 	at org.apache.commons.pool2.impl.GenericObjectPool.evict(GenericObjectPool.java:721)
> 	at org.apache.commons.pool2.impl.BaseGenericObjectPool$Evictor.run(BaseGenericObjectPool.java:1077)
> 	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
> 	at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:308)
> 	at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:180)
> 	at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:294)
> 	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
> 	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
> 	at java.lang.Thread.run(Thread.java:748){noformat}
> I think the cancel would need to remember the future which returned from {{executor::scheduleWithFixedDelay()}} in {{schedule()}} and then do something like this (see  [^EvictionTimer.java] compared to original  [^EvictionTimer.java.original-from-2.5.0.java] )
> {code:java}
> if (futures.containsKey(task)) {
>     futures.remove(task).cancel(false);
>     executor.purge();
> }{code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)