This is an automated email from the ASF dual-hosted git repository.
psteitz pushed a commit to branch pool-427-426-branch
in repository https://gitbox.apache.org/repos/asf/commons-pool.git
The following commit(s) were added to refs/heads/pool-427-426-branch by this
push:
new 35f3b638 Fix regression in fix for POOL-425, fix race in POOL-426.
35f3b638 is described below
commit 35f3b638af64970beb664c4cf6093dde7bcd4579
Author: Phil Steitz <[email protected]>
AuthorDate: Thu Dec 25 15:39:44 2025 -0700
Fix regression in fix for POOL-425, fix race in POOL-426.
---
src/changes/changes.xml | 4 +-
.../commons/pool2/impl/GenericObjectPool.java | 15 ++--
.../commons/pool2/impl/TestGenericObjectPool.java | 83 ++++++++++++++++++++++
3 files changed, 96 insertions(+), 6 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index a7339d1c..db69a0bc 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -45,8 +45,10 @@ The <action> type attribute can be add,update,fix,remove.
<title>Apache Commons Pool Release Notes</title>
</properties>
<body>
- <release version="" date="YYYY-MM-DD" description="This is a feature and
maintenance release. Java 8 or later is required.">
+ <release version="2.13.1" date="YYYY-MM-DD" description="This is a feature
and maintenance release. Java 8 or later is required.">
<!-- FIX -->
+ <action type="fix" issue="POOL-427" dev="psteitz" due-to="Raju Gupta">
The fix for POOL-425 introduced a regression where addObject fails when maxIdle
is negative (indicating no limit).</action>
+ <action type="fix" issue="POOL-426" dev="psteitz" due-to="Raju
Gupta">GenericObjectPool addObject can cause maxIdle to be exceeded</action>
<!-- ADD -->
<!-- UPDATE -->
</release>
diff --git a/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
b/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
index 07ab9af7..78be17e4 100644
--- a/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
+++ b/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
@@ -212,11 +212,16 @@ public class GenericObjectPool<T> extends
BaseGenericObjectPool<T>
if (factory == null) {
throw new IllegalStateException("Cannot add objects without a
factory.");
}
-
- final int localMaxTotal = getMaxTotal();
- final int localMaxIdle = getMaxIdle();
- if (getNumIdle() < localMaxIdle && (localMaxTotal < 0 ||
createCount.get() < localMaxTotal)) {
- addIdleObject(create(getMaxWaitDuration()));
+ synchronized (makeObjectCountLock) {
+ final int localMaxTotal = getMaxTotal();
+ final int localMaxIdle = getMaxIdle();
+ if (localMaxIdle >= 0 && getNumIdle() >= localMaxIdle) {
+ // Pool already has maxIdle idle objects or maxIdle is 0.
+ return;
+ }
+ if (localMaxTotal < 0 || createCount.get() < localMaxTotal) {
+ addIdleObject(create(getMaxWaitDuration()));
+ }
}
}
diff --git
a/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
b/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
index 21b41700..bc2d1238 100644
--- a/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
+++ b/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
@@ -3065,4 +3065,87 @@ class TestGenericObjectPool extends TestBaseObjectPool {
assertEquals(1, genericObjectPool.getNumIdle());
genericObjectPool.close();
}
+
+ @Test
+ void testAddObjectNegativeMaxTotal() throws Exception {
+ genericObjectPool.setMaxTotal(-1);
+ genericObjectPool.addObject();
+ assertEquals(1, genericObjectPool.getNumIdle());
+ }
+
+ @Test
+ void testAddObjectNegativeMaxIdle() throws Exception {
+ genericObjectPool.setMaxIdle(-1);
+ genericObjectPool.addObject();
+ assertEquals(1, genericObjectPool.getNumIdle());
+ }
+
+ @Test
+ void testAddObjectZeroMaxIdle() throws Exception {
+ genericObjectPool.setMaxIdle(0);
+ genericObjectPool.addObject();
+ assertEquals(0, genericObjectPool.getNumIdle());
+ }
+
+ @Test
+ @Timeout(value = 60000, unit = TimeUnit.MILLISECONDS)
+ void testAddObjectRespectsMaxIdleLimit() throws Exception {
+ genericObjectPool.setMaxIdle(1);
+ genericObjectPool.addObject();
+ genericObjectPool.addObject();
+ assertEquals(1, genericObjectPool.getNumIdle(), "should be one idle");
+
+ genericObjectPool.setMaxIdle(-1);
+ genericObjectPool.addObject();
+ genericObjectPool.addObject();
+ genericObjectPool.addObject();
+ assertEquals(4, genericObjectPool.getNumIdle(), "should be four idle");
+ }
+
+ @Test
+ @Timeout(value = 60000, unit = TimeUnit.MILLISECONDS)
+ void testAddObjectConcurrentCallsRespectsMaxIdle() throws Exception {
+ final int maxIdleLimit = 5;
+ final int numThreads = 10;
+ genericObjectPool.setMaxIdle(maxIdleLimit);
+
+ final CountDownLatch startLatch = new CountDownLatch(1);
+ final CountDownLatch doneLatch = new CountDownLatch(numThreads);
+
+ final List<Runnable> tasks = getRunnables(numThreads, startLatch,
doneLatch);
+
+ final ExecutorService executorService =
Executors.newFixedThreadPool(numThreads);
+ tasks.forEach(executorService::submit);
+ try {
+ startLatch.countDown(); // Start all threads simultaneously
+ doneLatch.await(); // Wait for all threads to complete
+ } finally {
+ executorService.shutdown();
+ assertTrue(executorService.awaitTermination(Long.MAX_VALUE,
TimeUnit.NANOSECONDS));
+ }
+
+ assertTrue(genericObjectPool.getNumIdle() <= maxIdleLimit,
+ "Concurrent addObject() calls should not exceed maxIdle limit of "
+ maxIdleLimit +
+ ", but found " + genericObjectPool.getNumIdle() + " idle
objects");
+ }
+
+ private List<Runnable> getRunnables(final int numThreads,
+ final CountDownLatch startLatch,
+ final CountDownLatch doneLatch) {
+ final List<Runnable> tasks = new ArrayList<>();
+
+ for (int i = 0; i < numThreads; i++) {
+ tasks.add(() -> {
+ try {
+ startLatch.await(); // Wait for all threads to be ready
+ genericObjectPool.addObject();
+ } catch (Exception e) {
+ Thread.currentThread().interrupt(); // Restore interrupt
status
+ } finally {
+ doneLatch.countDown();
+ }
+ });
+ }
+ return tasks;
+ }
}
