[
https://issues.apache.org/jira/browse/POOL-411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17696483#comment-17696483
]
Gary D. Gregory edited comment on POOL-411 at 3/4/23 8:17 PM:
--------------------------------------------------------------
I thought I could reproduce it using the test below, but no:
{noformat}
diff --git
a/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
b/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
index 9ec3f60..a074257 100644
---
a/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
+++
b/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
@@ -33,7 +33,9 @@
import java.lang.management.ManagementFactory;
import java.time.Duration;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.HashSet;
+import java.util.List;
import java.util.NoSuchElementException;
import java.util.Objects;
import java.util.Random;
@@ -41,6 +43,7 @@
import java.util.Timer;
import java.util.TimerTask;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
@@ -976,7 +979,7 @@
// Start and park threads waiting to borrow objects
final TestThread[] threads = new TestThread[numThreads];
- for(int i=0;i<numThreads;i++) {
+ for (int i = 0; i < numThreads; i++) {
threads[i] = new TestThread<>(gkoPool, 1, 0, 2000, false, "0" +
String.valueOf(i % maxTotal), "0");
final Thread t = new Thread(threads[i]);
t.start();
@@ -1005,6 +1008,69 @@
}
/**
+ * Tests POOL-411, or least tries to reproduce the NPE.
+ *
+ * @throws TestException
+ */
+ @Test
+ public void testConcurrentBorrowAndClear() throws TestException {
+ final int threadCount = 64;
+ final int taskCount = 64;
+ final int addCount = 1;
+ final int borrowCycles = 1024;
+ final int clearCycles = 1024;
+
+ final GenericKeyedObjectPoolConfig<String> config = new
GenericKeyedObjectPoolConfig<>();
+ final int maxTotalPerKey = borrowCycles + 1;
+ config.setMaxTotalPerKey(threadCount);
+ config.setMaxIdlePerKey(threadCount);
+ config.setMaxTotal(maxTotalPerKey * threadCount);
+ config.setBlockWhenExhausted(false); // pool exhausted indicates a bug
in the test
+
+ gkoPool = new GenericKeyedObjectPool<>(simpleFactory, config);
+ gkoPool.addObjects(Arrays.asList("0"), threadCount);
+ // all objects in the pool are now idle.
+
+ final ExecutorService threadPool =
Executors.newFixedThreadPool(threadCount);
+ final List<Future<?>> futures = new ArrayList<>();
+ try {
+ for (int t = 0; t < taskCount; t++) {
+ futures.add(threadPool.submit(() -> {
+ for (int i = 0; i < clearCycles; i++) {
+ Thread.yield();
+ gkoPool.clear("0", true);
+ try {
+ gkoPool.addObjects(Arrays.asList("0"), addCount);
+ } catch (IllegalArgumentException | TestException e) {
+ fail(e);
+ }
+ }
+ }));
+ futures.add(threadPool.submit(() -> {
+ try {
+ for (int i = 0; i < borrowCycles; i++) {
+ Thread.yield();
+ final String pooled = gkoPool.borrowObject("0");
+ gkoPool.returnObject("0", pooled);
+ }
+ } catch (TestException e) {
+ fail(e);
+ }
+ }));
+ }
+ futures.forEach(f -> {
+ try {
+ f.get();
+ } catch (InterruptedException | ExecutionException e) {
+ fail(e);
+ }
+ });
+ } finally {
+ threadPool.shutdownNow();
+ }
+ }
+
+ /**
* POOL-192
* Verify that clear(key) does not leak capacity.
*
{noformat}
was (Author: garydgregory):
I thought I could reproduce it like the test below, but no:
{noformat}
diff --git
a/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
b/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
index 9ec3f60..a074257 100644
---
a/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
+++
b/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
@@ -33,7 +33,9 @@
import java.lang.management.ManagementFactory;
import java.time.Duration;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.HashSet;
+import java.util.List;
import java.util.NoSuchElementException;
import java.util.Objects;
import java.util.Random;
@@ -41,6 +43,7 @@
import java.util.Timer;
import java.util.TimerTask;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
@@ -976,7 +979,7 @@
// Start and park threads waiting to borrow objects
final TestThread[] threads = new TestThread[numThreads];
- for(int i=0;i<numThreads;i++) {
+ for (int i = 0; i < numThreads; i++) {
threads[i] = new TestThread<>(gkoPool, 1, 0, 2000, false, "0" +
String.valueOf(i % maxTotal), "0");
final Thread t = new Thread(threads[i]);
t.start();
@@ -1005,6 +1008,69 @@
}
/**
+ * Tests POOL-411, or least tries to reproduce the NPE.
+ *
+ * @throws TestException
+ */
+ @Test
+ public void testConcurrentBorrowAndClear() throws TestException {
+ final int threadCount = 64;
+ final int taskCount = 64;
+ final int addCount = 1;
+ final int borrowCycles = 1024;
+ final int clearCycles = 1024;
+
+ final GenericKeyedObjectPoolConfig<String> config = new
GenericKeyedObjectPoolConfig<>();
+ final int maxTotalPerKey = borrowCycles + 1;
+ config.setMaxTotalPerKey(threadCount);
+ config.setMaxIdlePerKey(threadCount);
+ config.setMaxTotal(maxTotalPerKey * threadCount);
+ config.setBlockWhenExhausted(false); // pool exhausted indicates a bug
in the test
+
+ gkoPool = new GenericKeyedObjectPool<>(simpleFactory, config);
+ gkoPool.addObjects(Arrays.asList("0"), threadCount);
+ // all objects in the pool are now idle.
+
+ final ExecutorService threadPool =
Executors.newFixedThreadPool(threadCount);
+ final List<Future<?>> futures = new ArrayList<>();
+ try {
+ for (int t = 0; t < taskCount; t++) {
+ futures.add(threadPool.submit(() -> {
+ for (int i = 0; i < clearCycles; i++) {
+ Thread.yield();
+ gkoPool.clear("0", true);
+ try {
+ gkoPool.addObjects(Arrays.asList("0"), addCount);
+ } catch (IllegalArgumentException | TestException e) {
+ fail(e);
+ }
+ }
+ }));
+ futures.add(threadPool.submit(() -> {
+ try {
+ for (int i = 0; i < borrowCycles; i++) {
+ Thread.yield();
+ final String pooled = gkoPool.borrowObject("0");
+ gkoPool.returnObject("0", pooled);
+ }
+ } catch (TestException e) {
+ fail(e);
+ }
+ }));
+ }
+ futures.forEach(f -> {
+ try {
+ f.get();
+ } catch (InterruptedException | ExecutionException e) {
+ fail(e);
+ }
+ });
+ } finally {
+ threadPool.shutdownNow();
+ }
+ }
+
+ /**
* POOL-192
* Verify that clear(key) does not leak capacity.
*
{noformat}
> Potential NPE when deregistering key at end of borrow
> -----------------------------------------------------
>
> Key: POOL-411
> URL: https://issues.apache.org/jira/browse/POOL-411
> Project: Commons Pool
> Issue Type: Task
> Affects Versions: 2.11.1
> Reporter: Richard Eckart de Castilho
> Priority: Major
>
> There is a potential for an NPE happening in the finally block of
> borrowObject:
> {noformat}
> Caused by: java.lang.NullPointerException: Cannot invoke
> "org.apache.commons.pool2.impl.GenericKeyedObjectPool$ObjectDeque.getNumInterested()"
> because "objectDeque" is null
> at
> org.apache.commons.pool2.impl.GenericKeyedObjectPool.deregister(GenericKeyedObjectPool.java:821)
> at
> org.apache.commons.pool2.impl.GenericKeyedObjectPool.borrowObject(GenericKeyedObjectPool.java:507)
> at
> org.apache.commons.pool2.impl.GenericKeyedObjectPool.borrowObject(GenericKeyedObjectPool.java:350)
>
> {noformat}
> From reading the code, it seems this could happen e.g. if a pool is
> concurrently cleared while a borrow is in progress.
> Not sure what a proper solution here would be. Maybe deregister should
> silently do nothing if poolMap.get(k) returns null?
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
