[
https://issues.apache.org/jira/browse/POOL-337?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16481790#comment-16481790
]
Phil Steitz edited comment on POOL-337 at 5/20/18 4:59 PM:
-----------------------------------------------------------
Let me start with a disclaimer here. I have not committed to pool in a while
and I have not worked out the details of this solution.
That said, you are right, I meant `BaseGenericObjectPool` and basically what
you describe, changing
{code:java}
evictor = new Evictor();
EvictionTimer.schedule(evictor, delay, delay);
{code}
in BGOP.startEvictor to
{code:java}
evictorFuture = EvictionTimer.schedule(evictor, delay, delay);
{code}
so GOP instances hold references to the ScheduledFutures. You are right that
that requires a change to the `EvictionTimer`
API but the method(s) in question are package scope, so not part of the public
API, so probably OK to change in a minor release.
Repeating disclaimer, I have not actually tried this or fully thought through
the consequences. It would be good to get [~markt] to comment on this, as IIRC
he designed the current setup.
was (Author: psteitz):
Let me start with a disclaimer here. I have not committed to pool in a while
and I have not worked out the details of this solution.
That said, you are right, I meant `BaseGenericObjectPool` and basically what
you describe, changing
{code:java}
evictor = new Evictor();
EvictionTimer.schedule(evictor, delay, delay);
{code}
in BGOP.startEvictor to
{code:java}
evictor = EvictionTimer.schedule(delay, delay);
{code}
hence changing the type of the evictor. You are right that that requires a
change to the `EvictionTimer`
API but the method(s) in question are package scope, so not part of the public
API, so probably OK to change in a minor release. Actually, it is just adding
new methods, but it does effectively obsolete the other ones.
Repeating disclaimer, I have not actually tried this or fully thought through
the consequences. It would be good to get [~markt] to comment on this, as IIRC
he designed the current setup.
> 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)
