Improve thread safety in property source cache

Update `SpringIterableConfigurationPropertySource` so that they cache
and cache key are not stored in different fields. Prior to this commit
it was possible that the an incorrect cache could be returned from
because the key and cache were out of sync.

This commit also allows more lenient handling of ConcurrentModification
exceptions if they are thrown during cache retrieval.

Closes gh-17017
See gh-17013

Author: philwebb

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SpringIterableConfigurationPropertySource.java

@@ -18,6 +18,7 @@
 
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.ConcurrentModificationException;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
@@ -44,8 +45,6 @@
 class SpringIterableConfigurationPropertySource extends SpringConfigurationPropertySource
 		implements IterableConfigurationPropertySource {
 
-	private volatile Object cacheKey;
-
 	private volatile Cache cache;
 
 	SpringIterableConfigurationPropertySource(EnumerablePropertySource<?> propertySource,
@@ -131,16 +130,23 @@ private PropertyMapping[] getPropertyMappings(Cache cache) {
 	}
 
 	private Cache getCache() {
-		CacheKey cacheKey = CacheKey.get(getPropertySource());
-		if (cacheKey == null) {
+		CacheKey key = CacheKey.get(getPropertySource());
+		if (key == null) {
 			return null;
 		}
-		if (ObjectUtils.nullSafeEquals(cacheKey, this.cacheKey)) {
-			return this.cache;
+		Cache cache = this.cache;
+		try {
+			if (cache != null && cache.hasKeyEqualTo(key)) {
+				return cache;
+			}
+			cache = new Cache(key.copy());
+			this.cache = cache;
+			return cache;
+		}
+		catch (ConcurrentModificationException ex) {
+			// Not fatal at this point, we can continue without a cache
+			return null;
 		}
-		this.cache = new Cache();
-		this.cacheKey = cacheKey.copy();
-		return this.cache;
 	}
 
 	@Override
@@ -150,10 +156,20 @@ protected EnumerablePropertySource<?> getPropertySource() {
 
 	private static class Cache {
 
+		private final CacheKey key;
+
 		private List<ConfigurationPropertyName> names;
 
 		private PropertyMapping[] mappings;
 
+		Cache(CacheKey key) {
+			this.key = key;
+		}
+
+		public boolean hasKeyEqualTo(CacheKey key) {
+			return this.key.equals(key);
+		}
+
 		public List<ConfigurationPropertyName> getNames() {
 			return this.names;
 		}

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/SpringIterableConfigurationPropertySourceTests.java

@@ -16,8 +16,12 @@
 
 package org.springframework.boot.context.properties.source;
 
+import java.util.ConcurrentModificationException;
+import java.util.Iterator;
 import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
 import java.util.Map;
+import java.util.Set;
 
 import org.junit.Test;
 
@@ -169,6 +173,21 @@ public void propertySourceKeyDataChangeInvalidatesCache() {
 		assertThat(adapter.stream().count()).isEqualTo(3);
 	}
 
+	@Test
+	public void concurrentModificationExceptionInvalidatesCache() {
+		// gh-17013
+		ConcurrentModificationThrowingMap<String, Object> map = new ConcurrentModificationThrowingMap<>();
+		map.put("key1", "value1");
+		map.put("key2", "value2");
+		EnumerablePropertySource<?> source = new MapPropertySource("test", map);
+		SpringIterableConfigurationPropertySource adapter = new SpringIterableConfigurationPropertySource(
+				source, DefaultPropertyMapper.INSTANCE);
+		assertThat(adapter.stream().count()).isEqualTo(2);
+		map.setThrowException(true);
+		map.put("key3", "value3");
+		assertThat(adapter.stream().count()).isEqualTo(3);
+	}
+
 	/**
 	 * Test {@link PropertySource} that's also an {@link OriginLookup}.
 	 */
@@ -206,4 +225,37 @@ public String toString() {
 
 	}
 
+	private static class ConcurrentModificationThrowingMap<K, V>
+			extends LinkedHashMap<K, V> {
+
+		private boolean throwException;
+
+		public void setThrowException(boolean throwException) {
+			this.throwException = throwException;
+		}
+
+		@Override
+		public Set<K> keySet() {
+			return new KeySet(super.keySet());
+		}
+
+		private class KeySet extends LinkedHashSet<K> {
+
+			KeySet(Set<K> keySet) {
+				super(keySet);
+			}
+
+			@Override
+			public Iterator<K> iterator() {
+				if (ConcurrentModificationThrowingMap.this.throwException) {
+					ConcurrentModificationThrowingMap.this.throwException = false;
+					throw new ConcurrentModificationException();
+				}
+				return super.iterator();
+			}
+
+		}
+
+	}
+
 }