You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Phil Steitz (JIRA)" <ji...@apache.org> on 2018/04/11 16:04:00 UTC
[jira] [Comment Edited] (POOL-337) EvictionTimer does not remove
cancelled tasks from the executor, leading to an IllegalStateException when
the evictor attempts to evict
[ https://issues.apache.org/jira/browse/POOL-337?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16433251#comment-16433251 ]
Phil Steitz edited comment on POOL-337 at 4/11/18 4:03 PM:
-----------------------------------------------------------
Sorry, I missed the unit test in the updated PR. In my testing I did not configure a listener to get the swallowed exceptions. I think the analysis is correct and the patch should fix the issue. It might be good to consider a little refactoring that would allow the GOP to hold a reference to the task that it needs to cancel (i.e. change the type of the Evictor) so you would not need to maintain the map in the EvictionTimer.
was (Author: psteitz):
Sorry, I missed the unit test in the updated PR. What I am still not getting is how the stack trace at the top can happen. I can see how the memory leak is a problem and I think there is definitely a bug here, but I would like to be able to reproduce this directly from the GOP API, which I can't do. The underlying Evictor task should get cancelled while the eviction lock is held, but somehow the executor is still firing it in the trace at the top.
> 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)