[jira] [Commented] (POOL-424) GenericObjectPool.invalidateObject() can leave other threads waiting to borrow hanging

["Phil Steitz (Jira)" <ji...@apache.org>]

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

Phil Steitz commented on POOL-424:
----------------------------------

Thanks for reporting this and providing the test.  Analysis looks correct to me.  ensureIdle will no-op if there is an idle instance available even when there is capacity to add.  To ensure liveness, we need to make sure the add happens if there is capacity in the pool. 

> GenericObjectPool.invalidateObject() can leave other threads waiting to borrow hanging
> --------------------------------------------------------------------------------------
>
>                 Key: POOL-424
>                 URL: https://issues.apache.org/jira/browse/POOL-424
>             Project: Commons Pool
>          Issue Type: Bug
>    Affects Versions: 2.12.1
>            Reporter: Steven Adams
>            Priority: Major
>         Attachments: PoolTest.java
>
>
> If multiple threads are waiting to borrow an object from a GenericObjectPool, and some other thread invalidates a number of existing objects already borrowed from the pool, the waiting threads can be left hanging waiting for new idle objects that never materialise.
> I've attached a simple standalone test program that demonstrates the issue.  It requires two runtime arguments: a test case (either '1' or '2') and a boolean value indicating if objects should be returned to the pool normally ('false') or invalidated instead ('true').
> The test uses a pool configured with a max size of 2.  Test 1 borrows 2 objects from the pool, then in the same thread either returns or invalidates them, then tries to borrow another 2 objects.
> Test 2 is similar, but starts two concurrent threads to each do the second set of borrows before the main thread returns or invalidates the first set of borrowed objects.
> Test 1 in either case, and test 2 in the "return normally" case all work as expected.  However test 2 in the invalidate case almost always fails with one of the borrow threads hanging until the borrow timeout expires:
> {noformat}
> $ java -classpath .:commons-pool2-2.12.1.jar PoolTest 2 true
> main: test-string-0
> main: test-string-1
> Thread-0: getting
> Thread-1: getting
> Thread-0: test-string-2
> Thread-0: got
> Exception in thread "Thread-1" 
> java.lang.RuntimeException: Exception in borrowObject()	at PoolTest.get(PoolTest.java:101)
>   at PoolTest.lambda$test2$0(PoolTest.java:71)
>   at java.base/java.lang.Thread.run(Thread.java:829)
> Caused by: java.util.NoSuchElementException: Timeout waiting for idle object, borrowMaxWaitDuration=PT4.999997S
>   at org.apache.commons.pool2.impl.GenericObjectPool.borrowObject(GenericObjectPool.java:309)
>   at org.apache.commons.pool2.impl.GenericObjectPool.borrowObject(GenericObjectPool.java:231)
>   at PoolTest.get(PoolTest.java:92)
>   ... 2 more
> {noformat}
> If the commented out code in the get() method at the end of the test class is enabled, to retry the borrow if a timeout occurs, then a new object is added to the pool then and able to be borrowed.  But this requires the timeout to expire first:
> {noformat}
> $ java -classpath .:commons-pool2-2.12.1.jar PoolTest 2 true
> main: test-string-0
> main: test-string-1
> Thread-0: getting
> Thread-1: getting
> Thread-0: test-string-2
> Thread-0: got
> failed to obtain object: java.util.NoSuchElementException: Timeout waiting for idle object, borrowMaxWaitDuration=PT4.999994S
> trying again
> Thread-1: test-string-3
> Thread-1: got{noformat}
> I believe the issue is the "ensureIdle(1, false)" call at the end of GenericObjectPool.invalidateObject(T, DestroyMode).  This only ensures that at most one idle object remains in the pool, regardless of how many threads are waiting on the pool.  In the test case, if a thread invalidates multiple objects in quick succession before waiting threads get a chance to wake up and take, then only one new object is created to replace all those that have been invalidated.
> The expected behaviour, in the test case, is that both waiting threads should be able to borrow a pooled object as soon as the existing objects have been invalidated.
>  



--
This message was sent by Atlassian Jira
(v8.20.10#820010)