(commons-pool) 01/02: 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

[ggregory]

This is an automated email from the ASF dual-hosted git repository.

ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-pool.git

commit 1db3c359429e67e7df6ffbed86a11d83422cb8d9
Author: Gary D. Gregory <garydgreg...@gmail.com>
AuthorDate: Fri May 2 11:02:15 2025 -0400

    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
---
 src/changes/changes.xml                            |  1 +
 .../java/org/apache/commons/pool3/PoolUtils.java   | 22 +++++++++++++++-------
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index db802edd..d381623b 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -48,6 +48,7 @@ The <action> type attribute can be add,update,fix,remove.
     <!-- FIX -->
     <action type="fix" dev="psteitz" issue="POOL-407">Add 
ReslientPooledObjectFactory to provide resilence against factory 
outages.</action>
     <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>
     <!-- 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/PoolUtils.java 
b/src/main/java/org/apache/commons/pool3/PoolUtils.java
index ef5c456f..430bd71c 100644
--- a/src/main/java/org/apache/commons/pool3/PoolUtils.java
+++ b/src/main/java/org/apache/commons/pool3/PoolUtils.java
@@ -24,6 +24,7 @@ import java.util.Map;
 import java.util.NoSuchElementException;
 import java.util.Timer;
 import java.util.TimerTask;
+import java.util.concurrent.locks.ReentrantLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock.ReadLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock.WriteLock;
@@ -49,6 +50,9 @@ public final class PoolUtils {
      * frequently.
      */
     private static final class ErodingFactor {
+
+        private static final float MAX_INTERVAL = 15f;
+
         /** Determines frequency of "erosion" events */
         private final float factor;
 
@@ -56,7 +60,9 @@ public final class PoolUtils {
         private transient volatile long nextShrinkMillis;
 
         /** High water mark - largest numIdle encountered */
-        private transient volatile int idleHighWaterMark;
+        private transient volatile int idleHighWaterMark = 1;
+
+        private final ReentrantLock lock = new ReentrantLock();
 
         /**
          * Creates a new ErodingFactor with the given erosion factor.
@@ -67,7 +73,6 @@ public final class PoolUtils {
         ErodingFactor(final float factor) {
             this.factor = factor;
             nextShrinkMillis = System.currentTimeMillis() + (long) (900000 * 
factor); // now + 15 min * factor
-            idleHighWaterMark = 1;
         }
 
         /**
@@ -98,11 +103,14 @@ public final class PoolUtils {
          */
         public void update(final long nowMillis, final int numIdle) {
             final int idle = Math.max(0, numIdle);
-            idleHighWaterMark = Math.max(idle, idleHighWaterMark);
-            final float maxInterval = 15f;
-            final float minutes = maxInterval +
-                    (1f - maxInterval) / idleHighWaterMark * idle;
-            nextShrinkMillis = nowMillis + (long) (minutes * 60000f * factor);
+            lock.lock();
+            try {
+                idleHighWaterMark = Math.max(idle, idleHighWaterMark);
+                final float minutes = MAX_INTERVAL + (1f - MAX_INTERVAL) / 
idleHighWaterMark * idle;
+                nextShrinkMillis = nowMillis + (long) (minutes * 60000f * 
factor);
+            } finally {
+                lock.unlock();
+            }
         }
     }
     /**