[ https://issues.apache.org/jira/browse/POOL-396?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17366264#comment-17366264 ]
Phil Steitz commented on POOL-396: ---------------------------------- Sorry for the slow response. Really nice work on this [~jeremyk-91]. One small change (responding to your question above) is that I would just rethrow the original exception rather than wrapping in a NSEE. That makes sense on borrow, but here it's probably better to just propagate whatever was thrown by the factory. > Exceptions in validation can cause an object's lifecycle to be stuck in > EVICTION state > -------------------------------------------------------------------------------------- > > Key: POOL-396 > URL: https://issues.apache.org/jira/browse/POOL-396 > Project: Commons Pool > Issue Type: Bug > Affects Versions: 2.4.2, 2.9.0 > Reporter: Jeremy Kong > Priority: Major > Time Spent: 0.5h > Remaining Estimate: 0h > > Currently, if validation of an object throws an exception when testing an > idle object during eviction, an exception will be thrown from the evict() > method. This causes the object to be left in the EVICTION state, but still be > present in the idleObjects queue. > Future runs of the eviction thread will run into an infinite loop, because > they pick up the afflicted object from the queue, fail to set its state to > EVICTION, don't count it as a test, and repeat ad infinitum. This infinite > loop means that no further evictions will take place; it also causes > increases in garbage collection owing to allocation of the EvictionIterator > on each loop iteration. > This test added to TestGenericObjectPool (plus a small change allowing > exceptions to be thrown from the validateObject method of SimpleFactory) > reproduces the issue. > {code:java} > @Test > @Timeout(value = 60000, unit = TimeUnit.MILLISECONDS) > public void testExceptionInValidationDuringEviction() throws Exception { > genericObjectPool.setMaxIdle(1); > genericObjectPool.setMaxTotal(2); > genericObjectPool.setNumTestsPerEvictionRun(1); > genericObjectPool.setMinEvictableIdleTime(Duration.ofMillis(0)); > genericObjectPool.setTestWhileIdle(true); > String active = genericObjectPool.borrowObject(); > genericObjectPool.returnObject(active); > simpleFactory.setThrowExceptionOnValidate(true); > try { > genericObjectPool.evict(); > } catch (Exception e) { > // OK that a given run fails. But the object should still be evicted. > } > genericObjectPool.evict(); // currently fails: causes an infinite loop! > assertEquals(0, genericObjectPool.getNumActive()); > assertEquals(0, genericObjectPool.getNumIdle()); > } > {code} > It seems that protection against exceptions thrown by validation code exist > elsewhere (e.g. in borrowObject()); similarly, protections against exceptions > thrown by user eviction policy code exist in evict(). So it looks like the > testWhileIdle() portion of evict() needs to similarly be protected. > Thanks! > Jeremy -- This message was sent by Atlassian Jira (v8.3.4#803005)