Fix MapperService StackOverflowError (#23605)

MapperService#parentTypes is rewrapped in an UnmodifiableSet in MapperService#internalMerge every time the cluster state is updated. After thousands of updates the collection is wrapped so deeply that calling a method on it generates a StackOverflowError.

Closes #23604

Author: jkiang13

core/src/main/java/org/elasticsearch/index/mapper/MapperService.java

@@ -112,7 +112,7 @@ public enum MergeReason {
     private volatile Map<String, DocumentMapper> mappers = emptyMap();
 
     private volatile FieldTypeLookup fieldTypes;
-    private volatile Map<String, ObjectMapper> fullPathObjectMappers = new HashMap<>();
+    private volatile Map<String, ObjectMapper> fullPathObjectMappers = emptyMap();
     private boolean hasNested = false; // updated dynamically to true when a nested object is added
     private boolean allEnabled = false; // updated dynamically to true when _all is enabled
 
@@ -391,6 +391,7 @@ private synchronized Map<String, DocumentMapper> internalMerge(@Nullable Documen
 
             for (ObjectMapper objectMapper : objectMappers) {
                 if (fullPathObjectMappers == this.fullPathObjectMappers) {
+                    // first time through the loops
                     fullPathObjectMappers = new HashMap<>(this.fullPathObjectMappers);
                 }
                 fullPathObjectMappers.put(objectMapper.fullPath(), objectMapper);
@@ -411,6 +412,7 @@ private synchronized Map<String, DocumentMapper> internalMerge(@Nullable Documen
 
             if (oldMapper == null && newMapper.parentFieldMapper().active()) {
                 if (parentTypes == this.parentTypes) {
+                    // first time through the loop
                     parentTypes = new HashSet<>(this.parentTypes);
                 }
                 parentTypes.add(mapper.parentFieldMapper().type());
@@ -453,8 +455,15 @@ private synchronized Map<String, DocumentMapper> internalMerge(@Nullable Documen
         // make structures immutable
         mappers = Collections.unmodifiableMap(mappers);
         results = Collections.unmodifiableMap(results);
-        parentTypes = Collections.unmodifiableSet(parentTypes);
-        fullPathObjectMappers = Collections.unmodifiableMap(fullPathObjectMappers);
+
+        // only need to immutably rewrap these if the previous reference was changed.
+        // if not then they are already implicitly immutable.
+        if (fullPathObjectMappers != this.fullPathObjectMappers) {
+            fullPathObjectMappers = Collections.unmodifiableMap(fullPathObjectMappers);
+        }
+        if (parentTypes != this.parentTypes) {
+            parentTypes = Collections.unmodifiableSet(parentTypes);
+        }
 
         // commit the change
         if (defaultMappingSource != null) {

core/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java

@@ -36,6 +36,7 @@
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.ExecutionException;
 import java.util.function.Function;
 
@@ -188,6 +189,22 @@ public void testMergeWithMap() throws Throwable {
         assertThat(e.getMessage(), startsWith("Failed to parse mapping [type1]: "));
     }
 
+    public void testMergeParentTypesSame() {
+        // Verifies that a merge (absent a DocumentMapper change)
+        // doesn't change the parentTypes reference.
+        // The collection was being rewrapped with each merge
+        // in v5.2 resulting in eventual StackOverflowErrors.
+        // https://github.com/elastic/elasticsearch/issues/23604
+
+        IndexService indexService1 = createIndex("index1");
+        MapperService mapperService = indexService1.mapperService();
+        Set<String> parentTypes = mapperService.getParentTypes();
+
+        Map<String, Map<String, Object>> mappings = new HashMap<>();
+        mapperService.merge(mappings, MergeReason.MAPPING_UPDATE, false);
+        assertSame(parentTypes, mapperService.getParentTypes());
+    }
+
     public void testOtherDocumentMappersOnlyUpdatedWhenChangingFieldType() throws IOException {
         IndexService indexService = createIndex("test");