POOL-425 addObject() should respect maxIdle property.

Author: raju.gupta

src/main/java/org/apache/commons/pool3/impl/GenericObjectPool.java

@@ -513,6 +513,11 @@ private PooledObject<T> create(final Duration maxWaitDurationRequest) throws E {
         if (localMaxTotal < 0) {
             localMaxTotal = Integer.MAX_VALUE;
         }
+        int localMaxIdle = getMaxIdle();
+        if (localMaxIdle < 0) {
+            localMaxIdle = Integer.MAX_VALUE;
+        }
+        final int maxCapacity = Math.min(localMaxTotal, localMaxIdle);
         final Instant localStartInstant = Instant.now();
         // Flag that indicates if create should:
         // - TRUE:  call the factory to create an object
@@ -525,7 +530,7 @@ private PooledObject<T> create(final Duration maxWaitDurationRequest) throws E {
             final Duration remainingWaitDuration = maxWaitDuration.minus(durationSince(startInstant));
             synchronized (makeObjectCountLock) {
                 final long newCreateCount = createCount.incrementAndGet();
-                if (newCreateCount > localMaxTotal) {
+                if (newCreateCount > maxCapacity) {
                     // The pool is currently at capacity or in the process of
                     // making enough new objects to take it to capacity.
                     createCount.decrementAndGet();

src/test/java/org/apache/commons/pool3/impl/TestGenericObjectPool.java

@@ -43,6 +43,9 @@
 import java.util.Set;
 import java.util.Timer;
 import java.util.TimerTask;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
 import java.util.concurrent.Semaphore;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
@@ -1012,6 +1015,71 @@ void testAddObject() throws Exception {
         assertEquals(0, genericObjectPool.getNumActive(), "should be zero active");
     }
 
+
+    /*https://issues.apache.org/jira/browse/POOL-425*/
+    @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);
+
+        List<Runnable> tasks = getRunnables(numThreads, startLatch, doneLatch);
+
+        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) {
+        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;
+    }
+
+
     @Test
     void testAppendStats() {
         assertFalse(genericObjectPool.getMessageStatistics());