From 2f82f3cd02506ff11fc8275a84d388ec8cb791da Mon Sep 17 00:00:00 2001 From: Fernando Blanch Date: Tue, 24 Sep 2024 16:09:27 +0200 Subject: [PATCH] Remove the use of synchronized in the getBinder method of DefaultBinderFactory class for virtual-threads The getBinder method in DefaultBinderFactory was made thread-safe using ReentrantLock. This commit ensures that the method is friendly for virtual threads to avoid blocking and pinning. The lock is acquired at the beginning of the method and released in a finally block to ensure it is always released, even if an exception occurs. Fixes gh-3004 --- .../stream/binder/DefaultBinderFactory.java | 56 +++++++++++-------- 1 file changed, 32 insertions(+), 24 deletions(-) diff --git a/core/spring-cloud-stream/src/main/java/org/springframework/cloud/stream/binder/DefaultBinderFactory.java b/core/spring-cloud-stream/src/main/java/org/springframework/cloud/stream/binder/DefaultBinderFactory.java index 99d82a376..0eae62ed9 100644 --- a/core/spring-cloud-stream/src/main/java/org/springframework/cloud/stream/binder/DefaultBinderFactory.java +++ b/core/spring-cloud-stream/src/main/java/org/springframework/cloud/stream/binder/DefaultBinderFactory.java @@ -30,6 +30,7 @@ import java.util.Map.Entry; import java.util.Properties; import java.util.Set; +import java.util.concurrent.locks.ReentrantLock; import java.util.stream.Stream; import org.apache.commons.logging.Log; @@ -98,6 +99,8 @@ public class DefaultBinderFactory implements BinderFactory, DisposableBean, Appl private volatile String defaultBinder; + private static final ReentrantLock lock = new ReentrantLock(); + public DefaultBinderFactory(Map binderConfigurations, BinderTypeRegistry binderTypeRegistry, BinderCustomizer binderCustomizer) { this.binderConfigurations = new HashMap<>(binderConfigurations); @@ -142,34 +145,39 @@ public void destroy() { @SuppressWarnings({ "unchecked", "rawtypes" }) @Override + public Binder getBinder(String name, Class bindingTargetType) { + lock.lock(); + try { + String binderName = StringUtils.hasText(name) ? name : this.defaultBinder; - public synchronized Binder getBinder(String name, Class bindingTargetType) { - String binderName = StringUtils.hasText(name) ? name : this.defaultBinder; - - Map binders = this.context == null ? Collections.emptyMap() : this.context.getBeansOfType(Binder.class); - Binder binder; - if (StringUtils.hasText(binderName) && binders.containsKey(binderName)) { - binder = (Binder) this.context.getBean(binderName); - } - else if (binders.size() == 1) { - binder = binders.values().iterator().next(); - } - else if (binders.size() > 1) { - throw new IllegalStateException( + Map binders = this.context == null ? Collections.emptyMap() : this.context.getBeansOfType(Binder.class); + Binder binder; + if (StringUtils.hasText(binderName) && binders.containsKey(binderName)) { + binder = (Binder) this.context.getBean(binderName); + } + else if (binders.size() == 1) { + binder = binders.values().iterator().next(); + } + else if (binders.size() > 1) { + throw new IllegalStateException( "Multiple binders are available, however neither default nor " - + "per-destination binder name is provided. Available binders are " - + binders.keySet()); - } - else { - /* - * This is the fallback to the old bootstrap that relies on spring.binders. - */ - binder = this.doGetBinder(binderName, bindingTargetType); + + "per-destination binder name is provided. Available binders are " + + binders.keySet()); + } + else { + /* + * This is the fallback to the old bootstrap that relies on spring.binders. + */ + binder = this.doGetBinder(binderName, bindingTargetType); + } + if (this.binderCustomizer != null) { + this.binderCustomizer.customize(binder, binderName); + } + return binder; } - if (this.binderCustomizer != null) { - this.binderCustomizer.customize(binder, binderName); + finally { + lock.unlock(); } - return binder; } private Binder doGetBinder(String name, Class bindingTargetType) {