On 15/09/2025 12:18, Gary Gregory wrote:
Mark and all,
Would using a concurrent map be better?
I don't think it would. The purpose of the lock is to keep executor and
TASK-MAP in sync. Given you have to use an external lock to do that (or
at least I can't think of a way to do it without an external lock) there
isn't any point switching to a concurrent map as it will have marginally
more overhead.
Mark
Gary
On Mon, Sep 15, 2025, 04:47 <ma...@apache.org> wrote:
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch POOL_2_X
in repository https://gitbox.apache.org/repos/asf/commons-pool.git
The following commit(s) were added to refs/heads/POOL_2_X by this push:
new c8d672a4 Add missing synchronized.
c8d672a4 is described below
commit c8d672a43d04f5922e041c7e6e668915a5dcf606
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Sep 15 09:20:22 2025 +0100
Add missing synchronized.
Access to executor and TASK_MAP are meant to be guarded by a lock on
EvictionTimer.class.
Reported by Coverity via Tomcat
---
src/changes/changes.xml | 1 +
src/main/java/org/apache/commons/pool2/impl/EvictionTimer.java | 6 ++++--
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index b355622e..14272837 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -62,6 +62,7 @@ The <action> type attribute can be add,update,fix,remove.
<action type="fix" dev="ggregory" due-to="Gary Gregory">Operation on
the "idleHighWaterMark" shared variable in "ErodingFactor" class is not
atomic [org.apache.commons.pool2.PoolUtils$ErodingFactor] At
PoolUtils.java:[line 98]
AT_NONATOMIC_OPERATIONS_ON_SHARED_VARIABLE.</action>
<action type="fix" dev="ggregory" due-to="Gary
Gregory">org.apache.commons.pool2.impl.GenericObjectPool.create(Duration)
should normalize a negative duration to zero.</action>
<action type="fix" dev="markt" due-to="Coverity Scan">Fix
potential ConcurrentModificationException in EvictionTimer thread
clean-up.</action>
+ <action type="fix" dev="markt" due-to="Coverity Scan">Fix
potential ConcurrentModificationException in EvictionTimer tasks.</action>
<!-- ADD -->
<action type="add" dev="ggregory" due-to="Gary Gregory">Add
org.apache.commons.pool2.PooledObject.nonNull(PooledObject).</action>
<action type="add" dev="ggregory" due-to="Gary Gregory">Add
org.apache.commons.pool2.PooledObject.getObject(PooledObject).</action>
diff --git
a/src/main/java/org/apache/commons/pool2/impl/EvictionTimer.java
b/src/main/java/org/apache/commons/pool2/impl/EvictionTimer.java
index e9a62b89..808c9a8d 100644
--- a/src/main/java/org/apache/commons/pool2/impl/EvictionTimer.java
+++ b/src/main/java/org/apache/commons/pool2/impl/EvictionTimer.java
@@ -120,8 +120,10 @@ final class EvictionTimer {
if (task != null) {
task.run();
} else {
- executor.remove(this);
- TASK_MAP.remove(ref);
+ synchronized (EvictionTimer.class) {
+ executor.remove(this);
+ TASK_MAP.remove(ref);
+ }
}
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
