BindingServiceProperties.bindings regressed to a non-thread-safe TreeMap in GH-3158: concurrent first use of binding names corrupts it

Aperta Adatta ai principianti
  • Qualcosa non va?
#3,321 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

@AkashB23 ci sta già lavorando.

Dal 2/11/2026.

  • #3322 di @AkashB23 — aperta

Valutazione

Difficoltà
2/5
Tempo stimato
1-3 ore
Idoneità per principianti
62/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
java
Ambito
backend, stream-processing

Direzione di ricerca

Inizia dal campo bindings e da bindIfNecessary() in BindingServiceProperties, quindi segui i chiamanti di getBindingDestination, getBindingProperties, getProducerProperties e getConsumerProperties. Tieni presente il tipo del campo e il comparatore, perché la correzione deve restare thread-safe e insensibile alle maiuscole. Aggiungi la riproduzione multithread dell'issue come test nella classe di test delle proprietà di binding. È completato quando quel test passa, il controllo sul numero di chiavi corrisponde e le ricerche insensibili alle maiuscole e l'iterazione ordinata continuano a funzionare.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

BindingServiceProperties.bindings has been a plain TreeMap again since GH-3158 (845a3e58). That map is written to on read, from many threads, with no synchronization, so concurrent first use of binding names corrupts it. This is the defect fixed in #3011, brought back. I mentioned it in a comment on #3158, but that issue is closed, so I am opening this one for tracking.

What goes wrong

getBindingDestination(), getBindingProperties(), getProducerProperties() and getConsumerProperties() all call bindIfNecessary(), which, for a name that is not yet in the map, runs bindToDefault() and does this.bindings.put(...):

private void bindIfNecessary(String bindingName) {
    if (!this.bindings.containsKey(bindingName)) {
        this.bindToDefault(bindingName);      // ... this.bindings.put(binding, target)
    }
}

Spring Cloud Stream itself calls these from arbitrary threads, with no lock: StreamBridge.send(), BindingService, AbstractMessageChannelBinder, FunctionConfiguration, MessageConverterConfigurer, ChannelsEndpoint (and BindingsLifecycleController calls setBindings). Any application that publishes with StreamBridge from several threads, or starts bindings concurrently, can hit it. Applications cannot work around it by locking on their side, because Spring's own callers do not take that lock.

An unsynchronized TreeMap that is mutated concurrently is corrupted in the ways TreeMap documents: lost entries, size() no longer matching the number of entries, NullPointerException in TreeMap.rotateRight (the failure reported in #3011), or an iteration that never ends.

Observed in production

Two incidents on Spring Cloud Stream 4.3.3 (the line numbers in the stack traces match that version):

  • One thread stays RUNNABLE in TreeMap.successor (once under BindingServiceProperties.setBindings -> putAll) and never returns. About 40 and 140 other threads, respectively, then block behind it, thread pools starve and stop advancing until the pod is restarted.
  • A heap dump of the stuck JVM shows the map's size field at 288 while only 66 entries are reachable from root, and a parent/child cycle between two nodes. In the second dump, 94 declared and 93 reachable, with two nodes cross-linked and three wrong parent pointers.
Reproduction

No broker is needed. This fails on a TreeMap and passes with a thread-safe map:

GenericApplicationContext context = new GenericApplicationContext();
context.refresh();
BindingServiceProperties properties = new BindingServiceProperties();
properties.setApplicationContext(context);

int threads = 16, operations = 300, names = 400;
CountDownLatch start = new CountDownLatch(1);
ExecutorService executor = Executors.newFixedThreadPool(threads);
List<Future<?>> futures = new ArrayList<>();
for (int t = 0; t < threads; t++) {
    Random random = new Random(t);
    futures.add(executor.submit(() -> {
        start.await();
        for (int i = 0; i < operations; i++) {
            String name = "binding-" + random.nextInt(names);
            properties.getBindingDestination(name);
            properties.getBindingProperties(name);
            properties.getProducerProperties(name);
            properties.getConsumerProperties(name + "-in");
        }
        return null;
    }));
}
start.countDown();
for (Future<?> f : futures) {
    f.get(30, TimeUnit.SECONDS);      // a spinning thread shows up as a TimeoutException
}
int iterated = 0;
for (String ignored : properties.getBindings().keySet()) {
    iterated++;
}
assertThat(iterated).isEqualTo(properties.getBindings().size());   // fails on TreeMap, e.g. expected: 798 but was: 800

History of the field

Commit Date Change
fd20513 (#3011) 2024-09-26 TreeMap -> ConcurrentSkipListMap, to fix a NullPointerException in TreeMap.rotateRight called from StreamBridge.send
afc8fa2 (#3076) 2025-02-26 ConcurrentSkipListMap -> ConcurrentHashMap: "Using 'ConcurrentSkipListMap' was causing significant performance issues"
845a3e5 (#3158) 2025-12-01 ConcurrentHashMap -> TreeMap(String.CASE_INSENSITIVE_ORDER), because ConcurrentHashMap made binding names case-sensitive

I checked the bytecode of the released jars. The map is a TreeMap (not thread-safe) in 4.3.1, 4.3.2, 4.3.3, 5.0.1, 5.0.2, 5.0.3, 5.0.4, 5.1.0-M1 and 5.1.0-RC1, and on main. It is a ConcurrentSkipListMap in 4.2.0 and a ConcurrentHashMap in 4.2.1 to 4.3.0 and 5.0.0.

So the two earlier fixes each traded one problem for another: the ConcurrentHashMap is thread-safe but case-sensitive, and the TreeMap is case-insensitive but not thread-safe.

The skip list was not shown to be slow

#3076 is the reason the ConcurrentSkipListMap was dropped, but I could not find anything in that thread that shows the map was the cause:

  • The reporter tried the ConcurrentHashMap build (4.2.1-SNAPSHOT) and reported "I do not see improvements with 4.2.1-SNAPSHOT. I see same performance as 4.2.0".
  • They traced the slowdown to a Spring Boot commit that changed child-context creation (spring-projects/spring-boot@09fdb9d), and later found their output-bindings property was under spring.cloud.config instead of spring.cloud.stream, so every first getBinder() call went through bindToDefault() (~4 ms, which is the Binder work, not the map). They closed it after optimizing their own binder.

For the per-lookup cost I measured the three candidates in a plain timing loop (not JMH; 300 entries, ~29-character keys, Java 21, warmed up, read-only):

Map get, 1 thread get, 16 threads
TreeMap(CASE_INSENSITIVE_ORDER) ~72 ns ~152 ns
ConcurrentSkipListMap(CASE_INSENSITIVE_ORDER) ~188 ns ~381 ns
ConcurrentHashMap (case-sensitive) ~5 ns ~11 ns

That is about 0.1 µs more per lookup than the TreeMap, and a binding is only added once, so I would not expect it to matter for start-up or for sending.

Proposal

Use new ConcurrentSkipListMap<>(String.CASE_INSENSITIVE_ORDER). It is thread-safe, keeps case-insensitive lookup, the original key casing and sorted iteration (so what #3158 needs), and needs no other change. A ConcurrentHashMap cannot be used because it is case-sensitive.

I have a one-line patch with a test (the reproduction above) and can open a pull request.

Lingua principale
Java
Stelle
1.1k
Fork
651
Merge medio
1g 15h
PR unite (30g)
17

Preparare l'ambiente

Questo progetto non fornisce container di sviluppo, Dockerfile né guida per i contributori, quindi l'ambiente è a tuo carico: parti dal suo README e consulta la nostra guida al primo contributo per i passaggi generali.

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di spring-cloud/spring-cloud-stream

Tutte le issue di spring-cloud/spring-cloud-stream

Issue simili

Altre issue su Java

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.