diff --git a/api/all/src/main/java/io/opentelemetry/api/internal/ConfigUtil.java b/api/all/src/main/java/io/opentelemetry/api/internal/ConfigUtil.java index 56d2d378d72..3a10a23cb99 100644 --- a/api/all/src/main/java/io/opentelemetry/api/internal/ConfigUtil.java +++ b/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. + * + *

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() diff --git a/api/all/src/test/java/io/opentelemetry/api/internal/ConfigUtilTest.java b/api/all/src/test/java/io/opentelemetry/api/internal/ConfigUtilTest.java index f7dde6a9735..a546c32862b 100644 --- a/api/all/src/test/java/io/opentelemetry/api/internal/ConfigUtilTest.java +++ b/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> 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(); + } + } } diff --git a/sdk-extensions/autoconfigure-spi/src/main/java/io/opentelemetry/sdk/autoconfigure/spi/internal/DefaultConfigProperties.java b/sdk-extensions/autoconfigure-spi/src/main/java/io/opentelemetry/sdk/autoconfigure/spi/internal/DefaultConfigProperties.java index af0e4bae2bf..b5496df7c48 100644 --- a/sdk-extensions/autoconfigure-spi/src/main/java/io/opentelemetry/sdk/autoconfigure/spi/internal/DefaultConfigProperties.java +++ b/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 defaultProperties) { - return new DefaultConfigProperties(System.getProperties(), System.getenv(), defaultProperties); + return new DefaultConfigProperties( + ConfigUtil.safeSystemProperties(), System.getenv(), defaultProperties); } /**