[POOL-419] Fix for POOL-419. Before adding the returned object to the…

… pool, we must check whether it is already deallocated.
apache · Feb 3, 2025 · d38ea75 · d38ea75
1 parent 0b4d913
commit d38ea75
Show file tree

Hide file tree

Showing 2 changed files with 97 additions and 4 deletions.
diff --git a/src/test/java/org/apache/commons/pool3/TestPoolUtils.java b/src/test/java/org/apache/commons/pool3/TestPoolUtils.java
@@ -692,6 +692,22 @@ public void testTimerHolder() {
         assertNotNull(PoolUtils.TimerHolder.MIN_IDLE_TIMER);
     }
 
+    private BasePooledObjectFactory<Object, RuntimeException> createDefaultPooledObjectFactory() {
+        return new BasePooledObjectFactory<>() {
+            @Override
+            public Object create() {
+                return new Object();
+            }
+
+            @Override
+            public PooledObject<Object> wrap(final Object obj) {
+                // fake
+                return new DefaultPooledObject<>(obj);
+            }
+        };
+    }
+
+
     /*
     * Test for POOL-419.
     * https://issues.apache.org/jira/browse/POOL-419
@@ -701,8 +717,6 @@ void testPool419() throws Exception{
 
         ExecutorService executor = Executors.newFixedThreadPool(100);
 
-        final List<String> calledMethods = new ArrayList<>();
-
         final GenericObjectPoolConfig<Object> config = new GenericObjectPoolConfig<>();
 
         final int maxConnections = 10000;
@@ -711,8 +725,8 @@ void testPool419() throws Exception{
         config.setMaxIdle(maxConnections);
         config.setMinIdle(1);
 
-        @SuppressWarnings("unchecked")
-        final PooledObjectFactory<Object, RuntimeException> pof = createProxy(PooledObjectFactory.class, calledMethods);
+        final BasePooledObjectFactory<Object, RuntimeException> pof = createDefaultPooledObjectFactory();
+
         try (final ObjectPool<Object, RuntimeException> connectionPool = new GenericObjectPool<>(pof, config)) {
             assertNotNull(connectionPool);
 

diff --git a/src/test/java/org/apache/commons/pool3/impl/TestGenericObjectPool.java b/src/test/java/org/apache/commons/pool3/impl/TestGenericObjectPool.java
@@ -41,6 +41,10 @@
 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.FutureTask;
 import java.util.concurrent.Semaphore;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
@@ -3150,4 +3154,79 @@ public void testWhenExhaustedFail() throws Exception {
         genericObjectPool.close();
     }
 
+    private BasePooledObjectFactory<Object, RuntimeException> createPooledObjectFactory() {
+        return new BasePooledObjectFactory<>() {
+            @Override
+            public Object create() {
+                return new Object();
+            }
+
+            @Override
+            public PooledObject<Object> wrap(final Object obj) {
+                // fake
+                return new DefaultPooledObject<>(obj);
+            }
+        };
+    }
+
+
+    /*
+     * Test for POOL-419.
+     * https://issues.apache.org/jira/browse/POOL-419
+     */
+    @Test
+    void testPool419() throws Exception{
+
+        ExecutorService executor = Executors.newFixedThreadPool(100);
+
+        final GenericObjectPoolConfig<Object> config = new GenericObjectPoolConfig<>();
+
+        final int maxConnections = 10000;
+
+        config.setMaxTotal(maxConnections);
+        config.setMaxIdle(maxConnections);
+        config.setMinIdle(1);
+
+        final BasePooledObjectFactory<Object, RuntimeException> pof = createPooledObjectFactory();
+
+        try (final ObjectPool<Object, RuntimeException> connectionPool = new GenericObjectPool<>(pof, config)) {
+            assertNotNull(connectionPool);
+
+            CountDownLatch startLatch = new CountDownLatch(1);
+
+            List<Object> poolObjects = new ArrayList<>();
+
+            List<FutureTask<Boolean>> tasks = new ArrayList<>();
+
+            for (int i = 0; i < maxConnections; i++) {
+                poolObjects.add(connectionPool.borrowObject());
+            }
+
+            for(Object poolObject : poolObjects) {
+
+                tasks.add(new FutureTask<>(() -> {
+                    startLatch.await();
+                    connectionPool.invalidateObject(poolObject);
+                    return true;
+                }));
+
+                tasks.add(new FutureTask<>(() -> {
+                    startLatch.await();
+                    connectionPool.returnObject(poolObject);
+                    return true;
+                }));
+            }
+
+            tasks.forEach(executor::submit);
+
+            startLatch.countDown(); // Start all tasks simultaneously
+
+            executor.shutdown();
+            assertTrue(executor.awaitTermination(Long.MAX_VALUE, TimeUnit.NANOSECONDS));
+
+            assertEquals(0, connectionPool.getNumActive(), "getNumActive() must not return a negative value");
+
+        }
+    }
+
 }