PropertyEditorRegistrySupport.createDefaultEditors is not thread-safe when a registry is shared across threads

Bắt đầu với PropertyEditorRegistrySupport.getDefaultEditor() và createDefaultEditors() trong spring-beans, sau đó kiểm tra các bài kiểm thử hiện có về việc đăng ký trình chỉnh sửa mặc định. Thêm một bài kiểm thử hồi quy thực hiện các lần tra cứu đầu tiên đồng thời trên một registry dùng chung; hoàn thành khi việc khởi tạo an toàn và mỗi lần tra cứu trả về trình chỉnh sửa mong đợi.

Affects: spring-beans 7.0.9 (via Spring Boot 4.1.1); createDefaultEditors() has had the same shape for many years, so older lines are presumably affected too.

Problem

PropertyEditorRegistrySupport.getDefaultEditor lazily calls createDefaultEditors() when defaultEditors is null (lines 203-206 in 7.0.9). createDefaultEditors() assigns this.defaultEditors = new HashMap<>(64) first (line 213) and then re-reads the field for every put (lines 217 onwards). Two threads that both observe null therefore race: the second thread's assignment replaces the first thread's map while the first thread is still populating through the field, so both threads mutate one plain HashMap concurrently, and each thread's lookup at the end may hit a map that is not yet fully populated.

On a CI build we observe it as

java.lang.ClassCastException: class java.util.HashMap$Node cannot be cast to class java.util.HashMap$TreeNode
    at java.util.HashMap$TreeNode.moveRootToFront(HashMap.java:1995)
    at java.util.HashMap$TreeNode.treeify(HashMap.java:2111)
    at java.util.HashMap.treeifyBin(HashMap.java:779)
    at java.util.HashMap.putVal(HashMap.java:651)
    at java.util.HashMap.put(HashMap.java:619)
    at org.springframework.beans.PropertyEditorRegistrySupport.createDefaultEditors(PropertyEditorRegistrySupport.java:267)
    at org.springframework.beans.PropertyEditorRegistrySupport.getDefaultEditor(PropertyEditorRegistrySupport.java:204)
    at org.springframework.boot.context.properties.bind.BindConverter$TypeConverterConverter.matches(BindConverter.java:230)
    at org.springframework.core.convert.support.GenericConversionService$ConvertersForPair.getConverter(GenericConversionService.java:624)
    ...
    at org.springframework.boot.context.config.ConfigDataProperties.get(ConfigDataProperties.java:94)

surfacing as BindException: Failed to bind properties under 'spring.config.activate.on-profile' to java.lang.String[] in every SpringApplication that is starting at that moment.

How a registry ends up shared across threads

Spring Boot's BindConverter keeps a static sharedInstance, used by any Binder built without a custom conversion service, including the config-data phase of every SpringApplication.run. Its TypeConverterConverter holds a single SimpleTypeConverter, so the first binder use on each thread goes through the same PropertyEditorRegistrySupport. The race shows up when several SpringApplications start concurrently in one JVM, for example JUnit 5 running @SpringBootTest classes in parallel; on our build three contexts started within the same millisecond.

Minimal reproducer

Only spring-beans, spring-core and a commons-logging provider on the classpath. Each round shares one fresh SimpleTypeConverter between N threads released together, and each thread asks for the default editor of java.io.File, which createDefaultEditors() always registers.

import java.util.concurrent.CountDownLatch;
import java.util.concurrent.atomic.AtomicInteger;
import org.springframework.beans.SimpleTypeConverter;

public class DefaultEditorsRace {
    public static void main(String[] args) throws Exception {
        int rounds = Integer.parseInt(args[0]);
        int threads = Integer.parseInt(args[1]);
        AtomicInteger exceptions = new AtomicInteger();
        AtomicInteger missing = new AtomicInteger();
        for (int round = 0; round < rounds; round++) {
            SimpleTypeConverter shared = new SimpleTypeConverter();
            CountDownLatch ready = new CountDownLatch(threads);
            CountDownLatch go = new CountDownLatch(1);
            Thread[] workers = new Thread[threads];
            for (int t = 0; t < threads; t++) {
                workers[t] = new Thread(() -> {
                    ready.countDown();
                    try {
                        go.await();
                        if (shared.getDefaultEditor(java.io.File.class) == null) {
                            missing.incrementAndGet();
                        }
                    } catch (Exception e) {
                        exceptions.incrementAndGet();
                        System.out.println(e);
                    }
                });
                workers[t].start();
            }
            ready.await();
            go.countDown();
            for (Thread w : workers) {
                w.join();
            }
        }
        System.out.println("rounds=" + rounds + " threads=" + threads
            + " exceptions=" + exceptions + " nullEditorForFile=" + missing);
    }
}

Measured on OpenJDK 25.0.4, macOS, spring-beans 7.0.9, spring-core 7.0.9:

$ java -cp ... DefaultEditorsRace 2000 1
rounds=2000 threads=1 exceptions=0 nullEditorForFile=0

$ java -cp ... DefaultEditorsRace 2000 8      (three runs)
rounds=2000 threads=8 exceptions=0 nullEditorForFile=62
rounds=2000 threads=8 exceptions=0 nullEditorForFile=100
rounds=2000 threads=8 exceptions=0 nullEditorForFile=90

$ java -cp ... DefaultEditorsRace 2000 32     (two runs)
rounds=2000 threads=32 exceptions=0 nullEditorForFile=150
rounds=2000 threads=32 exceptions=0 nullEditorForFile=169

With one thread the editor is never missing. With several threads getDefaultEditor(File.class) returns null in roughly 0.5 to 1 percent of calls, because the caller is reading a map another thread is still filling. The ClassCastException from the stack trace above is the same race at a different moment (a concurrent put during treeification); the reproducer does not hit that particular interleaving on this machine but does show the shared map being mutated concurrently.

Expected

The lazy initialisation is safe to call from several threads, since Spring Boot shares the registry by design. Building the map into a local variable and assigning the field once at the end (optionally with a volatile field) removes the race; a ConcurrentHashMap would too.

Workaround we use

Warm the shared converter once before any parallel work. For JUnit 5 parallel execution, a LauncherSessionListener that binds one delimited property to String[] through a plain Binder fills the default editors on the shared instance before the first test class starts:

import java.util.Map;
import org.junit.platform.launcher.LauncherSession;
import org.junit.platform.launcher.LauncherSessionListener;
import org.springframework.boot.context.properties.bind.Bindable;
import org.springframework.boot.context.properties.bind.Binder;
import org.springframework.boot.context.properties.source.MapConfigurationPropertySource;

public final class BindConverterWarmUp implements LauncherSessionListener {

    private static final String PROPERTY = "bind-converter-warm-up";

    @Override
    public void launcherSessionOpened(LauncherSession session) {
        warmUp();
    }

    public static String[] warmUp() {
        var source = new MapConfigurationPropertySource(Map.of(PROPERTY, "first,second"));
        return new Binder(source).bind(PROPERTY, Bindable.of(String[].class)).get();
    }
}

registered in META-INF/services/org.junit.platform.launcher.LauncherSessionListener:

com.example.BindConverterWarmUp

Placed in a small test-support artifact on the test classpath of every module that runs context-booting test classes in parallel.