Fix for JIRA: POOL-425. Make addObject no-op when maxIdle is attained.

psteitz · psteitz · commit c5d63adfca0d · 2025-11-05T20:24:51.000-07:00
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
@@ -47,7 +47,8 @@ The <action> type attribute can be add,update,fix,remove.
   <body>
   <release version="2.13.0" date="YYYY-MM-DD" description="This is a feature and maintenance release. Java 8 or later is required.">
     <!-- FIX -->
-    <action type="fix" issue="POOL-350" dev="psteitz" due-to="Phil Steitz">Make placement of calls to GKOP reuseCapacity configurable.</action>
+    <action type="fix" issue="POOL-425" dev="psteitz">GenericObjectPool addObject does not respect maxIdle.</action>
+    <action type="fix" issue="POOL-350" dev="psteitz">Make placement of calls to GKOP reuseCapacity configurable.</action>
     <action type="fix" issue="POOL-290" dev="psteitz" due-to="Serge Angelov">TestSoftRefOutOfMemory (unit test) can loop infinitely on failure.</action>
     <action type="fix" issue="POOL-419" dev="psteitz" due-to="Raju Gupta, Phil Steitz">GenericObjectPool counters and object collections can be corrupted when returnObject and invalidate are invoked concurrently by client threads on the same pooled object.</action>
     <action type="fix" issue="POOL-421" dev="psteitz" due-to="Phil Steitz">GenericObjectPool addObject should return immediately when there is no capacity to add.</action>
diff --git a/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java b/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
@@ -193,10 +193,11 @@ private void addIdleObject(final PooledObject<T> p) throws Exception {
     }
 
     /**
-     * Creates an object, and place it into the pool. addObject() is useful for
+     * Creates an object, and places it into the pool. addObject() is useful for
      * "pre-loading" a pool with idle objects.
      * <p>
-     * If there is no capacity available to add to the pool, this is a no-op
+     * If there is no capacity available to add to the pool, or there are already
+     * {@link #getMaxIdle()} idle instances in the pool, this is a no-op
      * (no exception, no impact to the pool).
      * </p>
      * <p>
@@ -213,7 +214,8 @@ public void addObject() throws Exception {
         }
 
         final int localMaxTotal = getMaxTotal();
-        if (localMaxTotal < 0 || createCount.get() < localMaxTotal) {
+        final int localMaxIdle = getMaxIdle();
+        if (getNumIdle() < localMaxIdle && (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
@@ -750,6 +750,7 @@ private void checkEvictorVisiting(final boolean lifo) throws Exception {
                     trackerPool.setMaxIdle(-1);
                     final int instanceCount = 10 + random.nextInt(20);
                     trackerPool.setMaxTotal(instanceCount);
+                    trackerPool.setMaxIdle(instanceCount);
                     for (int k = 0; k < instanceCount; k++) {
                         trackerPool.addObject();
                     }
@@ -2980,4 +2981,32 @@ void testWhenExhaustedFail() throws Exception {
         assertEquals(1, genericObjectPool.getNumIdle());
         genericObjectPool.close();
     }
+
+    @Test
+    void testAddObjectRespectsMaxIdle() throws Exception {
+        genericObjectPool.setMaxIdle(1);
+        genericObjectPool.addObject();
+        genericObjectPool.addObject(); // should be no-op
+        assertEquals(1, genericObjectPool.getNumIdle());
+    }
+
+    @Test
+    void testAddObjectRespectsMaxTotal() throws Exception {
+        genericObjectPool.setMaxTotal(1);
+        genericObjectPool.addObject();
+        genericObjectPool.addObject(); // should be no-op
+        assertEquals(1, genericObjectPool.getNumIdle());
+    }
+
+    @Test
+    void testAddObjectCanAddToMaxIdle() throws Exception {
+        genericObjectPool.setMaxTotal(5);
+        genericObjectPool.borrowObject();
+        genericObjectPool.borrowObject();
+        genericObjectPool.setMaxIdle(3);
+        for (int i = 0; i < 3; i++) {
+            genericObjectPool.addObject();
+        }
+        assertEquals(3, genericObjectPool.getNumIdle());
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -193,10 +193,11 @@ private void addIdleObject(final PooledObject<T> p) throws Exception {`
`193`	`193`	`}`
`194`	`194`
`195`	`195`	`/**`
`196`		`- * Creates an object, and place it into the pool. addObject() is useful for`
	`196`	`+ * Creates an object, and places it into the pool. addObject() is useful for`
`197`	`197`	`* "pre-loading" a pool with idle objects.`
`198`	`198`	`* <p>`
`199`		`- * If there is no capacity available to add to the pool, this is a no-op`
	`199`	`+ * If there is no capacity available to add to the pool, or there are already`
	`200`	`+ * {@link #getMaxIdle()} idle instances in the pool, this is a no-op`
`200`	`201`	`* (no exception, no impact to the pool).`
`201`	`202`	`* </p>`
`202`	`203`	`* <p>`
`@@ -213,7 +214,8 @@ public void addObject() throws Exception {`
`213`	`214`	`}`
`214`	`215`
`215`	`216`	`final int localMaxTotal = getMaxTotal();`
`216`		`- if (localMaxTotal < 0 \|\| createCount.get() < localMaxTotal) {`
	`217`	`+ final int localMaxIdle = getMaxIdle();`
	`218`	`+ if (getNumIdle() < localMaxIdle && (localMaxTotal < 0 \|\| createCount.get() < localMaxTotal)) {`
`217`	`219`	`addIdleObject(create(getMaxWaitDuration()));`
`218`	`220`	`}`
`219`	`221`	`}`