Makes context use ConcurrentHashMap

computeIfAbsent might throw ConcurrentModificationException when working on multiple threads fixes gh-3874
micrometer-metrics · Sep 27, 2023 · d2b7ca4 · d2b7ca4
1 parent db78517
commit d2b7ca4
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 1 deletion.
diff --git a/micrometer-observation/src/main/java/io/micrometer/observation/Observation.java b/micrometer-observation/src/main/java/io/micrometer/observation/Observation.java
@@ -24,6 +24,7 @@
 import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Function;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
@@ -864,7 +865,7 @@ default boolean isNoop() {
     @SuppressWarnings("unchecked")
     class Context implements ContextView {
 
-        private final Map<Object, Object> map = new HashMap<>();
+        private final Map<Object, Object> map = new ConcurrentHashMap<>();
 
         private String name;
 

diff --git a/micrometer-observation/src/test/java/io/micrometer/observation/ObservationTests.java b/micrometer-observation/src/test/java/io/micrometer/observation/ObservationTests.java
@@ -17,6 +17,13 @@
 
 import io.micrometer.common.KeyValue;
 import io.micrometer.common.KeyValues;
+
+import java.util.ConcurrentModificationException;
+import java.util.concurrent.TimeUnit;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import org.awaitility.Awaitility;
 import io.micrometer.observation.Observation.Context;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -324,6 +331,34 @@ void instrumentationAndCustomAndDefaultAndGlobalConventionShouldResolveToInstrum
                 KeyValue.of("instrumentation", "present"), KeyValue.of("low", "instrumentation"));
     }
 
+    @Test
+    void contextShouldWorkOnMultipleThreads() {
+        Observation.Context context = new Observation.Context();
+        AtomicBoolean exception = new AtomicBoolean();
+
+        new Thread(() -> {
+            try {
+                for (int i = 0; i < 1000; i++) {
+                    context.computeIfAbsent("foo", o -> "bar");
+                }
+            }
+            catch (ConcurrentModificationException ex) {
+                exception.set(true);
+            }
+        }).start();
+        new Thread(() -> {
+            for (int i = 0; i < 1000; i++) {
+                context.clear();
+            }
+            context.put("thread", "done");
+        }).start();
+
+        Awaitility.await().atMost(150, TimeUnit.MILLISECONDS).pollDelay(10, TimeUnit.MILLISECONDS).untilAsserted(() -> {
+            assertThat(exception).as("ConcurrentModificationException must not be thrown").isFalse();
+            assertThat((String) context.get("thread")).isEqualTo("done");
+        });
+    }
+
     @Test
     void observe() {
         String result;