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

Open Beginner friendly
#3,321 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

@AkashB23 is already working on this.

Since Nov 2, 2026.

  • #3322 by @AkashB23 — open

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
62/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
java

Research direction

Start with the bindings field and bindIfNecessary() in BindingServiceProperties, then follow the getBindingDestination, getBindingProperties, getProducerProperties and getConsumerProperties callers. Keep the field's type and comparator in mind, since the fix must stay thread-safe and case-insensitive. Add the multi-threaded reproduction from the issue as a test in the binding properties test class. Done means that test passes, the key-count check matches, and case-insensitive lookups and sorted iteration still work.

Written by the indexing model from the issue text.

Description

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.

Dominant language
Java
Stars
1.1k
Forks
651
Avg merge
1d 15h
Merged PRs (30d)
17

Getting set up

This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from spring-cloud/spring-cloud-stream

All issues in spring-cloud/spring-cloud-stream

Similar issues

More Java issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.