Fix removal from iterator and thread safety issue

Found by coverity.
apache · Sep 29, 2023 · 6780ac8 · 6780ac8
1 parent 2e1c93f
commit 6780ac8
Showing 1 changed file with 9 additions and 5 deletions.
diff --git 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.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;
@@ -74,11 +75,12 @@ private static class Reaper implements Runnable {
         @Override
         public void run() {
             synchronized (EvictionTimer.class) {
-                for (final Entry<WeakReference<BaseGenericObjectPool<?, ?>.Evictor>, WeakRunner<BaseGenericObjectPool<?, ?>.Evictor>> entry : TASK_MAP
-                        .entrySet()) {
+                Iterator<Entry<WeakReference<BaseGenericObjectPool<?, ?>.Evictor>, WeakRunner<BaseGenericObjectPool<?, ?>.Evictor>>> taskMapIterator = TASK_MAP.entrySet().iterator();
+                while (taskMapIterator.hasNext()) {
+                    final Entry<WeakReference<BaseGenericObjectPool<?, ?>.Evictor>, WeakRunner<BaseGenericObjectPool<?, ?>.Evictor>> entry = taskMapIterator.next();
                     if (entry.getKey().get() == null) {
                         executor.remove(entry.getValue());
-                        TASK_MAP.remove(entry.getKey());
+                        taskMapIterator.remove();
                     }
                 }
                 if (TASK_MAP.isEmpty() && executor != null) {
@@ -114,8 +116,10 @@ public void run() {
             if (task != null) {
                 task.run();
             } else {
-                executor.remove(this);
-                TASK_MAP.remove(ref);
+                synchronized (EvictionTimer.class) {
+                    executor.remove(this);
+                    TASK_MAP.remove(ref);
+                }
             }
         }
     }