You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by ma...@apache.org on 2025/09/03 12:55:49 UTC
(commons-pool) branch master updated (38bc588b -> 1bf82a68)
This is an automated email from the ASF dual-hosted git repository.
markt pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/commons-pool.git
from 38bc588b Clarify that poolMap must use ConcurrentHashMap. No functional change.
new 3199aa4a Remove unnecessary code
new 1bf82a68 Fix potential ConcurrentModificationException in Reaper clean-up thread.
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
src/changes/changes.xml | 1 +
.../apache/commons/pool3/impl/EvictionTimer.java | 15 ++++++++++++
.../commons/pool3/impl/TestGenericObjectPool.java | 27 +++++++++++++++-------
3 files changed, 35 insertions(+), 8 deletions(-)
(commons-pool) 01/02: Remove unnecessary code
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-pool.git
commit 3199aa4ae1ba1eeb7d4a2f96b7d89337d85b0fdd
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Sep 2 17:43:22 2025 +0100
Remove unnecessary code
This was testing that the deprecated getter and the new getter returned
the same value. With the removal of the deprecated getter this check is
no longer required.
---
src/test/java/org/apache/commons/pool3/impl/TestGenericObjectPool.java | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/test/java/org/apache/commons/pool3/impl/TestGenericObjectPool.java b/src/test/java/org/apache/commons/pool3/impl/TestGenericObjectPool.java
index 6317f1a3..9b385809 100644
--- a/src/test/java/org/apache/commons/pool3/impl/TestGenericObjectPool.java
+++ b/src/test/java/org/apache/commons/pool3/impl/TestGenericObjectPool.java
@@ -975,7 +975,6 @@ class TestGenericObjectPool extends TestBaseObjectPool {
config.setJmxEnabled(false);
GenericObjectPool<String, TestException> abandoned = new GenericObjectPool<>(simpleFactory, config);
abandoned.setDurationBetweenEvictionRuns(Duration.ofMillis(100)); // Starts evictor
- assertEquals(abandoned.getRemoveAbandonedTimeoutDuration(), abandoned.getRemoveAbandonedTimeoutDuration());
// This is ugly, but forces GC to hit the pool
final WeakReference<GenericObjectPool<String, TestException>> ref = new WeakReference<>(abandoned);
(commons-pool) 02/02: Fix potential ConcurrentModificationException in Reaper clean-up thread.
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-pool.git
commit 1bf82a68026b3a3600141a642b9a06be5cae756e
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Sep 3 13:55:42 2025 +0100
Fix potential ConcurrentModificationException in Reaper clean-up thread.
---
src/changes/changes.xml | 1 +
.../apache/commons/pool3/impl/EvictionTimer.java | 15 ++++++++++++++
.../commons/pool3/impl/TestGenericObjectPool.java | 24 ++++++++++++++++------
3 files changed, 34 insertions(+), 6 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 2f0e4024..d070a722 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -55,6 +55,7 @@ The <action> type attribute can be add,update,fix,remove.
<action type="fix" dev="ggregory" due-to="Gary Gregory">Remove -nouses directive from maven-bundle-plugin. OSGi package imports now state 'uses' definitions for package imports, this doesn't affect JPMS (from org.apache.commons:commons-parent:80).</action>
<action type="fix" dev="ggregory" due-to="Gary Gregory">Operation on the "idleHighWaterMark" shared variable in "ErodingFactor" class is not atomic [org.apache.commons.pool3.PoolUtils$ErodingFactor] At PoolUtils.java:[line 101] AT_NONATOMIC_OPERATIONS_ON_SHARED_VARIABLE.</action>
<action type="fix" dev="ggregory" due-to="shengulong, Gary Gregory" issue="POOL-422">org.apache.commons.pool3.impl.GenericObjectPool.create(Duration) duration computation doesn't match 2.x. See also PR #409 from shengulong.</action>
+ <action type="fix" dev="markt" due-to="Coverity Scan">Fix potential ConcurrentModificationException in EvictionTimer thread clean-up.</action>
<!-- REMOVE -->
<action dev="ggregory" type="remove">Deprecations from version 2.x have been removed.</action>
<!-- UPDATE -->
diff --git a/src/main/java/org/apache/commons/pool3/impl/EvictionTimer.java b/src/main/java/org/apache/commons/pool3/impl/EvictionTimer.java
index 219634eb..0bf5f9bf 100644
--- a/src/main/java/org/apache/commons/pool3/impl/EvictionTimer.java
+++ b/src/main/java/org/apache/commons/pool3/impl/EvictionTimer.java
@@ -21,6 +21,7 @@ import java.security.AccessController;
import java.security.PrivilegedAction;
import java.time.Duration;
import java.util.HashMap;
+import java.util.Iterator;
import java.util.Map.Entry;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.ScheduledThreadPoolExecutor;
@@ -73,6 +74,20 @@ final class EvictionTimer {
@Override
public void run() {
synchronized (EvictionTimer.class) {
+ /*
+ * Need to use iterator over TASK_MAP so entries can be removed when iterating without triggering a
+ * ConcurrentModificationException.
+ */
+ final Iterator<Entry<WeakReference<BaseGenericObjectPool<?, ?>.Evictor>, WeakRunner<BaseGenericObjectPool<?, ?>.Evictor>>> iterator =
+ TASK_MAP.entrySet().iterator();
+ while (iterator.hasNext()) {
+ final Entry<WeakReference<BaseGenericObjectPool<?, ?>.Evictor>, WeakRunner<BaseGenericObjectPool<?, ?>.Evictor>> entry = iterator.next();
+ if (entry.getKey().get() == null) {
+ executor.remove(entry.getValue());
+ iterator.remove();
+ }
+ }
+
for (final Entry<WeakReference<BaseGenericObjectPool<?, ?>.Evictor>, WeakRunner<BaseGenericObjectPool<?, ?>.Evictor>> entry : TASK_MAP
.entrySet()) {
if (entry.getKey().get() == null) {
diff --git a/src/test/java/org/apache/commons/pool3/impl/TestGenericObjectPool.java b/src/test/java/org/apache/commons/pool3/impl/TestGenericObjectPool.java
index 9b385809..e4bc90df 100644
--- a/src/test/java/org/apache/commons/pool3/impl/TestGenericObjectPool.java
+++ b/src/test/java/org/apache/commons/pool3/impl/TestGenericObjectPool.java
@@ -973,13 +973,25 @@ class TestGenericObjectPool extends TestBaseObjectPool {
void testAbandonedPool() throws TestException, InterruptedException {
final GenericObjectPoolConfig<String> config = new GenericObjectPoolConfig<>();
config.setJmxEnabled(false);
- GenericObjectPool<String, TestException> abandoned = new GenericObjectPool<>(simpleFactory, config);
- abandoned.setDurationBetweenEvictionRuns(Duration.ofMillis(100)); // Starts evictor
- // This is ugly, but forces GC to hit the pool
- final WeakReference<GenericObjectPool<String, TestException>> ref = new WeakReference<>(abandoned);
- abandoned = null;
- while (ref.get() != null) {
+ // Need to create at least 2 pools to test EvictorTimer Reaper for ConcurrentModificationException
+ // Note: If the test fails, the ConcurrentModificationException isn't visible without modifications to the
+ // Reaper class.
+ GenericObjectPool<String, TestException> abandoned1 = new GenericObjectPool<>(simpleFactory, config);
+ abandoned1.setDurationBetweenEvictionRuns(Duration.ofMillis(100)); // Starts evictor
+ GenericObjectPool<String, TestException> abandoned2 = new GenericObjectPool<>(simpleFactory, config);
+ abandoned2.setDurationBetweenEvictionRuns(Duration.ofMillis(100)); // Starts evictor
+
+ // This is ugly, but forces GC to hit the pools
+ final WeakReference<GenericObjectPool<String, TestException>> ref1 = new WeakReference<>(abandoned1);
+ abandoned1 = null;
+ while (ref1.get() != null) {
+ System.gc();
+ Thread.sleep(100);
+ }
+ final WeakReference<GenericObjectPool<String, TestException>> ref2 = new WeakReference<>(abandoned2);
+ abandoned2 = null;
+ while (ref2.get() != null) {
System.gc();
Thread.sleep(100);
}