[POOL-431] Improve replenishment strategy in GenericObjectPool.invalidateObject

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

@@ -913,8 +913,13 @@ public void invalidateObject(final T obj) throws Exception {
      * {@inheritDoc}
      * <p>
      * Activation of this method decrements the active count and attempts to destroy the instance, using the provided
-     * {@link DestroyMode}. To ensure liveness of the pool, {@link #addObject()} is called to replace the invalidated
-     * instance.
+     * {@link DestroyMode}. To ensure liveness of the pool, when threads are waiting to borrow, a non-blocking
+     * attempt is made to create a replacement idle instance via {@link #addIdleObject(PooledObject)} with
+     * {@link Duration#ZERO}, directly bypassing the {@link #getMaxIdle()} gate that {@link #addObject()} enforces.
+     * Replenishment is only attempted when the object is actually destroyed (i.e. not already in
+     * {@link PooledObjectState#INVALID} state). Any exception thrown by the factory during replenishment
+     * is swallowed via {@link #swallowException(Exception)} so that the invalidation itself is never aborted
+     * by a factory failure.
      * </p>
      *
      * @throws Exception if an exception occurs destroying the object
@@ -933,11 +938,15 @@ public void invalidateObject(final T obj, final DestroyMode destroyMode) throws
         synchronized (p) {
             if (p.getState() != PooledObjectState.INVALID) {
                 destroy(p, destroyMode);
+                if (!isClosed() && idleObjects.hasTakeWaiters()) {
+                    try {
+                        addIdleObject(create(Duration.ZERO));
+                    } catch (final Exception e) {
+                        swallowException(e);
+                    }
+                }
             }
         }
-        if (!isClosed()) {
-             addObject();
-        }
     }
 
     /**

src/test/java/org/apache/commons/pool2/impl/TestAbandonedObjectPool.java

@@ -333,10 +333,7 @@ void testAbandonedReturn() throws Exception {
         // will deem abandoned.  Make sure it is not returned to the borrower.
         returner.start();    // short delay, then return instance
         assertTrue(pool.borrowObject().hashCode() != deadMansHash);
-        // There should be n - 2 - 1 idle instance in the pool
-        // The n - 2 instances deemed abandoned should have been replaced, but
-        // one of the new ones has been checked out.
-        assertEquals(n - 2 - 1, pool.getNumIdle());
+        assertEquals(0, pool.getNumIdle());
         assertEquals(1, pool.getNumActive());
     }
 

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

@@ -145,6 +145,27 @@ public PooledObject<String> wrap(final String obj) {
         }
     }
 
+    private static final class ControlledFailureFactory extends BasePooledObjectFactory<String> {
+        private boolean failOnCreate;
+
+        @Override
+        public String create() throws InterruptedException {
+            if (failOnCreate) {
+                throw new RuntimeException("Factory fails at runtime");
+            }
+            return "created";
+        }
+
+        public void failOnCreate(boolean failOnCreate) {
+            this.failOnCreate = failOnCreate;
+        }
+
+        @Override
+        public PooledObject<String> wrap(final String obj) {
+            return new DefaultPooledObject<>(obj);
+        }
+    }
+
     private static final class DummyFactory extends BasePooledObjectFactory<Object> {
         @Override
         public Object create() {
@@ -2046,6 +2067,80 @@ void testInvalidateFreesCapacity() throws Exception {
         }
     }
 
+    /**
+     * Verify that invalidateObject does not propagate factory exceptions that occur during the waiter-replenishment
+     * attempt. The exception must be swallowed; invalidation itself must complete normally.
+     */
+    @Test
+    void testInvalidateSwallowsReplenishmentFactoryException() throws Exception {
+        final ControlledFailureFactory factory = new ControlledFailureFactory();
+        final List<Exception> swallowed = new ArrayList<>();
+        try (GenericObjectPool<String> pool = new GenericObjectPool<>(factory)) {
+            pool.setMaxTotal(1);
+            pool.setSwallowedExceptionListener(swallowed::add);
+
+            final String obj = pool.borrowObject();
+            new WaitingTestThread(pool, 0).start();
+            Thread.sleep(50);
+
+            // Cause the replenishment attempt to fail.
+            factory.failOnCreate(true);
+
+            // Must complete without throwing even though factory.makeObject() will throw.
+            pool.invalidateObject(obj);
+
+            // The swallowed list proves the exception was captured, not propagated.
+            assertFalse(swallowed.isEmpty(), "Factory exception during replenishment must be swallowed, not propagated");
+        }
+    }
+
+    /**
+     * Verify that a thread waiting to borrow is served by the idle object created during invalidateObject replenishment.
+     */
+    @Test
+    @Timeout(value = 10000, unit = TimeUnit.MILLISECONDS)
+    void testInvalidateReplenishesWaiter() throws Exception {
+        final SimpleFactory factory = new SimpleFactory();
+        try (GenericObjectPool<String> pool = new GenericObjectPool<>(factory)) {
+            pool.setMaxTotal(1);
+            pool.setMaxWait(Duration.ofMillis(3000));
+
+            final String obj = pool.borrowObject(); // pool exhausted
+
+            // Start a waiter; it must be served by the replenishment triggered inside invalidateObject.
+            final WaitingTestThread waiter = new WaitingTestThread(pool, 0);
+            waiter.start();
+            Thread.sleep(50); // ensure the waiter is blocked
+
+            pool.invalidateObject(obj);
+
+            waiter.join(2000);
+            assertNull(waiter.thrown, "Waiter should have been served by invalidateObject replenishment, but threw: " + waiter.thrown);
+        }
+    }
+
+    /**
+     * Verify that {@code invalidateObject} does not attempt replenishment when no thread is waiting to borrow,
+     * i.e. the replenishment path is gated on {@code hasTakeWaiters()}.
+     */
+    @Test
+    void testInvalidateDoesNotReplenishWhenNoWaiter() throws Exception {
+        final ControlledFailureFactory factory = new ControlledFailureFactory();
+        final List<Exception> swallowed = new ArrayList<>();
+        try (GenericObjectPool<String> pool = new GenericObjectPool<>(factory)) {
+            pool.setMaxTotal(1);
+            pool.setSwallowedExceptionListener(swallowed::add);
+
+            final String obj = pool.borrowObject(); // pool exhausted, no thread waiting
+            // Cause the replenishment attempt to fail, in case factory called.
+            factory.failOnCreate(true);
+
+            pool.invalidateObject(obj);
+
+            assertTrue(swallowed.isEmpty(), "ObjectPool not excepted to replenish when there is no waiter");
+        }
+    }
+
     /**
      * Ensure the pool is registered.
      */