Mark and all, Would using a concurrent map be better?
Gary On Mon, Sep 15, 2025, 04:47 <[email protected]> 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 <[email protected]> > 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); > + } > } > } > } > >
