From 9aa82ee2c6f847af9858b784fcc38f3849997ce3 Mon Sep 17 00:00:00 2001 From: baldwk <47660099+baldwk@users.noreply.github.com> Date: Mon, 28 Sep 2026 10:28:42 +0800 Subject: [PATCH 1/2] [POOL-420] Stop creation-capacity waits when a positive budget expires Exit the per-key creation wait when its initially positive budget is exhausted, using the existing path to release the maxTotal reservation. Leave zero and negative duration handling unchanged. Add a latch-controlled regression that requires a waiting borrower to finish before the in-flight creation completes and checks global capacity is available to another key after the timeout. --- .../pool2/impl/GenericKeyedObjectPool.java | 4 +- .../impl/TestGenericKeyedObjectPool.java | 51 +++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java b/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java index c9f4a177e..f67205c2d 100644 --- a/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java +++ b/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java @@ -860,7 +860,9 @@ private PooledObject create(final K key, final Duration maxWaitDuration) thro // bring the pool to capacity. Those calls might also // fail so wait until they complete and then re-test if // the pool is at capacity or not. - if (!remainingWaitDuration.isNegative()) { + if (PoolImplUtils.isPositive(maxWaitDuration) && !PoolImplUtils.isPositive(remainingWaitDuration)) { + create = Boolean.FALSE; + } else if (!remainingWaitDuration.isNegative()) { objectDeque.makeObjectCountLock.wait(remainingWaitDuration.toMillis(), remainingWaitDuration.getNano() % 1_000_000); } diff --git a/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java b/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java index 9f6e687ff..1d526db96 100644 --- a/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java +++ b/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java @@ -40,6 +40,7 @@ import java.util.Timer; import java.util.TimerTask; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -2105,6 +2106,56 @@ void testMaxWaitMultiThreaded() throws Exception { } + @Test + @Timeout(10) + void testMaxWaitDuringConcurrentCreation() throws Exception { + final CountDownLatch creationStarted = new CountDownLatch(1); + final CountDownLatch finishCreation = new CountDownLatch(1); + final AtomicInteger creationCount = new AtomicInteger(); + final BaseKeyedPooledObjectFactory factory = new BaseKeyedPooledObjectFactory() { + @Override + public String create(final String key) throws InterruptedException { + creationCount.incrementAndGet(); + if ("a".equals(key)) { + creationStarted.countDown(); + finishCreation.await(); + } + return key; + } + + @Override + public PooledObject wrap(final String value) { + return new DefaultPooledObject<>(value); + } + }; + final ExecutorService executor = Executors.newFixedThreadPool(2, new DaemonThreadFactory()); + try (GenericKeyedObjectPool pool = new GenericKeyedObjectPool<>(factory)) { + pool.setMaxTotal(2); + pool.setMaxTotalPerKey(1); + pool.setMaxWait(Duration.ofMinutes(1)); + final Future creator = executor.submit(() -> pool.borrowObject("a")); + try { + assertTrue(creationStarted.await(2, TimeUnit.SECONDS)); + final Future borrower = executor.submit(() -> pool.borrowObject("a", Duration.ofMillis(100))); + final ExecutionException exception = assertThrows(ExecutionException.class, () -> borrower.get(2, TimeUnit.SECONDS)); + assertTrue(exception.getCause() instanceof NoSuchElementException); + assertFalse(creator.isDone(), "The waiting borrower must time out before the in-flight creation completes"); + assertEquals(1, creationCount.get(), "The timed-out borrower must not create another object"); + // A timed-out creation attempt must release its reservation against maxTotal. + final String other = pool.borrowObject("b", Duration.ZERO); + assertEquals("b", other); + pool.returnObject("b", other); + } finally { + finishCreation.countDown(); + pool.returnObject("a", creator.get(2, TimeUnit.SECONDS)); + } + } finally { + finishCreation.countDown(); + executor.shutdownNow(); + assertTrue(executor.awaitTermination(2, TimeUnit.SECONDS)); + } + } + /** * JIRA: POOL-420 (clone of POOL-418 for GKOP) * From eeb23ed018a94b3e54e77a968eb2cb9d5d70b256 Mon Sep 17 00:00:00 2001 From: baldwk <47660099+baldwk@users.noreply.github.com> Date: Mon, 28 Sep 2026 14:34:25 +0800 Subject: [PATCH 2/2] [POOL-420] Reproduce repeated connection creation failures Replace the single blocked factory call with six concurrent borrowers. Each creation attempt fails after three seconds, with maxWait set to ten seconds. Verify that queued borrowers time out and release their pool reservations. --- .../impl/TestGenericKeyedObjectPool.java | 68 +++++++++++-------- 1 file changed, 40 insertions(+), 28 deletions(-) diff --git a/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java b/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java index 1d526db96..a8e660b69 100644 --- a/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java +++ b/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java @@ -27,6 +27,7 @@ import static org.junit.jupiter.api.Assertions.fail; import java.lang.management.ManagementFactory; +import java.net.SocketTimeoutException; import java.time.Duration; import java.time.Instant; import java.util.ArrayList; @@ -2107,18 +2108,19 @@ void testMaxWaitMultiThreaded() throws Exception { @Test - @Timeout(10) - void testMaxWaitDuringConcurrentCreation() throws Exception { - final CountDownLatch creationStarted = new CountDownLatch(1); - final CountDownLatch finishCreation = new CountDownLatch(1); + @Timeout(30) + void testMaxWaitDuringRepeatedCreationFailures() throws Exception { + final int borrowerCount = 6; + final CountDownLatch borrowersReady = new CountDownLatch(borrowerCount); + final CountDownLatch startBorrowing = new CountDownLatch(1); final AtomicInteger creationCount = new AtomicInteger(); final BaseKeyedPooledObjectFactory factory = new BaseKeyedPooledObjectFactory() { @Override - public String create(final String key) throws InterruptedException { - creationCount.incrementAndGet(); + public String create(final String key) throws Exception { if ("a".equals(key)) { - creationStarted.countDown(); - finishCreation.await(); + creationCount.incrementAndGet(); + Thread.sleep(3000); + throw new SocketTimeoutException("Simulated connection timeout"); } return key; } @@ -2128,31 +2130,41 @@ public PooledObject wrap(final String value) { return new DefaultPooledObject<>(value); } }; - final ExecutorService executor = Executors.newFixedThreadPool(2, new DaemonThreadFactory()); + final ExecutorService executor = Executors.newFixedThreadPool(borrowerCount, new DaemonThreadFactory()); try (GenericKeyedObjectPool pool = new GenericKeyedObjectPool<>(factory)) { - pool.setMaxTotal(2); + pool.setMaxTotal(borrowerCount); pool.setMaxTotalPerKey(1); - pool.setMaxWait(Duration.ofMinutes(1)); - final Future creator = executor.submit(() -> pool.borrowObject("a")); - try { - assertTrue(creationStarted.await(2, TimeUnit.SECONDS)); - final Future borrower = executor.submit(() -> pool.borrowObject("a", Duration.ofMillis(100))); - final ExecutionException exception = assertThrows(ExecutionException.class, () -> borrower.get(2, TimeUnit.SECONDS)); - assertTrue(exception.getCause() instanceof NoSuchElementException); - assertFalse(creator.isDone(), "The waiting borrower must time out before the in-flight creation completes"); - assertEquals(1, creationCount.get(), "The timed-out borrower must not create another object"); - // A timed-out creation attempt must release its reservation against maxTotal. - final String other = pool.borrowObject("b", Duration.ZERO); - assertEquals("b", other); - pool.returnObject("b", other); - } finally { - finishCreation.countDown(); - pool.returnObject("a", creator.get(2, TimeUnit.SECONDS)); + pool.setMaxWait(Duration.ofSeconds(10)); + final List> borrowers = new ArrayList<>(); + for (int i = 0; i < borrowerCount; i++) { + borrowers.add(executor.submit(() -> { + borrowersReady.countDown(); + startBorrowing.await(); + return pool.borrowObject("a"); + })); } + assertTrue(borrowersReady.await(5, TimeUnit.SECONDS)); + startBorrowing.countDown(); + int poolTimeouts = 0; + for (final Future borrower : borrowers) { + final ExecutionException exception = assertThrows(ExecutionException.class, borrower::get); + if (exception.getCause() instanceof NoSuchElementException) { + poolTimeouts++; + } else { + assertTrue(exception.getCause() instanceof SocketTimeoutException); + } + } + assertTrue(creationCount.get() > 1, "Multiple borrowers must attempt creation"); + assertTrue(poolTimeouts > 0, "Borrowers waiting for a creation slot must time out"); + // Failed and timed-out borrows must release their reservations against maxTotal. + pool.setMaxTotal(1); + final String other = pool.borrowObject("b", Duration.ZERO); + assertEquals("b", other); + pool.returnObject("b", other); } finally { - finishCreation.countDown(); + startBorrowing.countDown(); executor.shutdownNow(); - assertTrue(executor.awaitTermination(2, TimeUnit.SECONDS)); + assertTrue(executor.awaitTermination(5, TimeUnit.SECONDS)); } }