[
https://issues.apache.org/jira/browse/POOL-386?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17156586#comment-17156586
]
Mark Thomas commented on POOL-386:
----------------------------------
Sorry, just re-read my previous comment and wanted to make clear that the first
sentence was a genuine question along the lines of "Do you have any preliminary
ideas of how you might use GC to do this?" rather than trying to suggest the
idea has no merit along the lines of "Have you lost the plot?"
> Closing a pool can cause Evictor in another pool to be cancelled
> ----------------------------------------------------------------
>
> Key: POOL-386
> URL: https://issues.apache.org/jira/browse/POOL-386
> Project: Commons Pool
> Issue Type: Task
> Affects Versions: 2.6.1, 2.6.2, 2.7.0, 2.8.0
> Reporter: Phil Steitz
> Priority: Major
> Fix For: 2.8.1
>
>
> The fix for POOL-337 introduced a race condition that can cause the shared
> EvictionTimer to be cancelled when a pool is closed but another pool still
> has an active Evictor. The EvictionTimer cancel method used to cancel an
> eviction task owned by a client pool uses this test to determine whether or
> not to shutdown its executor:
> {code:java}
> if (executor != null && executor.getQueue().isEmpty()){code}
> The executor may report an empty queue if it is executing a task. This will
> cause the executor to be shut down and scheduling of new tasks to stop.
> The unit test below illustrates the problem.
> {code:java}
>
> public void testEvictionTimerMultiplePools() throws InterruptedException {
> final AtomicIntegerFactory factory = new AtomicIntegerFactory();
> factory.setValidateLatency(50);
> final GenericObjectPool<AtomicInteger> evictingPool = new
> GenericObjectPool<>(factory);
> evictingPool.setTimeBetweenEvictionRunsMillis(100);
> evictingPool.setNumTestsPerEvictionRun(5);
> evictingPool.setTestWhileIdle(true);
> evictingPool.setMinEvictableIdleTimeMillis(50);
> for (int i = 0; i < 10; i++) {
> try {
> evictingPool.addObject();
> } catch (Exception e) {
> e.printStackTrace();
> }
> }
> for (int i = 0; i < 1000; i++) {
> GenericObjectPool<AtomicInteger> nonEvictingPool = new
> GenericObjectPool<>(factory);
> nonEvictingPool.close();
> }
> Thread.sleep(1000);
> Assert.assertEquals(0, evictingPool.getNumIdle());
> evictingPool.close();
> }
> {code}
> Proposed fix:
> Change the test guarding executor shutdown to
> {code:java}
> executor.getQueue().isEmpty() && executor.getActiveCount() == 0
> {code}
> This still exposes a low-probability race - a task completes after the
> isEmpty test and is requeued before the activecount test - but this is very
> unlikely.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)