[
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)
