On Mon, Sep 15, 2025 at 10:42 AM Mark Thomas <ma...@apache.org> wrote: > > 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.
OK, thanks, Mark. G > > 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 > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
