Fix ConfigUtil#getString ConcurrentModificationException (#6841)

Co-authored-by: neugartf <[email protected]>
Co-authored-by: Fabian Neugart <[email protected]>

api/all/src/main/java/io/opentelemetry/api/internal/ConfigUtil.java

@@ -5,8 +5,10 @@
 
 package io.opentelemetry.api.internal;
 
+import java.util.ConcurrentModificationException;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Properties;
 import javax.annotation.Nullable;
 
 /**
@@ -19,6 +21,17 @@ public final class ConfigUtil {
 
   private ConfigUtil() {}
 
+  /**
+   * Returns a copy of system properties which is safe to iterate over.
+   *
+   * <p>In java 8 and android environments, iterating through system properties may trigger {@link
+   * ConcurrentModificationException}. This method ensures callers can iterate safely without risk
+   * of exception. See https://github.com/open-telemetry/opentelemetry-java/issues/6732 for details.
+   */
+  public static Properties safeSystemProperties() {
+    return (Properties) System.getProperties().clone();
+  }
+
   /**
    * Return the system property or environment variable for the {@code key}.
    *
@@ -33,8 +46,9 @@ private ConfigUtil() {}
    */
   public static String getString(String key, String defaultValue) {
     String normalizedKey = normalizePropertyKey(key);
+
     String systemProperty =
-        System.getProperties().entrySet().stream()
+        safeSystemProperties().entrySet().stream()
             .filter(entry -> normalizedKey.equals(normalizePropertyKey(entry.getKey().toString())))
             .map(entry -> entry.getValue().toString())
             .findFirst()

api/all/src/test/java/io/opentelemetry/api/internal/ConfigUtilTest.java

@@ -6,7 +6,15 @@
 package io.opentelemetry.api.internal;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
 
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
 import org.junit.jupiter.api.Test;
 import org.junitpioneer.jupiter.SetSystemProperty;
 
@@ -56,4 +64,45 @@ void defaultIfnull() {
     assertThat(ConfigUtil.defaultIfNull("val1", "val2")).isEqualTo("val1");
     assertThat(ConfigUtil.defaultIfNull(null, "val2")).isEqualTo("val2");
   }
+
+  @Test
+  @SuppressWarnings("ReturnValueIgnored")
+  void systemPropertiesConcurrentAccess() throws ExecutionException, InterruptedException {
+    int threads = 4;
+    ExecutorService executor = Executors.newFixedThreadPool(threads);
+    try {
+      int cycles = 1000;
+      CountDownLatch latch = new CountDownLatch(1);
+      List<Future<?>> futures = new ArrayList<>();
+      for (int i = 0; i < threads; i++) {
+        futures.add(
+            executor.submit(
+                () -> {
+                  try {
+                    latch.await();
+                  } catch (InterruptedException e) {
+                    Thread.currentThread().interrupt();
+                  }
+
+                  for (int j = 0; j < cycles; j++) {
+                    String property = "prop " + j;
+                    System.setProperty(property, "a");
+                    System.getProperties().remove(property);
+                  }
+                }));
+      }
+
+      latch.countDown();
+      for (int i = 0; i < cycles; i++) {
+        assertThatCode(() -> ConfigUtil.getString("x", "y")).doesNotThrowAnyException();
+      }
+
+      for (Future<?> future : futures) {
+        future.get();
+      }
+
+    } finally {
+      executor.shutdownNow();
+    }
+  }
 }

sdk-extensions/autoconfigure-spi/src/main/java/io/opentelemetry/sdk/autoconfigure/spi/internal/DefaultConfigProperties.java

@@ -47,7 +47,8 @@ public final class DefaultConfigProperties implements ConfigProperties {
    * priority over environment variables.
    */
   public static DefaultConfigProperties create(Map<String, String> defaultProperties) {
-    return new DefaultConfigProperties(System.getProperties(), System.getenv(), defaultProperties);
+    return new DefaultConfigProperties(
+        ConfigUtil.safeSystemProperties(), System.getenv(), defaultProperties);
   }
 
   /**