diff --git a/spring-integration-amqp/src/main/java/org/springframework/integration/amqp/outbound/AbstractAmqpOutboundEndpoint.java b/spring-integration-amqp/src/main/java/org/springframework/integration/amqp/outbound/AbstractAmqpOutboundEndpoint.java index b55f77c8707..171c53c4f12 100644 --- a/spring-integration-amqp/src/main/java/org/springframework/integration/amqp/outbound/AbstractAmqpOutboundEndpoint.java +++ b/spring-integration-amqp/src/main/java/org/springframework/integration/amqp/outbound/AbstractAmqpOutboundEndpoint.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2022 the original author or authors. + * Copyright 2016-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -23,6 +23,8 @@ import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.springframework.amqp.core.MessageDeliveryMode; import org.springframework.amqp.core.ReturnedMessage; @@ -58,6 +60,7 @@ * * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov * * @since 4.3 * @@ -115,6 +118,8 @@ public abstract class AbstractAmqpOutboundEndpoint extends AbstractReplyProducin private volatile ScheduledFuture confirmChecker; + private final Lock lock = new ReentrantLock(); + /** * Set a custom {@link AmqpHeaderMapper} for mapping request and reply headers. * Defaults to {@link DefaultAmqpHeaderMapper#outboundMapper()}. @@ -336,8 +341,14 @@ public void setConfirmTimeout(long confirmTimeout) { this.confirmTimeout = Duration.ofMillis(confirmTimeout); // NOSONAR sync inconsistency } - protected final synchronized void setConnectionFactory(ConnectionFactory connectionFactory) { - this.connectionFactory = connectionFactory; + protected final void setConnectionFactory(ConnectionFactory connectionFactory) { + this.lock.lock(); + try { + this.connectionFactory = connectionFactory; + } + finally { + this.lock.unlock(); + } } protected String getExchangeName() { @@ -487,26 +498,33 @@ protected void endpointInit() { } @Override - public synchronized void start() { - if (!this.running) { - if (!this.lazyConnect && this.connectionFactory != null) { - try { - Connection connection = this.connectionFactory.createConnection(); // NOSONAR (close) - if (connection != null) { - connection.close(); + public void start() { + this.lock.lock(); + try { + if (!this.running) { + if (!this.lazyConnect && this.connectionFactory != null) { + try { + Connection connection = this.connectionFactory.createConnection(); // NOSONAR (close) + if (connection != null) { + connection.close(); + } + } + catch (RuntimeException ex) { + logger.error(ex, "Failed to eagerly establish the connection."); } } - catch (RuntimeException ex) { - logger.error(ex, "Failed to eagerly establish the connection."); + doStart(); + if (this.confirmTimeout != null && getConfirmNackChannel() != null && getRabbitTemplate() != null) { + this.confirmChecker = getTaskScheduler() + .scheduleAtFixedRate(checkUnconfirmed(), this.confirmTimeout.dividedBy(2L)); } + this.running = true; } - doStart(); - if (this.confirmTimeout != null && getConfirmNackChannel() != null && getRabbitTemplate() != null) { - this.confirmChecker = getTaskScheduler() - .scheduleAtFixedRate(checkUnconfirmed(), this.confirmTimeout.dividedBy(2L)); - } - this.running = true; } + finally { + this.lock.unlock(); + } + } private Runnable checkUnconfirmed() { @@ -526,14 +544,20 @@ private Runnable checkUnconfirmed() { protected abstract RabbitTemplate getRabbitTemplate(); @Override - public synchronized void stop() { - if (this.running) { - doStop(); - } - this.running = false; - if (this.confirmChecker != null) { - this.confirmChecker.cancel(false); - this.confirmChecker = null; + public void stop() { + this.lock.lock(); + try { + if (this.running) { + doStop(); + } + this.running = false; + if (this.confirmChecker != null) { + this.confirmChecker.cancel(false); + this.confirmChecker = null; + } + } + finally { + this.lock.unlock(); } } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/channel/AbstractMessageChannel.java b/spring-integration-core/src/main/java/org/springframework/integration/channel/AbstractMessageChannel.java index 3d0b48dca9e..1517392ed36 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/channel/AbstractMessageChannel.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/channel/AbstractMessageChannel.java @@ -26,6 +26,8 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import io.micrometer.observation.ObservationRegistry; @@ -70,6 +72,7 @@ * @author Oleg Zhurakousky * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov */ @IntegrationManagedResource public abstract class AbstractMessageChannel extends IntegrationObjectSupport @@ -475,6 +478,8 @@ public void destroy() { */ protected static class ChannelInterceptorList { + private final Lock lock = new ReentrantLock(); + protected final List interceptors = new CopyOnWriteArrayList<>(); // NOSONAR private final LogAccessor logger; @@ -486,11 +491,15 @@ public ChannelInterceptorList(LogAccessor logger) { } public boolean set(List interceptors) { - synchronized (this.interceptors) { + this.lock.lock(); + try { this.interceptors.clear(); this.size = interceptors.size(); return this.interceptors.addAll(interceptors); } + finally { + this.lock.unlock(); + } } public int getSize() { diff --git a/spring-integration-core/src/main/java/org/springframework/integration/channel/DefaultHeaderChannelRegistry.java b/spring-integration-core/src/main/java/org/springframework/integration/channel/DefaultHeaderChannelRegistry.java index 1e09738224b..8efc15dc6c2 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/channel/DefaultHeaderChannelRegistry.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/channel/DefaultHeaderChannelRegistry.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2022 the original author or authors. + * Copyright 2013-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -24,6 +24,8 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.springframework.integration.context.IntegrationObjectSupport; import org.springframework.integration.support.channel.HeaderChannelRegistry; @@ -44,6 +46,7 @@ * @author Gary Russell * @author Artem Bilan * @author Trung Pham + * @author Christian Tzolov * * @since 3.0 * @@ -69,6 +72,8 @@ public class DefaultHeaderChannelRegistry extends IntegrationObjectSupport private volatile boolean explicitlyStopped; + private final Lock lock = new ReentrantLock(); + /** * Construct a registry with the default delay for channel expiry. */ @@ -120,25 +125,37 @@ protected void onInit() { } @Override - public synchronized void start() { - if (!this.running) { - Assert.notNull(getTaskScheduler(), "a task scheduler is required"); - this.reaperScheduledFuture = - getTaskScheduler() - .schedule(this, Instant.now().plusMillis(this.reaperDelay)); - - this.running = true; + public void start() { + this.lock.lock(); + try { + if (!this.running) { + Assert.notNull(getTaskScheduler(), "a task scheduler is required"); + this.reaperScheduledFuture = getTaskScheduler() + .schedule(this, Instant.now().plusMillis(this.reaperDelay)); + + this.running = true; + } + } + finally { + this.lock.unlock(); } } @Override - public synchronized void stop() { - this.running = false; - if (this.reaperScheduledFuture != null) { - this.reaperScheduledFuture.cancel(true); - this.reaperScheduledFuture = null; + public void stop() { + this.lock.lock(); + try { + this.running = false; + if (this.reaperScheduledFuture != null) { + this.reaperScheduledFuture.cancel(true); + this.reaperScheduledFuture = null; + } + this.explicitlyStopped = true; + } + finally { + this.lock.unlock(); } - this.explicitlyStopped = true; + } public void stop(Runnable callback) { @@ -200,35 +217,45 @@ public MessageChannel channelNameToChannel(@Nullable String name) { * Cancel the scheduled reap task and run immediately; then reschedule. */ @Override - public synchronized void runReaper() { - if (this.reaperScheduledFuture != null) { - this.reaperScheduledFuture.cancel(true); - this.reaperScheduledFuture = null; - } + public void runReaper() { + this.lock.lock(); + try { + if (this.reaperScheduledFuture != null) { + this.reaperScheduledFuture.cancel(true); + this.reaperScheduledFuture = null; + } - run(); + run(); + } + finally { + this.lock.unlock(); + } } @Override - public synchronized void run() { - logger.trace(() -> "Reaper started; channels size=" + this.channels.size()); - Iterator> iterator = this.channels.entrySet().iterator(); - long now = System.currentTimeMillis(); - while (iterator.hasNext()) { - Entry entry = iterator.next(); - if (entry.getValue().expireAt() < now) { - logger.debug(() -> "Expiring " + entry.getKey() + " (" + entry.getValue().channel() + ")"); - iterator.remove(); + public void run() { + this.lock.lock(); + try { + logger.trace(() -> "Reaper started; channels size=" + this.channels.size()); + Iterator> iterator = this.channels.entrySet().iterator(); + long now = System.currentTimeMillis(); + while (iterator.hasNext()) { + Entry entry = iterator.next(); + if (entry.getValue().expireAt() < now) { + logger.debug(() -> "Expiring " + entry.getKey() + " (" + entry.getValue().channel() + ")"); + iterator.remove(); + } } - } - this.reaperScheduledFuture = - getTaskScheduler() - .schedule(this, Instant.now().plusMillis(this.reaperDelay)); + this.reaperScheduledFuture = getTaskScheduler() + .schedule(this, Instant.now().plusMillis(this.reaperDelay)); - logger.trace(() -> "Reaper completed; channels size=" + this.channels.size()); + logger.trace(() -> "Reaper completed; channels size=" + this.channels.size()); + } + finally { + this.lock.unlock(); + } } - protected record MessageChannelWrapper(MessageChannel channel, long expireAt) { } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/config/AbstractSimpleMessageHandlerFactoryBean.java b/spring-integration-core/src/main/java/org/springframework/integration/config/AbstractSimpleMessageHandlerFactoryBean.java index 6784a0a9beb..44ef9096dbe 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/config/AbstractSimpleMessageHandlerFactoryBean.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/config/AbstractSimpleMessageHandlerFactoryBean.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,6 +17,8 @@ package org.springframework.integration.config; import java.util.List; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.aopalliance.aop.Advice; import org.apache.commons.logging.Log; @@ -58,6 +60,7 @@ * @author Gary Russell * @author Artem Bilan * @author David Liu + * @author Christian Tzolov */ public abstract class AbstractSimpleMessageHandlerFactoryBean implements FactoryBean, ApplicationContextAware, BeanFactoryAware, BeanNameAware, @@ -65,7 +68,7 @@ public abstract class AbstractSimpleMessageHandlerFactoryBean ((Orderable) this.handler).setOrder(theOrder)); this.initialized = true; } + finally { + this.initializationMonitor.unlock(); + } initializingBean(); return this.handler; } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/config/ConsumerEndpointFactoryBean.java b/spring-integration-core/src/main/java/org/springframework/integration/config/ConsumerEndpointFactoryBean.java index 809e8b70e5c..89df271895e 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/config/ConsumerEndpointFactoryBean.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/config/ConsumerEndpointFactoryBean.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,6 +17,8 @@ package org.springframework.integration.config; import java.util.List; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.function.Function; import org.aopalliance.aop.Advice; @@ -77,6 +79,7 @@ * @author Josh Long * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov */ public class ConsumerEndpointFactoryBean implements FactoryBean, BeanFactoryAware, BeanNameAware, BeanClassLoaderAware, @@ -84,9 +87,9 @@ public class ConsumerEndpointFactoryBean private static final LogAccessor LOGGER = new LogAccessor(LogFactory.getLog(ConsumerEndpointFactoryBean.class)); - private final Object initializationMonitor = new Object(); + private final Lock initializationMonitor = new ReentrantLock(); - private final Object handlerMonitor = new Object(); + private final Lock handlerMonitor = new ReentrantLock(); private MessageHandler handler; @@ -127,7 +130,8 @@ public class ConsumerEndpointFactoryBean public void setHandler(Object handler) { Assert.isTrue(handler instanceof MessageHandler || handler instanceof ReactiveMessageHandler, "'handler' must be an instance of 'MessageHandler' or 'ReactiveMessageHandler'"); - synchronized (this.handlerMonitor) { + this.handlerMonitor.lock(); + try { Assert.isNull(this.handler, "handler cannot be overridden"); if (handler instanceof ReactiveMessageHandler) { this.handler = new ReactiveMessageHandlerAdapter((ReactiveMessageHandler) handler); @@ -136,6 +140,9 @@ public void setHandler(Object handler) { this.handler = (MessageHandler) handler; } } + finally { + this.handlerMonitor.unlock(); + } } public MessageHandler getHandler() { @@ -303,7 +310,8 @@ public Class getObjectType() { } private void initializeEndpoint() { - synchronized (this.initializationMonitor) { + this.initializationMonitor.lock(); + try { if (this.initialized) { return; } @@ -340,6 +348,9 @@ else if (channel instanceof PollableChannel) { this.endpoint.afterPropertiesSet(); this.initialized = true; } + finally { + this.initializationMonitor.unlock(); + } } private MessageChannel resolveInputChannel() { diff --git a/spring-integration-core/src/main/java/org/springframework/integration/config/IdGeneratorConfigurer.java b/spring-integration-core/src/main/java/org/springframework/integration/config/IdGeneratorConfigurer.java index 15faf354dba..b26cfc98b18 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/config/IdGeneratorConfigurer.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/config/IdGeneratorConfigurer.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,6 +19,8 @@ import java.lang.reflect.Field; import java.util.HashSet; import java.util.Set; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -39,11 +41,14 @@ * @author Mark Fisher * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov * * @since 2.0.4 */ public final class IdGeneratorConfigurer implements ApplicationListener { + private final Lock lock = new ReentrantLock(); + private static final Set GENERATOR_CONTEXT_ID = new HashSet<>(); private static volatile IdGenerator theIdGenerator; @@ -51,21 +56,28 @@ public final class IdGeneratorConfigurer implements ApplicationListener 0; - if (contextHasIdGenerator && setIdGenerator(context)) { - IdGeneratorConfigurer.GENERATOR_CONTEXT_ID.add(context.getId()); + public void onApplicationEvent(ApplicationContextEvent event) { + this.lock.lock(); + try { + + ApplicationContext context = event.getApplicationContext(); + if (event instanceof ContextRefreshedEvent) { + boolean contextHasIdGenerator = context.getBeanNamesForType(IdGenerator.class).length > 0; + if (contextHasIdGenerator && setIdGenerator(context)) { + IdGeneratorConfigurer.GENERATOR_CONTEXT_ID.add(context.getId()); + } } - } - else if (event instanceof ContextClosedEvent - && IdGeneratorConfigurer.GENERATOR_CONTEXT_ID.contains(context.getId())) { + else if (event instanceof ContextClosedEvent + && IdGeneratorConfigurer.GENERATOR_CONTEXT_ID.contains(context.getId())) { - if (IdGeneratorConfigurer.GENERATOR_CONTEXT_ID.size() == 1) { - unsetIdGenerator(); + if (IdGeneratorConfigurer.GENERATOR_CONTEXT_ID.size() == 1) { + unsetIdGenerator(); + } + IdGeneratorConfigurer.GENERATOR_CONTEXT_ID.remove(context.getId()); } - IdGeneratorConfigurer.GENERATOR_CONTEXT_ID.remove(context.getId()); + } + finally { + this.lock.unlock(); } } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/config/IdempotentReceiverAutoProxyCreator.java b/spring-integration-core/src/main/java/org/springframework/integration/config/IdempotentReceiverAutoProxyCreator.java index 4c92d03479f..5259640b627 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/config/IdempotentReceiverAutoProxyCreator.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/config/IdempotentReceiverAutoProxyCreator.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2019 the original author or authors. + * Copyright 2014-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,6 +20,8 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.springframework.aop.Advisor; import org.springframework.aop.TargetSource; @@ -38,6 +40,8 @@ * * @author Artem Bilan * @author Gary Russell + * @author Christian Tzolov + * * @since 4.1 */ @SuppressWarnings("serial") @@ -47,6 +51,8 @@ class IdempotentReceiverAutoProxyCreator extends AbstractAutoProxyCreator { private volatile Map> idempotentEndpoints; // double check locking requires volatile + private final Lock lock = new ReentrantLock(); + public void setIdempotentEndpointsMapping(List> idempotentEndpointsMapping) { Assert.notEmpty(idempotentEndpointsMapping, "'idempotentEndpointsMapping' must not be empty"); this.idempotentEndpointsMapping = idempotentEndpointsMapping; //NOSONAR (inconsistent sync) @@ -85,8 +91,9 @@ protected Object[] getAdvicesAndAdvisorsForBean(Class beanClass, String beanN } private void initIdempotentEndpointsIfNecessary() { - if (this.idempotentEndpoints == null) { //NOSONAR (inconsistent sync) - synchronized (this) { + if (this.idempotentEndpoints == null) { // NOSONAR (inconsistent sync) + this.lock.lock(); + try { if (this.idempotentEndpoints == null) { this.idempotentEndpoints = new LinkedHashMap>(); for (Map mapping : this.idempotentEndpointsMapping) { @@ -104,6 +111,9 @@ private void initIdempotentEndpointsIfNecessary() { } } } + finally { + this.lock.unlock(); + } } } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/config/SourcePollingChannelAdapterFactoryBean.java b/spring-integration-core/src/main/java/org/springframework/integration/config/SourcePollingChannelAdapterFactoryBean.java index 899365a3389..b273a893a71 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/config/SourcePollingChannelAdapterFactoryBean.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/config/SourcePollingChannelAdapterFactoryBean.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,9 @@ package org.springframework.integration.config; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; + import org.springframework.beans.factory.BeanClassLoaderAware; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryAware; @@ -41,11 +44,12 @@ * @author Oleg Zhurakousky * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov */ public class SourcePollingChannelAdapterFactoryBean implements FactoryBean, BeanFactoryAware, BeanNameAware, BeanClassLoaderAware, InitializingBean, SmartLifecycle, DisposableBean { - private final Object initializationMonitor = new Object(); + private final Lock initializationMonitor = new ReentrantLock(); private MessageSource source; @@ -158,7 +162,8 @@ public Class getObjectType() { } private void initializeAdapter() { - synchronized (this.initializationMonitor) { + this.initializationMonitor.lock(); + try { if (this.initialized) { return; } @@ -207,6 +212,9 @@ private void initializeAdapter() { this.adapter = spca; this.initialized = true; } + finally { + this.initializationMonitor.unlock(); + } } /* diff --git a/spring-integration-core/src/main/java/org/springframework/integration/config/xml/SpelPropertyAccessorsParser.java b/spring-integration-core/src/main/java/org/springframework/integration/config/xml/SpelPropertyAccessorsParser.java index dbe624f5b60..0e8c9cdf72e 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/config/xml/SpelPropertyAccessorsParser.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/config/xml/SpelPropertyAccessorsParser.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2019 the original author or authors. + * Copyright 2013-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,6 +17,8 @@ package org.springframework.integration.config.xml; import java.util.Map; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.w3c.dom.Element; import org.w3c.dom.Node; @@ -38,10 +40,14 @@ * * @author Artem Bilan * @author Gary Russell + * @author Christian Tzolov + * * @since 3.0 */ public class SpelPropertyAccessorsParser implements BeanDefinitionParser { + private final Lock lock = new ReentrantLock(); + private final Map propertyAccessors = new ManagedMap(); @Override @@ -86,17 +92,23 @@ else if (delegate.nodeNameEquals(ele, BeanDefinitionParserDelegate.REF_ELEMENT)) return null; } - private synchronized void initializeSpelPropertyAccessorRegistrarIfNecessary(ParserContext parserContext) { - if (!parserContext.getRegistry() - .containsBeanDefinition(IntegrationContextUtils.SPEL_PROPERTY_ACCESSOR_REGISTRAR_BEAN_NAME)) { - - BeanDefinitionBuilder registrarBuilder = BeanDefinitionBuilder - .genericBeanDefinition(SpelPropertyAccessorRegistrar.class) - .setRole(BeanDefinition.ROLE_INFRASTRUCTURE) - .addConstructorArgValue(this.propertyAccessors); - parserContext.getRegistry() - .registerBeanDefinition(IntegrationContextUtils.SPEL_PROPERTY_ACCESSOR_REGISTRAR_BEAN_NAME, - registrarBuilder.getBeanDefinition()); + private void initializeSpelPropertyAccessorRegistrarIfNecessary(ParserContext parserContext) { + this.lock.lock(); + try { + if (!parserContext.getRegistry() + .containsBeanDefinition(IntegrationContextUtils.SPEL_PROPERTY_ACCESSOR_REGISTRAR_BEAN_NAME)) { + + BeanDefinitionBuilder registrarBuilder = BeanDefinitionBuilder + .genericBeanDefinition(SpelPropertyAccessorRegistrar.class) + .setRole(BeanDefinition.ROLE_INFRASTRUCTURE) + .addConstructorArgValue(this.propertyAccessors); + parserContext.getRegistry() + .registerBeanDefinition(IntegrationContextUtils.SPEL_PROPERTY_ACCESSOR_REGISTRAR_BEAN_NAME, + registrarBuilder.getBeanDefinition()); + } + } + finally { + this.lock.unlock(); } } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/core/MessagingTemplate.java b/spring-integration-core/src/main/java/org/springframework/integration/core/MessagingTemplate.java index c0180fafb84..e4dcca64aad 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/core/MessagingTemplate.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/core/MessagingTemplate.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,9 @@ package org.springframework.integration.core; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; + import org.springframework.beans.BeansException; import org.springframework.beans.factory.BeanFactory; import org.springframework.integration.context.IntegrationContextUtils; @@ -31,12 +34,15 @@ * @author Oleg Zhurakousky * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov * * @since 1.0 * */ public class MessagingTemplate extends GenericMessagingTemplate { + private final Lock lock = new ReentrantLock(); + private BeanFactory beanFactory; private volatile boolean throwExceptionOnLateReplySet; @@ -84,15 +90,19 @@ public void setDefaultChannel(MessageChannel channel) { @Nullable public Message sendAndReceive(MessageChannel destination, Message requestMessage) { if (!this.throwExceptionOnLateReplySet) { - synchronized (this) { + this.lock.lock(); + try { if (!this.throwExceptionOnLateReplySet) { - IntegrationProperties integrationProperties = - IntegrationContextUtils.getIntegrationProperties(this.beanFactory); + IntegrationProperties integrationProperties = IntegrationContextUtils + .getIntegrationProperties(this.beanFactory); super.setThrowExceptionOnLateReply( integrationProperties.isMessagingTemplateThrowExceptionOnLateReply()); this.throwExceptionOnLateReplySet = true; } } + finally { + this.lock.unlock(); + } } return super.sendAndReceive(destination, requestMessage); } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/dispatcher/AbstractDispatcher.java b/spring-integration-core/src/main/java/org/springframework/integration/dispatcher/AbstractDispatcher.java index a44984d88c5..f9277568db2 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/dispatcher/AbstractDispatcher.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/dispatcher/AbstractDispatcher.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,6 +17,8 @@ package org.springframework.integration.dispatcher; import java.util.Set; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -41,6 +43,7 @@ * @author Gary Russell * @author Diego Belfer * @author Artem Bilan + * @author Christian Tzolov */ public abstract class AbstractDispatcher implements MessageDispatcher { @@ -52,6 +55,8 @@ public abstract class AbstractDispatcher implements MessageDispatcher { private volatile MessageHandler theOneHandler; + private final Lock lock = new ReentrantLock(); + /** * Set the maximum subscribers allowed by this dispatcher. * @param maxSubscribers The maximum number of subscribers allowed. @@ -77,17 +82,23 @@ protected Set getHandlers() { * @return the result of {@link Set#add(Object)} */ @Override - public synchronized boolean addHandler(MessageHandler handler) { - Assert.notNull(handler, "handler must not be null"); - Assert.isTrue(this.handlers.size() < this.maxSubscribers, "Maximum subscribers exceeded"); - boolean added = this.handlers.add(handler); - if (this.handlers.size() == 1) { - this.theOneHandler = handler; + public boolean addHandler(MessageHandler handler) { + this.lock.lock(); + try { + Assert.notNull(handler, "handler must not be null"); + Assert.isTrue(this.handlers.size() < this.maxSubscribers, "Maximum subscribers exceeded"); + boolean added = this.handlers.add(handler); + if (this.handlers.size() == 1) { + this.theOneHandler = handler; + } + else { + this.theOneHandler = null; + } + return added; } - else { - this.theOneHandler = null; + finally { + this.lock.unlock(); } - return added; } /** @@ -96,16 +107,22 @@ public synchronized boolean addHandler(MessageHandler handler) { * @return the result of {@link Set#remove(Object)} */ @Override - public synchronized boolean removeHandler(MessageHandler handler) { - Assert.notNull(handler, "handler must not be null"); - boolean removed = this.handlers.remove(handler); - if (this.handlers.size() == 1) { - this.theOneHandler = this.handlers.iterator().next(); + public boolean removeHandler(MessageHandler handler) { + this.lock.lock(); + try { + Assert.notNull(handler, "handler must not be null"); + boolean removed = this.handlers.remove(handler); + if (this.handlers.size() == 1) { + this.theOneHandler = this.handlers.iterator().next(); + } + else { + this.theOneHandler = null; + } + return removed; } - else { - this.theOneHandler = null; + finally { + this.lock.unlock(); } - return removed; } protected boolean tryOptimizedDispatch(Message message) { diff --git a/spring-integration-core/src/main/java/org/springframework/integration/dispatcher/PartitionedDispatcher.java b/spring-integration-core/src/main/java/org/springframework/integration/dispatcher/PartitionedDispatcher.java index 1883f190fa1..afb8de28595 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/dispatcher/PartitionedDispatcher.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/dispatcher/PartitionedDispatcher.java @@ -25,6 +25,8 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.ThreadFactory; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.function.Function; import org.springframework.integration.util.ErrorHandlingTaskExecutor; @@ -49,6 +51,7 @@ * The rest of the logic is similar to {@link UnicastingDispatcher} behavior. * * @author Artem Bilan + * @author Christian Tzolov * * @since 6.1 */ @@ -73,6 +76,8 @@ public class PartitionedDispatcher extends AbstractDispatcher { private MessageHandlingTaskDecorator messageHandlingTaskDecorator = task -> task; + private final Lock lock = new ReentrantLock(); + /** * Instantiate based on a provided number of partitions and function for partition key against * the message to dispatch. @@ -153,7 +158,8 @@ public boolean dispatch(Message message) { private void populatedPartitions() { if (this.partitions.isEmpty()) { - synchronized (this.partitions) { + this.lock.lock(); + try { if (this.partitions.isEmpty()) { Map partitionsToUse = new HashMap<>(); for (int i = 0; i < this.partitionCount; i++) { @@ -162,6 +168,9 @@ private void populatedPartitions() { this.partitions.putAll(partitionsToUse); } } + finally { + this.lock.unlock(); + } } } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/endpoint/AbstractPollingEndpoint.java b/spring-integration-core/src/main/java/org/springframework/integration/endpoint/AbstractPollingEndpoint.java index 68cafafc58e..ad8acb75365 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/endpoint/AbstractPollingEndpoint.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/endpoint/AbstractPollingEndpoint.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -24,6 +24,8 @@ import java.util.concurrent.Callable; import java.util.concurrent.Executor; import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.stream.Collectors; import org.aopalliance.aop.Advice; @@ -78,6 +80,7 @@ * @author Gary Russell * @author Artem Bilan * @author Andreas Baer + * @author Christian Tzolov */ public abstract class AbstractPollingEndpoint extends AbstractEndpoint implements BeanClassLoaderAware { @@ -88,7 +91,7 @@ public abstract class AbstractPollingEndpoint extends AbstractEndpoint implement private final Collection appliedAdvices = new HashSet<>(); - private final Object initializationMonitor = new Object(); + private final Lock initializationMonitor = new ReentrantLock(); private Executor taskExecutor = new SyncTaskExecutor(); @@ -262,7 +265,8 @@ protected void setReceiveMessageSource(Object source) { @Override protected void onInit() { - synchronized (this.initializationMonitor) { + this.initializationMonitor.lock(); + try { if (this.initialized) { return; } @@ -280,6 +284,9 @@ protected void onInit() { } this.initialized = true; } + finally { + this.initializationMonitor.unlock(); + } try { super.onInit(); } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/endpoint/MethodInvokingMessageSource.java b/spring-integration-core/src/main/java/org/springframework/integration/endpoint/MethodInvokingMessageSource.java index e535ee92e63..f54ee39aaba 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/endpoint/MethodInvokingMessageSource.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/endpoint/MethodInvokingMessageSource.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,6 +17,8 @@ package org.springframework.integration.endpoint; import java.lang.reflect.Method; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.springframework.context.Lifecycle; import org.springframework.integration.support.management.ManageableLifecycle; @@ -31,6 +33,7 @@ * @author Mark Fisher * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov */ public class MethodInvokingMessageSource extends AbstractMessageSource implements ManageableLifecycle { @@ -42,7 +45,7 @@ public class MethodInvokingMessageSource extends AbstractMessageSource i private volatile boolean initialized; - private final Object initializationMonitor = new Object(); + private final Lock initializationMonitor = new ReentrantLock(); public void setObject(Object object) { @@ -67,7 +70,8 @@ public String getComponentType() { @Override protected void onInit() { - synchronized (this.initializationMonitor) { + this.initializationMonitor.lock(); + try { if (this.initialized) { return; } @@ -83,6 +87,9 @@ protected void onInit() { ReflectionUtils.makeAccessible(this.method); this.initialized = true; } + finally { + this.initializationMonitor.unlock(); + } } @Override diff --git a/spring-integration-core/src/main/java/org/springframework/integration/endpoint/SourcePollingChannelAdapter.java b/spring-integration-core/src/main/java/org/springframework/integration/endpoint/SourcePollingChannelAdapter.java index 1c4c5090c82..b76b69e76cf 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/endpoint/SourcePollingChannelAdapter.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/endpoint/SourcePollingChannelAdapter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,9 @@ package org.springframework.integration.endpoint; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; + import org.springframework.aop.framework.Advised; import org.springframework.beans.factory.BeanCreationException; import org.springframework.context.Lifecycle; @@ -43,6 +46,7 @@ * @author Oleg Zhurakousky * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov */ public class SourcePollingChannelAdapter extends AbstractPollingEndpoint implements TrackableComponent { @@ -59,6 +63,8 @@ public class SourcePollingChannelAdapter extends AbstractPollingEndpoint private volatile boolean shouldTrack; + private final Lock lock = new ReentrantLock(); + /** * Specify the source to be polled for Messages. * @@ -175,12 +181,16 @@ protected void onInit() { public MessageChannel getOutputChannel() { if (this.outputChannelName != null) { - synchronized (this) { + this.lock.lock(); + try { if (this.outputChannelName != null) { this.outputChannel = getChannelResolver().resolveDestination(this.outputChannelName); this.outputChannelName = null; } } + finally { + this.lock.unlock(); + } } return this.outputChannel; } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/expression/ReloadableResourceBundleExpressionSource.java b/spring-integration-core/src/main/java/org/springframework/integration/expression/ReloadableResourceBundleExpressionSource.java index a9957ca0954..37d3d7fb339 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/expression/ReloadableResourceBundleExpressionSource.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/expression/ReloadableResourceBundleExpressionSource.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -25,6 +25,8 @@ import java.util.Locale; import java.util.Map; import java.util.Properties; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -53,6 +55,7 @@ * @author Mark Fisher * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov * * @since 2.0 * @@ -80,16 +83,22 @@ public class ReloadableResourceBundleExpressionSource implements ExpressionSourc */ private final Map>> cachedFilenames = new HashMap<>(); + private final Lock cachedFilenamesMonitor = new ReentrantLock(); + /** * Cache to hold already loaded properties per filename. */ private final Map cachedProperties = new HashMap<>(); + private final Lock cachedPropertiesMonitor = new ReentrantLock(); + /** * Cache to hold merged loaded properties per locale. */ private final Map cachedMergedProperties = new HashMap<>(); + private final Lock cachedMergedPropertiesMonitor = new ReentrantLock(); + private final ExpressionParser parser = new SpelExpressionParser(new SpelParserConfiguration(true, true)); private String[] basenames = {}; @@ -282,7 +291,8 @@ private String getExpressionString(String key, Locale locale) { * cached forever. */ private PropertiesHolder getMergedProperties(Locale locale) { - synchronized (this.cachedMergedProperties) { + this.cachedMergedPropertiesMonitor.lock(); + try { PropertiesHolder mergedHolder = this.cachedMergedProperties.get(locale); if (mergedHolder != null) { return mergedHolder; @@ -303,6 +313,9 @@ private PropertiesHolder getMergedProperties(Locale locale) { this.cachedMergedProperties.put(locale, mergedHolder); return mergedHolder; } + finally { + this.cachedMergedPropertiesMonitor.unlock(); + } } /** @@ -316,7 +329,8 @@ private PropertiesHolder getMergedProperties(Locale locale) { * @see #calculateFilenamesForLocale */ private List calculateAllFilenames(String basename, Locale locale) { - synchronized (this.cachedFilenames) { + this.cachedFilenamesMonitor.lock(); + try { Map> localeMap = this.cachedFilenames.get(basename); if (localeMap != null) { List filenames = localeMap.get(locale); @@ -345,6 +359,9 @@ private List calculateAllFilenames(String basename, Locale locale) { } return filenames; } + finally { + this.cachedFilenamesMonitor.unlock(); + } } /** @@ -392,7 +409,8 @@ private List calculateFilenamesForLocale(String basename, Locale locale) * @return the current PropertiesHolder for the bundle */ private PropertiesHolder getProperties(String filename) { - synchronized (this.cachedProperties) { + this.cachedPropertiesMonitor.lock(); + try { PropertiesHolder propHolder = this.cachedProperties.get(filename); if (propHolder != null && (propHolder.getRefreshTimestamp() < 0 || @@ -401,6 +419,9 @@ private PropertiesHolder getProperties(String filename) { } return refreshProperties(filename, propHolder); } + finally { + this.cachedPropertiesMonitor.unlock(); + } } /** @@ -539,12 +560,21 @@ private void loadFromProperties(Resource resource, String filename, InputStream */ public void clearCache() { LOGGER.debug("Clearing entire resource bundle cache"); - synchronized (this.cachedProperties) { + this.cachedPropertiesMonitor.lock(); + try { this.cachedProperties.clear(); } - synchronized (this.cachedMergedProperties) { + finally { + this.cachedPropertiesMonitor.unlock(); + } + + this.cachedMergedPropertiesMonitor.lock(); + try { this.cachedMergedProperties.clear(); } + finally { + this.cachedMergedPropertiesMonitor.unlock(); + } } @Override diff --git a/spring-integration-core/src/main/java/org/springframework/integration/gateway/GatewayMessageHandler.java b/spring-integration-core/src/main/java/org/springframework/integration/gateway/GatewayMessageHandler.java index 4293fe14472..76cc64400a7 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/gateway/GatewayMessageHandler.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/gateway/GatewayMessageHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2022 the original author or authors. + * Copyright 2016-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,9 @@ package org.springframework.integration.gateway; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; + import org.springframework.beans.factory.BeanCreationException; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; @@ -28,6 +31,7 @@ * The {@link AbstractReplyProducingMessageHandler} implementation for mid-flow Gateway. * * @author Artem Bilan + * @author Christian Tzolov * * @since 5.0 */ @@ -39,6 +43,8 @@ public class GatewayMessageHandler extends AbstractReplyProducingMessageHandler private volatile boolean running; + private final Lock lock = new ReentrantLock(); + public GatewayMessageHandler() { this.gatewayProxyFactoryBean = new GatewayProxyFactoryBean<>(); } @@ -78,11 +84,15 @@ public void setReplyTimeout(Long replyTimeout) { @Override protected Object handleRequestMessage(Message requestMessage) { if (this.exchanger == null) { - synchronized (this) { + this.lock.lock(); + try { if (this.exchanger == null) { initialize(); } } + finally { + this.lock.unlock(); + } } return this.exchanger.exchange(requestMessage); } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/gateway/GatewayProxyFactoryBean.java b/spring-integration-core/src/main/java/org/springframework/integration/gateway/GatewayProxyFactoryBean.java index bdb953d78dc..4e980042932 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/gateway/GatewayProxyFactoryBean.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/gateway/GatewayProxyFactoryBean.java @@ -29,6 +29,8 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.Executor; import java.util.concurrent.Future; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Supplier; @@ -106,12 +108,13 @@ * @author Gary Russell * @author Artem Bilan * @author JingPeng Xie + * @author Christian Tzolov */ public class GatewayProxyFactoryBean extends AbstractEndpoint implements TrackableComponent, FactoryBean, MethodInterceptor, BeanClassLoaderAware, IntegrationManagement { - private final Object initializationMonitor = new Object(); + private final Lock initializationMonitor = new ReentrantLock(); private final Map gatewayMap = new HashMap<>(); @@ -455,7 +458,8 @@ public void registerMetricsCaptor(MetricsCaptor metricsCaptorToRegister) { @Override @SuppressWarnings("unchecked") protected void onInit() { - synchronized (this.initializationMonitor) { + this.initializationMonitor.lock(); + try { if (this.initialized) { return; } @@ -466,13 +470,15 @@ protected void onInit() { populateMethodInvocationGateways(); - ProxyFactory gatewayProxyFactory = - new ProxyFactory(this.serviceInterface, this); + ProxyFactory gatewayProxyFactory = new ProxyFactory(this.serviceInterface, this); gatewayProxyFactory.addAdvice(new DefaultMethodInvokingMethodInterceptor()); this.serviceProxy = (T) gatewayProxyFactory.getProxy(this.beanClassLoader); this.evaluationContext = ExpressionUtils.createStandardEvaluationContext(beanFactory); this.initialized = true; } + finally { + this.initializationMonitor.unlock(); + } } private void populateMethodInvocationGateways() { diff --git a/spring-integration-core/src/main/java/org/springframework/integration/gateway/MessagingGatewaySupport.java b/spring-integration-core/src/main/java/org/springframework/integration/gateway/MessagingGatewaySupport.java index 5f048953637..a8cfcbc3448 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/gateway/MessagingGatewaySupport.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/gateway/MessagingGatewaySupport.java @@ -19,6 +19,8 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import io.micrometer.observation.ObservationRegistry; import org.reactivestreams.Publisher; @@ -86,6 +88,7 @@ * @author Gary Russell * @author Artem Bilan * @author Trung Pham + * @author Christian Tzolov */ @IntegrationManagedResource public abstract class MessagingGatewaySupport extends AbstractEndpoint @@ -99,7 +102,7 @@ public abstract class MessagingGatewaySupport extends AbstractEndpoint private final HistoryWritingMessagePostProcessor historyWritingPostProcessor = new HistoryWritingMessagePostProcessor(); - private final Object replyMessageCorrelatorMonitor = new Object(); + private final Lock replyMessageCorrelatorMonitor = new ReentrantLock(); private final ManagementOverrides managementOverrides = new ManagementOverrides(); @@ -892,7 +895,8 @@ private RuntimeException wrapExceptionIfNecessary(Throwable t, String descriptio protected void registerReplyMessageCorrelatorIfNecessary() { MessageChannel replyChan = getReplyChannel(); if (replyChan != null && this.replyMessageCorrelator == null) { - synchronized (this.replyMessageCorrelatorMonitor) { + this.replyMessageCorrelatorMonitor.lock(); + try { if (this.replyMessageCorrelator != null) { return; } @@ -923,6 +927,9 @@ else if (replyChan instanceof ReactiveStreamsSubscribableChannel) { correlator.afterPropertiesSet(); this.replyMessageCorrelator = correlator; } + finally { + this.replyMessageCorrelatorMonitor.unlock(); + } if (isRunning()) { this.replyMessageCorrelator.start(); } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/graph/IntegrationGraphServer.java b/spring-integration-core/src/main/java/org/springframework/integration/graph/IntegrationGraphServer.java index 1cd1ecd56ef..1e11a2c8c86 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/graph/IntegrationGraphServer.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/graph/IntegrationGraphServer.java @@ -26,6 +26,8 @@ import java.util.Map.Entry; import java.util.Objects; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -61,6 +63,7 @@ * * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov * * @since 4.3 * @@ -70,6 +73,8 @@ public class IntegrationGraphServer implements ApplicationContextAware, Applicat private static final float GRAPH_VERSION = 1.2f; + private final Lock lock = new ReentrantLock(); + private final NodeFactory nodeFactory = new NodeFactory(this::enhance); private MicrometerNodeEnhancer micrometerEnhancer; @@ -127,12 +132,16 @@ public void onApplicationEvent(ContextRefreshedEvent event) { * @see #rebuild() */ public Graph getGraph() { - if (this.graph == null) { //NOSONAR (sync) - synchronized (this) { + if (this.graph == null) { // NOSONAR (sync) + this.lock.lock(); + try { if (this.graph == null) { buildGraph(); } } + finally { + this.lock.unlock(); + } } return this.graph; } @@ -169,35 +178,41 @@ private T enhance(T node) { } } - private synchronized Graph buildGraph() { - if (this.micrometerEnhancer == null && MicrometerMetricsCaptorConfiguration.METER_REGISTRY_PRESENT) { - this.micrometerEnhancer = new MicrometerNodeEnhancer(this.applicationContext); - } - String implementationVersion = IntegrationGraphServer.class.getPackage().getImplementationVersion(); - if (implementationVersion == null) { - implementationVersion = "unknown - is Spring Integration running from the distribution jar?"; - } - Map descriptor = new HashMap<>(); - descriptor.put("provider", "spring-integration"); - descriptor.put("providerVersion", implementationVersion); - descriptor.put("providerFormatVersion", GRAPH_VERSION); - String name = this.applicationName; - if (name == null) { - name = this.applicationContext.getEnvironment().getProperty("spring.application.name"); + private Graph buildGraph() { + this.lock.lock(); + try { + if (this.micrometerEnhancer == null && MicrometerMetricsCaptorConfiguration.METER_REGISTRY_PRESENT) { + this.micrometerEnhancer = new MicrometerNodeEnhancer(this.applicationContext); + } + String implementationVersion = IntegrationGraphServer.class.getPackage().getImplementationVersion(); + if (implementationVersion == null) { + implementationVersion = "unknown - is Spring Integration running from the distribution jar?"; + } + Map descriptor = new HashMap<>(); + descriptor.put("provider", "spring-integration"); + descriptor.put("providerVersion", implementationVersion); + descriptor.put("providerFormatVersion", GRAPH_VERSION); + String name = this.applicationName; + if (name == null) { + name = this.applicationContext.getEnvironment().getProperty("spring.application.name"); + } + if (name != null) { + descriptor.put("name", name); + } + this.nodeFactory.reset(); + Collection nodes = new ArrayList<>(); + Collection links = new ArrayList<>(); + Map channelNodes = channels(nodes); + pollingAdapters(nodes, links, channelNodes); + gateways(nodes, links, channelNodes); + producers(nodes, links, channelNodes); + consumers(nodes, links, channelNodes); + this.graph = new Graph(descriptor, nodes, links); + return this.graph; } - if (name != null) { - descriptor.put("name", name); + finally { + this.lock.unlock(); } - this.nodeFactory.reset(); - Collection nodes = new ArrayList<>(); - Collection links = new ArrayList<>(); - Map channelNodes = channels(nodes); - pollingAdapters(nodes, links, channelNodes); - gateways(nodes, links, channelNodes); - producers(nodes, links, channelNodes); - consumers(nodes, links, channelNodes); - this.graph = new Graph(descriptor, nodes, links); - return this.graph; } private Map channels(Collection nodes) { diff --git a/spring-integration-core/src/main/java/org/springframework/integration/handler/AbstractReplyProducingMessageHandler.java b/spring-integration-core/src/main/java/org/springframework/integration/handler/AbstractReplyProducingMessageHandler.java index d6134f07764..3a2086e614d 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/handler/AbstractReplyProducingMessageHandler.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/handler/AbstractReplyProducingMessageHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,6 +18,8 @@ import java.util.LinkedList; import java.util.List; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.aopalliance.aop.Advice; @@ -41,10 +43,13 @@ * @author Artem Bilan * @author David Liu * @author Trung Pham + * @author Christian Tzolov */ public abstract class AbstractReplyProducingMessageHandler extends AbstractMessageProducingHandler implements BeanClassLoaderAware { + private final Lock lock = new ReentrantLock(); + private final List adviceChain = new LinkedList<>(); private ClassLoader beanClassLoader = ClassUtils.getDefaultClassLoader(); @@ -72,13 +77,17 @@ protected boolean getRequiresReply() { */ public void setAdviceChain(List adviceChain) { Assert.notEmpty(adviceChain, "adviceChain cannot be empty"); - synchronized (this.adviceChain) { + this.lock.lock(); + try { this.adviceChain.clear(); this.adviceChain.addAll(adviceChain); if (isInitialized()) { initAdvisedRequestHandlerIfAny(); } } + finally { + this.lock.unlock(); + } } protected boolean hasAdviceChain() { diff --git a/spring-integration-core/src/main/java/org/springframework/integration/handler/DelayHandler.java b/spring-integration-core/src/main/java/org/springframework/integration/handler/DelayHandler.java index 74a619faa30..690c1a18a97 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/handler/DelayHandler.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/handler/DelayHandler.java @@ -98,6 +98,7 @@ * @author Mark Fisher * @author Artem Bilan * @author Gary Russell + * @author Christian Tzolov * * @since 1.0.3 */ @@ -110,6 +111,8 @@ public class DelayHandler extends AbstractReplyProducingMessageHandler implement public static final long DEFAULT_RETRY_DELAY = 1_000; + private final Lock lock = new ReentrantLock(); + private final ConcurrentMap deliveries = new ConcurrentHashMap<>(); private final Lock removeReleasedMessageLock = new ReentrantLock(); @@ -618,22 +621,28 @@ public int getDelayedMessageCount() { * behavior is dictated by the avoidance of invocation thread overload. */ @Override - public synchronized void reschedulePersistedMessages() { - MessageGroup messageGroup = this.messageStore.getMessageGroup(this.messageGroupId); - try (Stream> messageStream = messageGroup.streamMessages()) { - TaskScheduler taskScheduler = getTaskScheduler(); - messageStream.forEach((message) -> // NOSONAR - taskScheduler.schedule(() -> { - // This is fine to keep the reference to the message, - // because the scheduled task is performed immediately. - long delay = determineDelayForMessage(message); - if (delay > 0) { - releaseMessageAfterDelay(message, delay); - } - else { - releaseMessage(message); - } - }, Instant.now())); + public void reschedulePersistedMessages() { + this.lock.lock(); + try { + MessageGroup messageGroup = this.messageStore.getMessageGroup(this.messageGroupId); + try (Stream> messageStream = messageGroup.streamMessages()) { + TaskScheduler taskScheduler = getTaskScheduler(); + messageStream.forEach((message) -> // NOSONAR + taskScheduler.schedule(() -> { + // This is fine to keep the reference to the message, + // because the scheduled task is performed immediately. + long delay = determineDelayForMessage(message); + if (delay > 0) { + releaseMessageAfterDelay(message, delay); + } + else { + releaseMessage(message); + } + }, Instant.now())); + } + } + finally { + this.lock.unlock(); } } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/handler/MessageHandlerChain.java b/spring-integration-core/src/main/java/org/springframework/integration/handler/MessageHandlerChain.java index 431a9f8f870..3f08fb285a1 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/handler/MessageHandlerChain.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/handler/MessageHandlerChain.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,6 +20,7 @@ import java.util.HashSet; import java.util.LinkedList; import java.util.List; +import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; import org.springframework.context.Lifecycle; @@ -66,11 +67,12 @@ * @author Gary Russell * @author Artem Bilan * @author Trung Pham + * @author Christian Tzolov */ public class MessageHandlerChain extends AbstractMessageProducingHandler implements CompositeMessageHandler, ManageableLifecycle { - private final Object initializationMonitor = new Object(); + private final Lock initializationMonitor = new ReentrantLock(); private final ReentrantLock lifecycleLock = new ReentrantLock(); @@ -102,13 +104,17 @@ public IntegrationPatternType getIntegrationPatternType() { @Override protected void onInit() { super.onInit(); - synchronized (this.initializationMonitor) { + this.initializationMonitor.lock(); + try { if (!this.initialized) { Assert.notEmpty(this.handlers, "handler list must not be empty"); configureChain(); this.initialized = true; } } + finally { + this.initializationMonitor.unlock(); + } } private void configureChain() { diff --git a/spring-integration-core/src/main/java/org/springframework/integration/handler/support/MessagingMethodInvokerHelper.java b/spring-integration-core/src/main/java/org/springframework/integration/handler/support/MessagingMethodInvokerHelper.java index f2b2453ce09..10ea3db87c6 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/handler/support/MessagingMethodInvokerHelper.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/handler/support/MessagingMethodInvokerHelper.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -34,6 +34,8 @@ import java.util.Properties; import java.util.Set; import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.function.Consumer; import java.util.function.Function; @@ -119,6 +121,8 @@ * @author Gary Russell * @author Artem Bilan * @author Trung Pham + * @author Christian Tzolov + * * @since 2.0 */ public class MessagingMethodInvokerHelper extends AbstractExpressionEvaluator implements ManageableLifecycle { @@ -160,6 +164,8 @@ public class MessagingMethodInvokerHelper extends AbstractExpressionEvaluator im SPEL_COMPILERS.put(SpelCompilerMode.MIXED, EXPRESSION_PARSER_MIXED); } + private final Lock lock = new ReentrantLock(); + private final Object targetObject; private final JsonObjectMapper jsonObjectMapper; @@ -486,23 +492,28 @@ private Object processInternal(ParametersWrapper parameters) { } } - private synchronized void initialize() { - if (isProvidedMessageHandlerFactoryBean()) { - LOGGER.trace("Overriding default instance of MessageHandlerMethodFactory with the one provided."); - this.messageHandlerMethodFactory = - getBeanFactory() - .getBean( - this.canProcessMessageList - ? IntegrationContextUtils.LIST_MESSAGE_HANDLER_FACTORY_BEAN_NAME - : IntegrationContextUtils.MESSAGE_HANDLER_FACTORY_BEAN_NAME, - MessageHandlerMethodFactory.class); + private void initialize() { + this.lock.lock(); + try { + if (isProvidedMessageHandlerFactoryBean()) { + LOGGER.trace("Overriding default instance of MessageHandlerMethodFactory with the one provided."); + this.messageHandlerMethodFactory = getBeanFactory() + .getBean( + this.canProcessMessageList + ? IntegrationContextUtils.LIST_MESSAGE_HANDLER_FACTORY_BEAN_NAME + : IntegrationContextUtils.MESSAGE_HANDLER_FACTORY_BEAN_NAME, + MessageHandlerMethodFactory.class); + } + else { + configureLocalMessageHandlerFactory(); + } + + prepareEvaluationContext(); + this.initialized = true; } - else { - configureLocalMessageHandlerFactory(); + finally { + this.lock.unlock(); } - - prepareEvaluationContext(); - this.initialized = true; } private boolean isProvidedMessageHandlerFactoryBean() { diff --git a/spring-integration-core/src/main/java/org/springframework/integration/history/MessageHistoryConfigurer.java b/spring-integration-core/src/main/java/org/springframework/integration/history/MessageHistoryConfigurer.java index 721fffe5fdb..18e3a96af8f 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/history/MessageHistoryConfigurer.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/history/MessageHistoryConfigurer.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,6 +20,8 @@ import java.util.Collection; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -44,6 +46,7 @@ * @author Mark Fisher * @author Artem Bilan * @author Gary Russell + * @author Christian Tzolov * * @since 2.0 */ @@ -54,6 +57,8 @@ public class MessageHistoryConfigurer implements ManageableSmartLifecycle, BeanF private static final Log LOGGER = LogFactory.getLog(MessageHistoryConfigurer.class); + private final Lock lock = new ReentrantLock(); + private final Set currentlyTrackedComponents = ConcurrentHashMap.newKeySet(); private String[] componentNamePatterns = {"*"}; @@ -180,7 +185,8 @@ public int getPhase() { @ManagedOperation @Override public void start() { - synchronized (this.currentlyTrackedComponents) { + this.lock.lock(); + try { if (!this.running) { for (TrackableComponent component : getTrackableComponents(this.beanFactory)) { trackComponentIfAny(component); @@ -188,12 +194,16 @@ public void start() { } } } + finally { + this.lock.unlock(); + } } @ManagedOperation @Override public void stop() { - synchronized (this.currentlyTrackedComponents) { + this.lock.lock(); + try { if (this.running) { this.currentlyTrackedComponents.forEach(component -> { component.setShouldTrack(false); @@ -207,6 +217,9 @@ public void stop() { this.running = false; } } + finally { + this.lock.unlock(); + } } private static Collection getTrackableComponents(ListableBeanFactory beanFactory) { diff --git a/spring-integration-core/src/main/java/org/springframework/integration/router/AbstractMessageRouter.java b/spring-integration-core/src/main/java/org/springframework/integration/router/AbstractMessageRouter.java index f412c9ad386..2a89d13dabb 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/router/AbstractMessageRouter.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/router/AbstractMessageRouter.java @@ -18,6 +18,8 @@ import java.util.Collection; import java.util.UUID; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.springframework.beans.factory.BeanFactory; import org.springframework.core.convert.ConversionService; @@ -43,11 +45,14 @@ * @author Soby Chacko * @author Stefan Ferstl * @author Artem Bilan + * @author Christian Tzolov */ @ManagedResource @IntegrationManagedResource public abstract class AbstractMessageRouter extends AbstractMessageHandler implements MessageRouter { + private final Lock lock = new ReentrantLock(); + private final MessagingTemplate messagingTemplate = new MessagingTemplate(); private volatile MessageChannel defaultOutputChannel; @@ -83,12 +88,16 @@ public void setDefaultOutputChannel(MessageChannel defaultOutputChannel) { @Override public MessageChannel getDefaultOutputChannel() { if (this.defaultOutputChannelName != null) { - synchronized (this) { + this.lock.lock(); + try { if (this.defaultOutputChannelName != null) { this.defaultOutputChannel = getChannelResolver().resolveDestination(this.defaultOutputChannelName); this.defaultOutputChannelName = null; } } + finally { + this.lock.unlock(); + } } return this.defaultOutputChannel; } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/selector/MessageSelectorChain.java b/spring-integration-core/src/main/java/org/springframework/integration/selector/MessageSelectorChain.java index f4824fc7f39..b3faa24eaa4 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/selector/MessageSelectorChain.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/selector/MessageSelectorChain.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,6 +18,8 @@ import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.springframework.integration.core.MessageSelector; import org.springframework.messaging.Message; @@ -32,9 +34,12 @@ * @author Mark Fisher * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov */ public class MessageSelectorChain implements MessageSelector { + private final Lock lock = new ReentrantLock(); + private volatile VotingStrategy votingStrategy = VotingStrategy.ALL; private final List selectors = new CopyOnWriteArrayList<>(); @@ -72,10 +77,14 @@ public void add(int index, MessageSelector selector) { */ public void setSelectors(List selectors) { Assert.notEmpty(selectors, "selectors must not be empty"); - synchronized (this.selectors) { + this.lock.lock(); + try { this.selectors.clear(); this.selectors.addAll(selectors); } + finally { + this.lock.unlock(); + } } /** diff --git a/spring-integration-core/src/main/java/org/springframework/integration/selector/MetadataStoreSelector.java b/spring-integration-core/src/main/java/org/springframework/integration/selector/MetadataStoreSelector.java index 4ec2ac066e5..094973a8060 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/selector/MetadataStoreSelector.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/selector/MetadataStoreSelector.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2020 the original author or authors. + * Copyright 2014-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,8 @@ package org.springframework.integration.selector; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.function.BiPredicate; import org.springframework.integration.core.MessageSelector; @@ -50,11 +52,14 @@ * * @author Artem Bilan * @author Gary Russell + * @author Christian Tzolov * * @since 4.1 */ public class MetadataStoreSelector implements MessageSelector { + private final Lock lock = new ReentrantLock(); + private final ConcurrentMetadataStore metadataStore; private final MessageProcessor keyStrategy; @@ -119,7 +124,8 @@ public boolean accept(Message message) { return this.metadataStore.putIfAbsent(key, value) == null; } else { - synchronized (this) { + this.lock.lock(); + try { String oldValue = this.metadataStore.get(key); if (oldValue == null) { return this.metadataStore.putIfAbsent(key, value) == null; @@ -129,6 +135,9 @@ public boolean accept(Message message) { } return false; } + finally { + this.lock.unlock(); + } } } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/store/AbstractMessageGroupStore.java b/spring-integration-core/src/main/java/org/springframework/integration/store/AbstractMessageGroupStore.java index bb9c7cc727b..8496a51bf1a 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/store/AbstractMessageGroupStore.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/store/AbstractMessageGroupStore.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,6 +19,8 @@ import java.util.Arrays; import java.util.Collection; import java.util.LinkedHashSet; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -33,6 +35,7 @@ * @author Oleg Zhurakousky * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov * * @since 2.0 */ @@ -42,6 +45,8 @@ public abstract class AbstractMessageGroupStore extends AbstractBatchingMessageG protected final Log logger = LogFactory.getLog(getClass()); // NOSONAR final + private final Lock lock = new ReentrantLock(); + private final Collection expiryCallbacks = new LinkedHashSet<>(); private final MessageGroupFactory persistentMessageGroupFactory = @@ -122,22 +127,28 @@ public void registerMessageGroupExpiryCallback(MessageGroupCallback callback) { @Override @ManagedOperation - public synchronized int expireMessageGroups(long timeout) { - int count = 0; - long threshold = System.currentTimeMillis() - timeout; - for (MessageGroup group : this) { - - long timestamp = group.getTimestamp(); - if (this.isTimeoutOnIdle() && group.getLastModified() > 0) { - timestamp = group.getLastModified(); - } + public int expireMessageGroups(long timeout) { + this.lock.lock(); + try { + int count = 0; + long threshold = System.currentTimeMillis() - timeout; + for (MessageGroup group : this) { + + long timestamp = group.getTimestamp(); + if (this.isTimeoutOnIdle() && group.getLastModified() > 0) { + timestamp = group.getLastModified(); + } - if (timestamp <= threshold) { - count++; - expire(copy(group)); + if (timestamp <= threshold) { + count++; + expire(copy(group)); + } } + return count; + } + finally { + this.lock.unlock(); } - return count; } /** diff --git a/spring-integration-core/src/main/java/org/springframework/integration/store/PersistentMessageGroup.java b/spring-integration-core/src/main/java/org/springframework/integration/store/PersistentMessageGroup.java index 4e1cd00efb4..68e245a4f17 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/store/PersistentMessageGroup.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/store/PersistentMessageGroup.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2021 the original author or authors. + * Copyright 2016-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,6 +21,8 @@ import java.util.Collections; import java.util.Iterator; import java.util.Spliterator; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.stream.Stream; import org.apache.commons.logging.Log; @@ -32,6 +34,7 @@ /** * @author Artem Bilan + * @author Christian Tzolov * * @since 4.3 */ @@ -39,6 +42,8 @@ class PersistentMessageGroup implements MessageGroup { private static final Log LOGGER = LogFactory.getLog(PersistentMessageGroup.class); + private final Lock lock = new ReentrantLock(); + private final MessageGroupStore messageGroupStore; private final Collection> messages = new PersistentCollection(); @@ -76,7 +81,8 @@ public Stream> streamMessages() { @Override public Message getOne() { if (this.oneMessage == null) { - synchronized (this) { + this.lock.lock(); + try { if (this.oneMessage == null) { if (LOGGER.isDebugEnabled()) { LOGGER.debug("Lazy loading of one message for messageGroup: " + this.original.getGroupId()); @@ -84,6 +90,9 @@ public Message getOne() { this.oneMessage = this.messageGroupStore.getOneMessageFromGroup(this.original.getGroupId()); } } + finally { + this.lock.unlock(); + } } return this.oneMessage; } @@ -109,7 +118,8 @@ public int getSequenceSize() { @Override public int size() { if (this.size == 0) { - synchronized (this) { + this.lock.lock(); + try { if (this.size == 0) { if (LOGGER.isDebugEnabled()) { LOGGER.debug("Lazy loading of group size for messageGroup: " + this.original.getGroupId()); @@ -117,6 +127,9 @@ public int size() { this.size = this.messageGroupStore.messageGroupSize(this.original.getGroupId()); } } + finally { + this.lock.unlock(); + } } return this.size; } @@ -195,6 +208,8 @@ public void clear() { private final class PersistentCollection extends AbstractCollection> { + private final Lock innerLock = new ReentrantLock(); + private volatile Collection> collection; PersistentCollection() { @@ -202,7 +217,8 @@ private final class PersistentCollection extends AbstractCollection> private void load() { if (this.collection == null) { - synchronized (this) { + this.innerLock.lock(); + try { if (this.collection == null) { Object groupId = PersistentMessageGroup.this.original.getGroupId(); if (LOGGER.isDebugEnabled()) { @@ -211,6 +227,9 @@ private void load() { this.collection = PersistentMessageGroup.this.messageGroupStore.getMessagesForGroup(groupId); } } + finally { + this.innerLock.unlock(); + } } } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/store/SimpleMessageGroup.java b/spring-integration-core/src/main/java/org/springframework/integration/store/SimpleMessageGroup.java index dc48b6e7252..2b878ef991e 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/store/SimpleMessageGroup.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/store/SimpleMessageGroup.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,6 +22,8 @@ import java.util.Iterator; import java.util.LinkedHashSet; import java.util.Set; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.springframework.integration.IntegrationMessageHeaderAccessor; import org.springframework.lang.Nullable; @@ -38,11 +40,14 @@ * @author Dave Syer * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov * * @since 2.0 */ public class SimpleMessageGroup implements MessageGroup { + private final Lock lock = new ReentrantLock(); + private final Object groupId; private final Collection> messages; @@ -189,10 +194,14 @@ public String getCondition() { @Override public Message getOne() { - synchronized (this.messages) { + this.lock.lock(); + try { Iterator> iterator = this.messages.iterator(); return iterator.hasNext() ? iterator.next() : null; } + finally { + this.lock.unlock(); + } } @Override diff --git a/spring-integration-core/src/main/java/org/springframework/integration/support/SmartLifecycleRoleController.java b/spring-integration-core/src/main/java/org/springframework/integration/support/SmartLifecycleRoleController.java index e122610a42c..5e4c72d6bae 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/support/SmartLifecycleRoleController.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/support/SmartLifecycleRoleController.java @@ -1,5 +1,5 @@ /* - * Copyright 2015-2021 the original author or authors. + * Copyright 2015-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -24,6 +24,8 @@ import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.function.Predicate; import java.util.stream.Collectors; @@ -52,6 +54,7 @@ * * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov * * @since 4.2 * @@ -63,6 +66,8 @@ public class SmartLifecycleRoleController implements ApplicationListener lifecycles = new LinkedMultiValueMap<>(); private final MultiValueMap lazyLifecycles = new LinkedMultiValueMap<>(); @@ -283,9 +288,15 @@ public Map getEndpointsRunningStatus(String role) { Lifecycle::isRunning)); } - private synchronized void addLazyLifecycles() { - this.lazyLifecycles.forEach(this::doAddLifecyclesToRole); - this.lazyLifecycles.clear(); + private void addLazyLifecycles() { + this.lock.lock(); + try { + this.lazyLifecycles.forEach(this::doAddLifecyclesToRole); + this.lazyLifecycles.clear(); + } + finally { + this.lock.unlock(); + } } private void doAddLifecyclesToRole(String role, List lifecycleBeanNames) { diff --git a/spring-integration-core/src/main/java/org/springframework/integration/support/channel/BeanFactoryChannelResolver.java b/spring-integration-core/src/main/java/org/springframework/integration/support/channel/BeanFactoryChannelResolver.java index 1817685c692..30428f34e37 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/support/channel/BeanFactoryChannelResolver.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/support/channel/BeanFactoryChannelResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,9 @@ package org.springframework.integration.support.channel; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; + import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -39,6 +42,7 @@ * @author Mark Fisher * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov * * @see BeanFactory */ @@ -46,6 +50,8 @@ public class BeanFactoryChannelResolver implements DestinationResolver "Stopped LeaderInitiator for " + getContext()); } - this.future = null; - LOGGER.debug(() -> "Stopped LeaderInitiator for " + getContext()); + } + finally { + this.lock.unlock(); } } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/transformer/support/RoutingSlipHeaderValueMessageProcessor.java b/spring-integration-core/src/main/java/org/springframework/integration/transformer/support/RoutingSlipHeaderValueMessageProcessor.java index b5277da2173..e554157f864 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/transformer/support/RoutingSlipHeaderValueMessageProcessor.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/transformer/support/RoutingSlipHeaderValueMessageProcessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2019 the original author or authors. + * Copyright 2014-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,6 +21,8 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.springframework.beans.BeansException; import org.springframework.beans.factory.BeanFactory; @@ -41,12 +43,16 @@ * * @author Artem Bilan * @author Gary Russell + * @author Christian Tzolov + * * @since 4.1 */ public class RoutingSlipHeaderValueMessageProcessor extends AbstractHeaderValueMessageProcessor, Integer>> implements BeanFactoryAware { + private final Lock lock = new ReentrantLock(); + private final List routingSlipPath; private volatile Map, Integer> routingSlip; @@ -79,7 +85,8 @@ public Map, Integer> processMessage(Message message) { // use a local variable to avoid the second access to volatile field on the happy path Map, Integer> slip = this.routingSlip; if (slip == null) { - synchronized (this) { + this.lock.lock(); + try { slip = this.routingSlip; if (slip == null) { List slipPath = this.routingSlipPath; @@ -118,6 +125,9 @@ public Map, Integer> processMessage(Message message) { this.routingSlip = slip; } } + finally { + this.lock.unlock(); + } } return slip; } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/util/AcceptOnceCollectionFilter.java b/spring-integration-core/src/main/java/org/springframework/integration/util/AcceptOnceCollectionFilter.java index 3506ddf74a8..26e49a9cff0 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/util/AcceptOnceCollectionFilter.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/util/AcceptOnceCollectionFilter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,6 +20,8 @@ import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; /** * An implementation of {@link CollectionFilter} that remembers the elements passed in @@ -28,6 +30,7 @@ * @param the collection element type. * * @author Mark Fisher + * @author Christian Tzolov * * @since 2.1 */ @@ -35,15 +38,23 @@ public class AcceptOnceCollectionFilter implements CollectionFilter { private volatile Collection lastSeenElements = Collections.emptyList(); - public synchronized Collection filter(Collection unfilteredElements) { - List filteredElements = new ArrayList<>(); - for (T element : unfilteredElements) { - if (!this.lastSeenElements.contains(element)) { - filteredElements.add(element); + private final Lock lock = new ReentrantLock(); + + public Collection filter(Collection unfilteredElements) { + this.lock.lock(); + try { + List filteredElements = new ArrayList<>(); + for (T element : unfilteredElements) { + if (!this.lastSeenElements.contains(element)) { + filteredElements.add(element); + } } + this.lastSeenElements = unfilteredElements; + return filteredElements; + } + finally { + this.lock.unlock(); } - this.lastSeenElements = unfilteredElements; - return filteredElements; } } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/util/SimplePool.java b/spring-integration-core/src/main/java/org/springframework/integration/util/SimplePool.java index 1154d54e14a..1658ff8c636 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/util/SimplePool.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/util/SimplePool.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -24,6 +24,8 @@ import java.util.concurrent.Semaphore; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -40,6 +42,7 @@ * @author Gary Russell * @author Sergey Bogatyrev * @author Artem Bilan + * @author Christian Tzolov * * @since 2.2 * @@ -48,6 +51,8 @@ public class SimplePool implements Pool { protected final Log logger = LogFactory.getLog(getClass()); // NOSONAR final + private final Lock lock = new ReentrantLock(); + private final PoolSemaphore permits = new PoolSemaphore(0); private final AtomicInteger poolSize = new AtomicInteger(); @@ -93,40 +98,46 @@ public SimplePool(int poolSize, PoolItemCallback callback) { * items are returned. * @param poolSize The desired target pool size. */ - public synchronized void setPoolSize(int poolSize) { - int delta = poolSize - this.poolSize.get(); - this.targetPoolSize.addAndGet(delta); - if (this.logger.isDebugEnabled()) { - this.logger.debug(String.format("Target pool size changed by %d, now %d", delta, - this.targetPoolSize.get())); - } - if (delta > 0) { - this.poolSize.addAndGet(delta); - this.permits.release(delta); - } - else { - this.permits.reducePermits(-delta); - - int inUseSize = this.inUse.size(); - int newPoolSize = Math.max(poolSize, inUseSize); - this.poolSize.set(newPoolSize); - - for (int i = this.available.size(); i > newPoolSize - inUseSize; i--) { - T item = this.available.poll(); - if (item != null) { - doRemoveItem(item); - } - else { - break; - } + public void setPoolSize(int poolSize) { + this.lock.lock(); + try { + int delta = poolSize - this.poolSize.get(); + this.targetPoolSize.addAndGet(delta); + if (this.logger.isDebugEnabled()) { + this.logger.debug(String.format("Target pool size changed by %d, now %d", delta, + this.targetPoolSize.get())); } + if (delta > 0) { + this.poolSize.addAndGet(delta); + this.permits.release(delta); + } + else { + this.permits.reducePermits(-delta); + + int inUseSize = this.inUse.size(); + int newPoolSize = Math.max(poolSize, inUseSize); + this.poolSize.set(newPoolSize); + + for (int i = this.available.size(); i > newPoolSize - inUseSize; i--) { + T item = this.available.poll(); + if (item != null) { + doRemoveItem(item); + } + else { + break; + } + } - int inUseDelta = poolSize - inUseSize; - if (inUseDelta < 0 && this.logger.isDebugEnabled()) { - this.logger.debug(String.format("Pool is overcommitted by %d; items will be removed when returned", - -inUseDelta)); + int inUseDelta = poolSize - inUseSize; + if (inUseDelta < 0 && this.logger.isDebugEnabled()) { + this.logger.debug(String.format("Pool is overcommitted by %d; items will be removed when returned", + -inUseDelta)); + } } } + finally { + this.lock.unlock(); + } } /** @@ -135,8 +146,14 @@ public synchronized void setPoolSize(int poolSize) { * to be set. */ @Override - public synchronized int getPoolSize() { - return this.poolSize.get(); + public int getPoolSize() { + this.lock.lock(); + try { + return this.poolSize.get(); + } + finally { + this.lock.unlock(); + } } @Override @@ -224,36 +241,48 @@ else if (this.callback.isStale(item)) { * Return an item to the pool. */ @Override - public synchronized void releaseItem(T item) { - Assert.notNull(item, "Item cannot be null"); - Assert.isTrue(this.allocated.contains(item), - "You can only release items that were obtained from the pool"); - if (this.inUse.contains(item)) { - if (this.poolSize.get() > this.targetPoolSize.get() || this.closed) { - this.poolSize.decrementAndGet(); - doRemoveItem(item); + public void releaseItem(T item) { + this.lock.lock(); + try { + Assert.notNull(item, "Item cannot be null"); + Assert.isTrue(this.allocated.contains(item), + "You can only release items that were obtained from the pool"); + if (this.inUse.contains(item)) { + if (this.poolSize.get() > this.targetPoolSize.get() || this.closed) { + this.poolSize.decrementAndGet(); + doRemoveItem(item); + } + else { + if (this.logger.isDebugEnabled()) { + this.logger.debug("Releasing " + item + " back to the pool"); + } + this.available.add(item); + this.inUse.remove(item); + this.permits.release(); + } } else { if (this.logger.isDebugEnabled()) { - this.logger.debug("Releasing " + item + " back to the pool"); + this.logger.debug("Ignoring release of " + item + " back to the pool - not in use"); } - this.available.add(item); - this.inUse.remove(item); - this.permits.release(); } } - else { - if (this.logger.isDebugEnabled()) { - this.logger.debug("Ignoring release of " + item + " back to the pool - not in use"); - } + finally { + this.lock.unlock(); } } @Override - public synchronized void removeAllIdleItems() { - T item; - while ((item = this.available.poll()) != null) { - doRemoveItem(item); + public void removeAllIdleItems() { + this.lock.lock(); + try { + T item; + while ((item = this.available.poll()) != null) { + doRemoveItem(item); + } + } + finally { + this.lock.unlock(); } } @@ -267,9 +296,15 @@ private void doRemoveItem(T item) { } @Override - public synchronized void close() { - this.closed = true; - removeAllIdleItems(); + public void close() { + this.lock.lock(); + try { + this.closed = true; + removeAllIdleItems(); + } + finally { + this.lock.unlock(); + } } @SuppressWarnings("serial") diff --git a/spring-integration-feed/src/main/java/org/springframework/integration/feed/inbound/FeedEntryMessageSource.java b/spring-integration-feed/src/main/java/org/springframework/integration/feed/inbound/FeedEntryMessageSource.java index bb3c0978d9e..229451fc4ec 100644 --- a/spring-integration-feed/src/main/java/org/springframework/integration/feed/inbound/FeedEntryMessageSource.java +++ b/spring-integration-feed/src/main/java/org/springframework/integration/feed/inbound/FeedEntryMessageSource.java @@ -28,6 +28,8 @@ import java.util.List; import java.util.Queue; import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import com.rometools.rome.feed.synd.SyndEntry; import com.rometools.rome.feed.synd.SyndFeed; @@ -56,6 +58,7 @@ * @author Oleg Zhurakousky * @author Artem Bilan * @author Aaron Loes + * @author Christian Tzolov * * @since 2.0 */ @@ -69,13 +72,13 @@ public class FeedEntryMessageSource extends AbstractMessageSource { private final Queue entries = new ConcurrentLinkedQueue<>(); - private final Object monitor = new Object(); + private final Lock monitor = new ReentrantLock(); private final Comparator syndEntryComparator = Comparator.comparing(FeedEntryMessageSource::getLastModifiedDate, Comparator.nullsFirst(Comparator.naturalOrder())); - private final Object feedMonitor = new Object(); + private final Lock feedMonitor = new ReentrantLock(); private SyndFeedInput syndFeedInput = new SyndFeedInput(); @@ -176,7 +179,8 @@ protected SyndEntry doReceive() { Assert.isTrue(this.initialized, "'FeedEntryReaderMessageSource' must be initialized before it can produce Messages."); SyndEntry nextEntry; - synchronized (this.monitor) { + this.monitor.lock(); + try { nextEntry = getNextEntry(); if (nextEntry == null) { // read feed and try again @@ -184,6 +188,9 @@ protected SyndEntry doReceive() { nextEntry = getNextEntry(); } } + finally { + this.monitor.unlock(); + } return nextEntry; } @@ -225,7 +232,8 @@ private void populateEntryList() { private SyndFeed getFeed() { try { - synchronized (this.feedMonitor) { + this.feedMonitor.lock(); + try { SyndFeed feed = buildSyndFeed(); logger.debug(() -> "Retrieved feed for [" + this + "]"); if (feed == null) { @@ -233,6 +241,9 @@ private SyndFeed getFeed() { } return feed; } + finally { + this.feedMonitor.unlock(); + } } catch (Exception e) { throw new MessagingException("Failed to retrieve feed for '" + this + "'", e); diff --git a/spring-integration-file/src/main/java/org/springframework/integration/file/FileWritingMessageHandler.java b/spring-integration-file/src/main/java/org/springframework/integration/file/FileWritingMessageHandler.java index a8b7bad8e40..66225d26c26 100644 --- a/spring-integration-file/src/main/java/org/springframework/integration/file/FileWritingMessageHandler.java +++ b/spring-integration-file/src/main/java/org/springframework/integration/file/FileWritingMessageHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -40,6 +40,7 @@ import java.util.Set; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.function.BiConsumer; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -109,6 +110,7 @@ * @author Tony Falabella * @author Alen Turkovic * @author Trung Pham + * @author Christian Tzolov */ public class FileWritingMessageHandler extends AbstractReplyProducingMessageHandler implements ManageableLifecycle, MessageTriggerAction { @@ -130,6 +132,8 @@ public class FileWritingMessageHandler extends AbstractReplyProducingMessageHand PosixFilePermission.OWNER_READ }; + private final Lock lock = new ReentrantLock(); + private final Map fileStates = new HashMap<>(); private final Expression destinationDirectoryExpression; @@ -459,12 +463,16 @@ public void start() { @Override public void stop() { - synchronized (this) { + this.lock.lock(); + try { if (this.flushTask != null) { this.flushTask.cancel(true); this.flushTask = null; } } + finally { + this.lock.unlock(); + } Flusher flusher = new Flusher(); flusher.run(); boolean needInterrupt = this.fileStates.size() > 0; @@ -873,37 +881,42 @@ private File evaluateDestinationDirectoryExpression(Message message) { return destinationDirectory; } - private synchronized FileState getFileState(File fileToWriteTo, boolean isString) + private FileState getFileState(File fileToWriteTo, boolean isString) throws FileNotFoundException { - - FileState state; - boolean appendNoFlush = FileExistsMode.APPEND_NO_FLUSH.equals(this.fileExistsMode); - if (appendNoFlush) { - String absolutePath = fileToWriteTo.getAbsolutePath(); - state = this.fileStates.get(absolutePath); - if (state != null // NOSONAR - && ((isString && state.stream != null) || (!isString && state.writer != null))) { - state.close(); - state = null; - this.fileStates.remove(absolutePath); - } - if (state == null) { - if (isString) { - state = new FileState(createWriter(fileToWriteTo, true), - this.lockRegistry.obtain(fileToWriteTo.getAbsolutePath())); + this.lock.lock(); + try { + FileState state; + boolean appendNoFlush = FileExistsMode.APPEND_NO_FLUSH.equals(this.fileExistsMode); + if (appendNoFlush) { + String absolutePath = fileToWriteTo.getAbsolutePath(); + state = this.fileStates.get(absolutePath); + if (state != null // NOSONAR + && ((isString && state.stream != null) || (!isString && state.writer != null))) { + state.close(); + state = null; + this.fileStates.remove(absolutePath); } - else { - state = new FileState(createOutputStream(fileToWriteTo, true), - this.lockRegistry.obtain(fileToWriteTo.getAbsolutePath())); + if (state == null) { + if (isString) { + state = new FileState(createWriter(fileToWriteTo, true), + this.lockRegistry.obtain(fileToWriteTo.getAbsolutePath())); + } + else { + state = new FileState(createOutputStream(fileToWriteTo, true), + this.lockRegistry.obtain(fileToWriteTo.getAbsolutePath())); + } + this.fileStates.put(absolutePath, state); } - this.fileStates.put(absolutePath, state); + state.lastWrite = Long.MAX_VALUE; // prevent flush while we write } - state.lastWrite = Long.MAX_VALUE; // prevent flush while we write + else { + state = null; + } + return state; } - else { - state = null; + finally { + this.lock.unlock(); } - return state; } /** @@ -975,7 +988,8 @@ public void flushIfNeeded(MessageFlushPredicate flushPredicate, Message filte private Map findFilesToFlush(MessageFlushPredicate flushPredicate, Message filterMessage) { Map toRemove = new HashMap<>(); - synchronized (this) { + this.lock.lock(); + try { Iterator> iterator = this.fileStates.entrySet().iterator(); while (iterator.hasNext()) { Entry entry = iterator.next(); @@ -986,12 +1000,21 @@ private Map findFilesToFlush(MessageFlushPredicate flushPredi } } } + finally { + this.lock.unlock(); + } return toRemove; } - private synchronized void clearState(final File fileToWriteTo, final FileState state) { + private void clearState(final File fileToWriteTo, final FileState state) { if (state != null) { - this.fileStates.remove(fileToWriteTo.getAbsolutePath()); + this.lock.lock(); + try { + this.fileStates.remove(fileToWriteTo.getAbsolutePath()); + } + finally { + this.lock.unlock(); + } } } @@ -1014,11 +1037,15 @@ private void doFlush(Map toRemove) { FileWritingMessageHandler.this.logger .debug("Interrupted during flush; not flushed: " + toRestore.keySet()); } - synchronized (this) { + this.lock.lock(); + try { for (Entry entry : toRestore.entrySet()) { this.fileStates.putIfAbsent(entry.getKey(), entry.getValue()); } } + finally { + this.lock.unlock(); + } } } @@ -1085,11 +1112,12 @@ private final class Flusher implements Runnable { @Override public void run() { Map toRemove = new HashMap<>(); - synchronized (FileWritingMessageHandler.this) { + FileWritingMessageHandler.this.lock.lock(); + try { long expired = FileWritingMessageHandler.this.flushTask == null ? Long.MAX_VALUE : (System.currentTimeMillis() - FileWritingMessageHandler.this.flushInterval); - Iterator> iterator = - FileWritingMessageHandler.this.fileStates.entrySet().iterator(); + Iterator> iterator = FileWritingMessageHandler.this.fileStates.entrySet() + .iterator(); while (iterator.hasNext()) { Entry entry = iterator.next(); FileState state = entry.getValue(); @@ -1100,6 +1128,9 @@ public void run() { } } } + finally { + FileWritingMessageHandler.this.lock.unlock(); + } doFlush(toRemove); } diff --git a/spring-integration-file/src/main/java/org/springframework/integration/file/config/FileListFilterFactoryBean.java b/spring-integration-file/src/main/java/org/springframework/integration/file/config/FileListFilterFactoryBean.java index a35c19eb0b8..8216721a7eb 100644 --- a/spring-integration-file/src/main/java/org/springframework/integration/file/config/FileListFilterFactoryBean.java +++ b/spring-integration-file/src/main/java/org/springframework/integration/file/config/FileListFilterFactoryBean.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,6 +19,8 @@ import java.io.File; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.springframework.beans.factory.FactoryBean; import org.springframework.integration.file.filters.AcceptAllFileListFilter; @@ -34,6 +36,8 @@ * @author Mark Fisher * @author Gunnar Hillert * @author Gary Russell + * @author Christian Tzolov + * * @since 1.0.3 */ public class FileListFilterFactoryBean implements FactoryBean> { @@ -52,7 +56,7 @@ public class FileListFilterFactoryBean implements FactoryBean filter) { this.filter = filter; @@ -95,9 +99,13 @@ public void setAlwaysAcceptDirectories(Boolean alwaysAcceptDirectories) { @NonNull public FileListFilter getObject() { if (this.result == null) { - synchronized (this.monitor) { + this.monitor.lock(); + try { this.initializeFileListFilter(); } + finally { + this.monitor.unlock(); + } } return this.result; } diff --git a/spring-integration-file/src/main/java/org/springframework/integration/file/filters/AcceptOnceFileListFilter.java b/spring-integration-file/src/main/java/org/springframework/integration/file/filters/AcceptOnceFileListFilter.java index f1c4d565167..a839ed848de 100644 --- a/spring-integration-file/src/main/java/org/springframework/integration/file/filters/AcceptOnceFileListFilter.java +++ b/spring-integration-file/src/main/java/org/springframework/integration/file/filters/AcceptOnceFileListFilter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,6 +21,8 @@ import java.util.Queue; import java.util.Set; import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.springframework.lang.Nullable; @@ -37,6 +39,7 @@ * @author Josh Long * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov */ public class AcceptOnceFileListFilter extends AbstractFileListFilter implements ReversibleFileListFilter, ResettableFileListFilter { @@ -46,7 +49,7 @@ public class AcceptOnceFileListFilter extends AbstractFileListFilter imple private final Set seenSet = new HashSet(); - private final Object monitor = new Object(); + private final Lock monitor = new ReentrantLock(); /** @@ -69,7 +72,8 @@ public AcceptOnceFileListFilter() { @Override public boolean accept(F file) { - synchronized (this.monitor) { + this.monitor.lock(); + try { if (this.seenSet.contains(file)) { return false; } @@ -81,11 +85,15 @@ public boolean accept(F file) { this.seenSet.add(file); return true; } + finally { + this.monitor.unlock(); + } } @Override public void rollback(F file, List files) { - synchronized (this.monitor) { + this.monitor.lock(); + try { boolean rollingBack = false; for (F fileToRollback : files) { if (fileToRollback.equals(file)) { @@ -96,6 +104,9 @@ public void rollback(F file, List files) { } } } + finally { + this.monitor.unlock(); + } } @Override diff --git a/spring-integration-file/src/main/java/org/springframework/integration/file/filters/CompositeFileListFilter.java b/spring-integration-file/src/main/java/org/springframework/integration/file/filters/CompositeFileListFilter.java index 3eacbcc72a8..5b10ddbf26f 100644 --- a/spring-integration-file/src/main/java/org/springframework/integration/file/filters/CompositeFileListFilter.java +++ b/spring-integration-file/src/main/java/org/springframework/integration/file/filters/CompositeFileListFilter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -26,6 +26,8 @@ import java.util.List; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.function.Consumer; import org.springframework.beans.factory.InitializingBean; @@ -46,6 +48,7 @@ * @author Josh Long * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov */ public class CompositeFileListFilter implements ReversibleFileListFilter, ResettableFileListFilter, DiscardAwareFileListFilter, Closeable { @@ -58,6 +61,7 @@ public class CompositeFileListFilter private boolean oneIsForRecursion; + private final Lock lock = new ReentrantLock(); public CompositeFileListFilter() { this.fileFilters = new LinkedHashSet<>(); @@ -104,24 +108,30 @@ public final CompositeFileListFilter addFilters(FileListFilter... filters) * @param filtersToAdd a list of filters to add * @return this CompositeFileListFilter instance with the added filters */ - public synchronized CompositeFileListFilter addFilters(Collection> filtersToAdd) { - for (FileListFilter elf : filtersToAdd) { - if (elf instanceof DiscardAwareFileListFilter) { - ((DiscardAwareFileListFilter) elf).addDiscardCallback(this.discardCallback); - } - if (elf instanceof InitializingBean) { - try { - ((InitializingBean) elf).afterPropertiesSet(); + public CompositeFileListFilter addFilters(Collection> filtersToAdd) { + this.lock.lock(); + try { + for (FileListFilter elf : filtersToAdd) { + if (elf instanceof DiscardAwareFileListFilter) { + ((DiscardAwareFileListFilter) elf).addDiscardCallback(this.discardCallback); } - catch (Exception e) { - throw new IllegalStateException(e); + if (elf instanceof InitializingBean) { + try { + ((InitializingBean) elf).afterPropertiesSet(); + } + catch (Exception e) { + throw new IllegalStateException(e); + } } + this.allSupportAccept = this.allSupportAccept && elf.supportsSingleFileFiltering(); + this.oneIsForRecursion |= elf.isForRecursion(); } - this.allSupportAccept = this.allSupportAccept && elf.supportsSingleFileFiltering(); - this.oneIsForRecursion |= elf.isForRecursion(); + this.fileFilters.addAll(filtersToAdd); + return this; + } + finally { + this.lock.unlock(); } - this.fileFilters.addAll(filtersToAdd); - return this; } @Override diff --git a/spring-integration-file/src/main/java/org/springframework/integration/file/remote/session/CachingSessionFactory.java b/spring-integration-file/src/main/java/org/springframework/integration/file/remote/session/CachingSessionFactory.java index 2ab2f38416c..be98c5f96ea 100644 --- a/spring-integration-file/src/main/java/org/springframework/integration/file/remote/session/CachingSessionFactory.java +++ b/spring-integration-file/src/main/java/org/springframework/integration/file/remote/session/CachingSessionFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,6 +19,8 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -40,6 +42,7 @@ * @author Gary Russell * @author Alen Turkovic * @author Artem Bilan + * @author Christian Tzolov * * @since 2.0 */ @@ -47,6 +50,8 @@ public class CachingSessionFactory implements SessionFactory, DisposableBe private static final Log LOGGER = LogFactory.getLog(CachingSessionFactory.class); + private final Lock lock = new ReentrantLock(); + private final SessionFactory sessionFactory; private final SimplePool> pool; @@ -146,24 +151,29 @@ public void destroy() { * Clear the cache of sessions; also any in-use sessions will be closed when * returned to the cache. */ - public synchronized void resetCache() { - LOGGER.debug("Cache reset; idle sessions will be removed, in-use sessions will be closed when returned"); - if (this.isSharedSessionCapable && ((SharedSessionCapable) this.sessionFactory).isSharedSession()) { - ((SharedSessionCapable) this.sessionFactory).resetSharedSession(); + public void resetCache() { + this.lock.lock(); + try { + LOGGER.debug("Cache reset; idle sessions will be removed, in-use sessions will be closed when returned"); + if (this.isSharedSessionCapable && ((SharedSessionCapable) this.sessionFactory).isSharedSession()) { + ((SharedSessionCapable) this.sessionFactory).resetSharedSession(); + } + long epoch = System.nanoTime(); + /* + * Spin until we get a new value - nano precision but may be lower resolution. We reset the epoch AFTER + * resetting the shared session so there is no possibility of an "old" session being created in the new + * epoch. There is a slight possibility that a "new" session might appear in the old epoch and thus be + * closed when returned to the cache. + */ + while (epoch == this.sharedSessionEpoch) { + epoch = System.nanoTime(); + } + this.sharedSessionEpoch = epoch; + this.pool.removeAllIdleItems(); } - long epoch = System.nanoTime(); - /* - * Spin until we get a new value - nano precision but may be lower resolution. - * We reset the epoch AFTER resetting the shared session so there is no possibility - * of an "old" session being created in the new epoch. There is a slight possibility - * that a "new" session might appear in the old epoch and thus be closed when returned to - * the cache. - */ - while (epoch == this.sharedSessionEpoch) { - epoch = System.nanoTime(); + finally { + this.lock.unlock(); } - this.sharedSessionEpoch = epoch; - this.pool.removeAllIdleItems(); } public class CachedSession implements Session { //NOSONAR must be final, but can't for mocking in tests @@ -174,6 +184,8 @@ public class CachedSession implements Session { //NOSONAR must be final, but private boolean dirty; + private final Lock lock = new ReentrantLock(); + /** * The epoch in which this session was created. */ @@ -185,35 +197,42 @@ private CachedSession(Session targetSession, long sharedSessionEpoch) { } @Override - public synchronized void close() { - if (this.released) { - if (LOGGER.isDebugEnabled()) { - LOGGER.debug("Session " + this.targetSession + " already released."); - } - } - else { - if (LOGGER.isDebugEnabled()) { - LOGGER.debug("Releasing Session " + this.targetSession + " back to the pool."); - } - if (this.sharedSessionEpoch != CachingSessionFactory.this.sharedSessionEpoch) { + public void close() { + this.lock.lock(); + try { + + if (this.released) { if (LOGGER.isDebugEnabled()) { - LOGGER.debug("Closing session " + this.targetSession + " after reset."); + LOGGER.debug("Session " + this.targetSession + " already released."); } - this.targetSession.close(); } - else if (this.dirty) { - this.targetSession.close(); - } - if (this.targetSession.isOpen()) { - try { - this.targetSession.finalizeRaw(); + else { + if (LOGGER.isDebugEnabled()) { + LOGGER.debug("Releasing Session " + this.targetSession + " back to the pool."); } - catch (IOException e) { - //No-op in this context + if (this.sharedSessionEpoch != CachingSessionFactory.this.sharedSessionEpoch) { + if (LOGGER.isDebugEnabled()) { + LOGGER.debug("Closing session " + this.targetSession + " after reset."); + } + this.targetSession.close(); } + else if (this.dirty) { + this.targetSession.close(); + } + if (this.targetSession.isOpen()) { + try { + this.targetSession.finalizeRaw(); + } + catch (IOException e) { + // No-op in this context + } + } + CachingSessionFactory.this.pool.releaseItem(this.targetSession); + this.released = true; } - CachingSessionFactory.this.pool.releaseItem(this.targetSession); - this.released = true; + } + finally { + this.lock.unlock(); } } diff --git a/spring-integration-hazelcast/src/main/java/org/springframework/integration/hazelcast/leader/LeaderInitiator.java b/spring-integration-hazelcast/src/main/java/org/springframework/integration/hazelcast/leader/LeaderInitiator.java index 2a843119cf3..1db7a909ea5 100644 --- a/spring-integration-hazelcast/src/main/java/org/springframework/integration/hazelcast/leader/LeaderInitiator.java +++ b/spring-integration-hazelcast/src/main/java/org/springframework/integration/hazelcast/leader/LeaderInitiator.java @@ -21,6 +21,8 @@ import java.util.concurrent.Future; import java.util.concurrent.Semaphore; import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import com.hazelcast.core.HazelcastInstance; import com.hazelcast.cp.CPSubsystem; @@ -54,6 +56,7 @@ * @author Mael Le Guével * @author Alexey Tsoy * @author Robert Höglund + * @author Christian Tzolov */ public class LeaderInitiator implements SmartLifecycle, DisposableBean, ApplicationEventPublisherAware { @@ -61,6 +64,8 @@ public class LeaderInitiator implements SmartLifecycle, DisposableBean, Applicat private static final Context NULL_CONTEXT = new NullContext(); + private final Lock lock = new ReentrantLock(); + /*** Hazelcast client. */ private final HazelcastInstance client; @@ -208,11 +213,17 @@ public Context getContext() { * Start the registration of the {@link #candidate} for leader election. */ @Override - public synchronized void start() { - if (!this.running) { - this.leaderSelector = new LeaderSelector(); - this.running = true; - this.future = this.taskExecutor.submit(this.leaderSelector); + public void start() { + this.lock.lock(); + try { + if (!this.running) { + this.leaderSelector = new LeaderSelector(); + this.running = true; + this.future = this.taskExecutor.submit(this.leaderSelector); + } + } + finally { + this.lock.unlock(); } } @@ -227,13 +238,19 @@ public void stop(Runnable callback) { * If the candidate is currently leader, its leadership will be revoked. */ @Override - public synchronized void stop() { - if (this.running) { - this.running = false; - if (this.future != null) { - this.future.cancel(true); + public void stop() { + this.lock.lock(); + try { + if (this.running) { + this.running = false; + if (this.future != null) { + this.future.cancel(true); + } + this.future = null; } - this.future = null; + } + finally { + this.lock.unlock(); } } diff --git a/spring-integration-http/src/main/java/org/springframework/integration/http/outbound/AbstractHttpRequestExecutingMessageHandler.java b/spring-integration-http/src/main/java/org/springframework/integration/http/outbound/AbstractHttpRequestExecutingMessageHandler.java index b36ad1e93ba..e279004c21a 100644 --- a/spring-integration-http/src/main/java/org/springframework/integration/http/outbound/AbstractHttpRequestExecutingMessageHandler.java +++ b/spring-integration-http/src/main/java/org/springframework/integration/http/outbound/AbstractHttpRequestExecutingMessageHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2017-2022 the original author or authors. + * Copyright 2017-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -26,6 +26,8 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import javax.xml.transform.Source; @@ -71,6 +73,7 @@ * @author Wallace Wadge * @author Shiliang Li * @author Florian Schöffl + * @author Christian Tzolov * * @since 5.0 */ @@ -81,6 +84,8 @@ public abstract class AbstractHttpRequestExecutingMessageHandler extends Abstrac protected final DefaultUriBuilderFactory uriFactory = new DefaultUriBuilderFactory(); // NOSONAR - final + private final Lock lock = new ReentrantLock(); + private final Map uriVariableExpressions = new HashMap<>(); private final Expression uriExpression; @@ -226,10 +231,14 @@ public void setHeaderMapper(HeaderMapper headerMapper) { * @param uriVariableExpressions The URI variable expressions. */ public void setUriVariableExpressions(Map uriVariableExpressions) { - synchronized (this.uriVariableExpressions) { + this.lock.lock(); + try { this.uriVariableExpressions.clear(); this.uriVariableExpressions.putAll(uriVariableExpressions); } + finally { + this.lock.unlock(); + } } /** diff --git a/spring-integration-ip/src/main/java/org/springframework/integration/ip/AbstractInternetProtocolSendingMessageHandler.java b/spring-integration-ip/src/main/java/org/springframework/integration/ip/AbstractInternetProtocolSendingMessageHandler.java index ef5ff630dd1..1f9049be2c8 100644 --- a/spring-integration-ip/src/main/java/org/springframework/integration/ip/AbstractInternetProtocolSendingMessageHandler.java +++ b/spring-integration-ip/src/main/java/org/springframework/integration/ip/AbstractInternetProtocolSendingMessageHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,6 +18,8 @@ import java.net.InetSocketAddress; import java.net.SocketAddress; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.springframework.integration.handler.AbstractMessageHandler; import org.springframework.integration.support.management.ManageableLifecycle; @@ -27,11 +29,15 @@ * Base class for UDP MessageHandlers. * * @author Gary Russell + * @author Christian Tzolov + * * @since 2.0 */ public abstract class AbstractInternetProtocolSendingMessageHandler extends AbstractMessageHandler implements CommonSocketOptions, ManageableLifecycle { + private final Lock lock = new ReentrantLock(); + private final SocketAddress destinationAddress; private final String host; @@ -119,20 +125,32 @@ public int getSoSendBufferSize() { @Override - public synchronized void start() { - if (!this.running) { - this.doStart(); - this.running = true; + public void start() { + this.lock.lock(); + try { + if (!this.running) { + this.doStart(); + this.running = true; + } + } + finally { + this.lock.unlock(); } } protected abstract void doStart(); @Override - public synchronized void stop() { - if (this.running) { - this.doStop(); - this.running = false; + public void stop() { + this.lock.lock(); + try { + if (this.running) { + this.doStop(); + this.running = false; + } + } + finally { + this.lock.unlock(); } } diff --git a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/TcpSendingMessageHandler.java b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/TcpSendingMessageHandler.java index 15b3d195218..fe33345b7f3 100644 --- a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/TcpSendingMessageHandler.java +++ b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/TcpSendingMessageHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,6 +21,8 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.springframework.context.ApplicationEventPublisher; import org.springframework.integration.handler.AbstractMessageHandler; @@ -48,6 +50,7 @@ * * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov * * @since 2.0 * @@ -60,7 +63,7 @@ public class TcpSendingMessageHandler extends AbstractMessageHandler implements */ public static final long DEFAULT_RETRY_INTERVAL = 60000; - protected final Object lifecycleMonitor = new Object(); // NOSONAR + protected final Lock lifecycleMonitor = new ReentrantLock(); // NOSONAR private final Map connections = new ConcurrentHashMap<>(); @@ -251,7 +254,8 @@ protected void onInit() { @Override public void start() { - synchronized (this.lifecycleMonitor) { + this.lifecycleMonitor.lock(); + try { if (!this.active) { this.active = true; if (this.clientConnectionFactory != null) { @@ -273,11 +277,15 @@ public void start() { } } } + finally { + this.lifecycleMonitor.unlock(); + } } @Override public void stop() { - synchronized (this.lifecycleMonitor) { + this.lifecycleMonitor.lock(); + try { if (this.active) { this.active = false; if (this.scheduledFuture != null) { @@ -292,6 +300,9 @@ public void stop() { } } } + finally { + this.lifecycleMonitor.unlock(); + } } @Override diff --git a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/AbstractConnectionFactory.java b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/AbstractConnectionFactory.java index d7f365e6c7b..55c6da1f631 100644 --- a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/AbstractConnectionFactory.java +++ b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/AbstractConnectionFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -42,6 +42,8 @@ import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.ApplicationEventPublisherAware; @@ -58,6 +60,7 @@ * * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov * * @since 2.0 * @@ -73,10 +76,12 @@ public abstract class AbstractConnectionFactory extends IntegrationObjectSupport private static final int DEFAULT_READ_DELAY = 100; - protected final Object lifecycleMonitor = new Object(); // NOSONAR final + protected final Lock lifecycleMonitor = new ReentrantLock(); // NOSONAR final private final Map connections = new ConcurrentHashMap<>(); + private final Lock connectionsMonitor = new ReentrantLock(); + private final BlockingQueue delayedReads = new LinkedBlockingQueue<>(); private final List senders = Collections.synchronizedList(new ArrayList<>()); @@ -546,13 +551,17 @@ protected Executor getTaskExecutor() { if (!this.active) { throw new MessagingException("Connection Factory not started"); } - synchronized (this.lifecycleMonitor) { + this.lifecycleMonitor.lock(); + try { if (this.taskExecutor == null) { this.privateExecutor = true; this.taskExecutor = Executors.newCachedThreadPool(); } return this.taskExecutor; } + finally { + this.lifecycleMonitor.unlock(); + } } /** @@ -561,7 +570,8 @@ protected Executor getTaskExecutor() { @Override public void stop() { this.active = false; - synchronized (this.connections) { + this.connectionsMonitor.lock(); + try { Iterator> iterator = this.connections.entrySet().iterator(); while (iterator.hasNext()) { TcpConnectionSupport connection = iterator.next().getValue(); @@ -575,7 +585,12 @@ public void stop() { } } } - synchronized (this.lifecycleMonitor) { + finally { + this.connectionsMonitor.unlock(); + } + + this.lifecycleMonitor.lock(); + try { if (this.privateExecutor) { ExecutorService executorService = (ExecutorService) this.taskExecutor; executorService.shutdown(); @@ -598,6 +613,9 @@ public void stop() { } } } + finally { + this.lifecycleMonitor.unlock(); + } logger.info(() -> "stopped " + this); } @@ -849,7 +867,8 @@ protected void doAccept(final Selector selector, ServerSocketChannel server, lon } protected void addConnection(TcpConnectionSupport connection) { - synchronized (this.connections) { + this.connectionsMonitor.lock(); + try { if (!this.active) { connection.close(); return; @@ -857,6 +876,9 @@ protected void addConnection(TcpConnectionSupport connection) { this.connections.put(connection.getConnectionId(), connection); logger.debug(() -> getComponentName() + ": Added new connection: " + connection.getConnectionId()); } + finally { + this.connectionsMonitor.unlock(); + } } /** @@ -864,7 +886,8 @@ protected void addConnection(TcpConnectionSupport connection) { * @return a list of open connection ids. */ private List removeClosedConnectionsAndReturnOpenConnectionIds() { - synchronized (this.connections) { + this.connectionsMonitor.lock(); + try { List openConnectionIds = new ArrayList<>(); Iterator> iterator = this.connections.entrySet().iterator(); while (iterator.hasNext()) { @@ -888,6 +911,9 @@ private List removeClosedConnectionsAndReturnOpenConnectionIds() { } return openConnectionIds; } + finally { + this.connectionsMonitor.unlock(); + } } /** @@ -948,7 +974,8 @@ public List getOpenConnectionIds() { public boolean closeConnection(String connectionId) { Assert.notNull(connectionId, "'connectionId' to close must not be null"); // closed connections are removed from #connections in #harvestClosedConnections() - synchronized (this.connections) { + this.connectionsMonitor.lock(); + try { boolean closed = false; TcpConnectionSupport connection = this.connections.remove(connectionId); if (connection != null) { @@ -964,6 +991,9 @@ public boolean closeConnection(String connectionId) { } return closed; } + finally { + this.connectionsMonitor.unlock(); + } } @Override diff --git a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/AbstractServerConnectionFactory.java b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/AbstractServerConnectionFactory.java index 6080df8960b..dba39b669f6 100644 --- a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/AbstractServerConnectionFactory.java +++ b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/AbstractServerConnectionFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2001-2022 the original author or authors. + * Copyright 2001-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -37,6 +37,7 @@ * * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov * * @since 2.0 */ @@ -75,13 +76,17 @@ public SocketAddress getServerSocketAddress() { @Override public void start() { - synchronized (this.lifecycleMonitor) { + this.lifecycleMonitor.lock(); + try { if (!isActive()) { this.setActive(true); this.shuttingDown = false; getTaskExecutor().execute(this); } } + finally { + this.lifecycleMonitor.unlock(); + } super.start(); } diff --git a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/CachingClientConnectionFactory.java b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/CachingClientConnectionFactory.java index b667976f0d7..f8ef6f00c29 100644 --- a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/CachingClientConnectionFactory.java +++ b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/CachingClientConnectionFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,6 +20,8 @@ import java.util.Map; import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.springframework.beans.factory.DisposableBean; import org.springframework.core.serializer.Deserializer; @@ -39,11 +41,15 @@ * false, or cache starvation will result. * * @author Gary Russell + * @author Christian Tzolov + * * @since 2.2 * */ public class CachingClientConnectionFactory extends AbstractClientConnectionFactory implements DisposableBean { + private final Lock lock = new ReentrantLock(); + private final AbstractClientConnectionFactory targetConnectionFactory; private final SimplePool pool; @@ -385,9 +391,15 @@ public void start() { } @Override - public synchronized void stop() { - this.targetConnectionFactory.stop(); - this.pool.removeAllIdleItems(); + public void stop() { + this.lock.lock(); + try { + this.targetConnectionFactory.stop(); + this.pool.removeAllIdleItems(); + } + finally { + this.lock.unlock(); + } } @Override diff --git a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/ClientModeConnectionManager.java b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/ClientModeConnectionManager.java index 663f3223e21..be92fce3970 100644 --- a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/ClientModeConnectionManager.java +++ b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/ClientModeConnectionManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,6 +17,8 @@ package org.springframework.integration.ip.tcp.connection; import java.util.Objects; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -31,6 +33,7 @@ * * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov * * @since 2.1 * @@ -39,6 +42,8 @@ public class ClientModeConnectionManager implements Runnable { private final Log logger = LogFactory.getLog(this.getClass()); + private final Lock lock = new ReentrantLock(); + private final AbstractConnectionFactory clientConnectionFactory; private volatile TcpConnection lastConnection; @@ -54,7 +59,8 @@ public ClientModeConnectionManager( @Override public void run() { - synchronized (this.clientConnectionFactory) { + this.lock.lock(); + try { try { TcpConnection connection = this.clientConnectionFactory.getConnection(); if (!Objects.equals(connection, this.lastConnection)) { @@ -73,6 +79,9 @@ public void run() { this.logger.error("Could not establish connection using " + this.clientConnectionFactory, ex); } } + finally { + this.lock.unlock(); + } } public boolean isConnected() { diff --git a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/FailoverClientConnectionFactory.java b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/FailoverClientConnectionFactory.java index 74da615ec37..3bb492fa023 100644 --- a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/FailoverClientConnectionFactory.java +++ b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/FailoverClientConnectionFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,6 +21,8 @@ import java.util.List; import java.util.UUID; import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import javax.net.ssl.SSLSession; @@ -37,6 +39,8 @@ * succeeds or the list is exhausted. * * @author Gary Russell + * @author Christian Tzolov + * * @since 2.2 * */ @@ -224,6 +228,8 @@ public boolean isRunning() { */ private final class FailoverTcpConnection extends TcpConnectionSupport implements TcpListener { + private final Lock lock = new ReentrantLock(); + private final List connectionFactories; private final String connectionId; @@ -257,46 +263,51 @@ void incrementEpoch() { * factories are down. * @throws InterruptedException if interrupted. */ - private synchronized void findAConnection() throws InterruptedException { - boolean success = false; - AbstractClientConnectionFactory lastFactoryToTry = this.currentFactory; - AbstractClientConnectionFactory nextFactory = null; - if (!this.factoryIterator.hasNext()) { - this.factoryIterator = this.connectionFactories.iterator(); - } - boolean restartedList = false; - while (!success) { - try { - nextFactory = this.factoryIterator.next(); - this.delegate = nextFactory.getConnection(); - if (logger.isDebugEnabled()) { - logger.debug("Got " + this.delegate.getConnectionId() + " from " + nextFactory); - } - this.delegate.registerListener(this); - this.currentFactory = nextFactory; - success = this.delegate.isOpen(); + private void findAConnection() throws InterruptedException { + this.lock.lock(); + try { + boolean success = false; + AbstractClientConnectionFactory lastFactoryToTry = this.currentFactory; + AbstractClientConnectionFactory nextFactory = null; + if (!this.factoryIterator.hasNext()) { + this.factoryIterator = this.connectionFactories.iterator(); } - catch (RuntimeException e) { - if (logger.isDebugEnabled()) { - logger.debug(nextFactory + " failed with " - + e.toString() - + ", trying another"); - } - if (restartedList && (lastFactoryToTry == null || lastFactoryToTry.equals(nextFactory))) { - logger.debug("Failover failed to find a connection"); - /* - * We've tried every factory including the - * one the current connection was on. - */ - this.open = false; - throw e; + boolean restartedList = false; + while (!success) { + try { + nextFactory = this.factoryIterator.next(); + this.delegate = nextFactory.getConnection(); + if (logger.isDebugEnabled()) { + logger.debug("Got " + this.delegate.getConnectionId() + " from " + nextFactory); + } + this.delegate.registerListener(this); + this.currentFactory = nextFactory; + success = this.delegate.isOpen(); } - if (!this.factoryIterator.hasNext()) { - this.factoryIterator = this.connectionFactories.iterator(); - restartedList = true; + catch (RuntimeException e) { + if (logger.isDebugEnabled()) { + logger.debug(nextFactory + " failed with " + + e.toString() + + ", trying another"); + } + if (restartedList && (lastFactoryToTry == null || lastFactoryToTry.equals(nextFactory))) { + logger.debug("Failover failed to find a connection"); + /* + * We've tried every factory including the one the current connection was on. + */ + this.open = false; + throw e; + } + if (!this.factoryIterator.hasNext()) { + this.factoryIterator = this.connectionFactories.iterator(); + restartedList = true; + } } } } + finally { + this.lock.unlock(); + } } @Override @@ -316,39 +327,46 @@ public boolean isOpen() { * If send fails on a connection from every factory, we give up. */ @Override - public synchronized void send(Message message) { - boolean success = false; - AbstractClientConnectionFactory lastFactoryToTry = this.currentFactory; - AbstractClientConnectionFactory lastFactoryTried = null; - boolean retried = false; - while (!success) { - try { - lastFactoryTried = this.currentFactory; - this.delegate.send(message); - success = true; - } - catch (RuntimeException e) { - if (retried && lastFactoryTried.equals(lastFactoryToTry)) { - logger.error("All connection factories exhausted", e); - this.open = false; - throw e; - } - retried = true; - if (logger.isDebugEnabled()) { - logger.debug("Send to " + this.delegate.getConnectionId() + " failed; attempting failover", e); - } - this.delegate.close(); + public void send(Message message) { + this.lock.lock(); + try { + boolean success = false; + AbstractClientConnectionFactory lastFactoryToTry = this.currentFactory; + AbstractClientConnectionFactory lastFactoryTried = null; + boolean retried = false; + while (!success) { try { - findAConnection(); - } - catch (@SuppressWarnings("unused") InterruptedException e1) { - Thread.currentThread().interrupt(); + lastFactoryTried = this.currentFactory; + this.delegate.send(message); + success = true; } - if (logger.isDebugEnabled()) { - logger.debug("Failing over to " + this.delegate.getConnectionId()); + catch (RuntimeException e) { + if (retried && lastFactoryTried.equals(lastFactoryToTry)) { + logger.error("All connection factories exhausted", e); + this.open = false; + throw e; + } + retried = true; + if (logger.isDebugEnabled()) { + logger.debug("Send to " + this.delegate.getConnectionId() + " failed; attempting failover", + e); + } + this.delegate.close(); + try { + findAConnection(); + } + catch (@SuppressWarnings("unused") InterruptedException e1) { + Thread.currentThread().interrupt(); + } + if (logger.isDebugEnabled()) { + logger.debug("Failing over to " + this.delegate.getConnectionId()); + } } } } + finally { + this.lock.unlock(); + } } @Override diff --git a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpConnectionInterceptorSupport.java b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpConnectionInterceptorSupport.java index 8bde317cc05..08a8d8539fd 100644 --- a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpConnectionInterceptorSupport.java +++ b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpConnectionInterceptorSupport.java @@ -18,6 +18,8 @@ import java.util.Collections; import java.util.List; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import javax.net.ssl.SSLSession; @@ -34,11 +36,14 @@ * * @author Gary Russell * @author Kazuki Shimizu + * @author Christian Tzolov * * @since 2.0 */ public abstract class TcpConnectionInterceptorSupport extends TcpConnectionSupport implements TcpConnectionInterceptor { + private final Lock lock = new ReentrantLock(); + private TcpConnectionSupport theConnection; private TcpListener tcpListener; @@ -238,17 +243,23 @@ public void addNewConnection(TcpConnection connection) { } @Override - public synchronized void removeDeadConnection(TcpConnection connection) { - if (this.removed) { - return; - } - this.removed = true; - if (this.theConnection instanceof TcpConnectionInterceptorSupport && !this.theConnection.equals(this)) { - ((TcpConnectionInterceptorSupport) this.theConnection).removeDeadConnection(this); + public void removeDeadConnection(TcpConnection connection) { + this.lock.lock(); + try { + if (this.removed) { + return; + } + this.removed = true; + if (this.theConnection instanceof TcpConnectionInterceptorSupport && !this.theConnection.equals(this)) { + ((TcpConnectionInterceptorSupport) this.theConnection).removeDeadConnection(this); + } + TcpSender sender = getSender(); + if (sender != null && !(sender instanceof TcpConnectionInterceptorSupport)) { + this.interceptedSenders.forEach(snder -> snder.removeDeadConnection(connection)); + } } - TcpSender sender = getSender(); - if (sender != null && !(sender instanceof TcpConnectionInterceptorSupport)) { - this.interceptedSenders.forEach(snder -> snder.removeDeadConnection(connection)); + finally { + this.lock.unlock(); } } diff --git a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpNetConnection.java b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpNetConnection.java index b7defc78e8e..22e16e0c6b6 100644 --- a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpNetConnection.java +++ b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpNetConnection.java @@ -1,5 +1,5 @@ /* - * Copyright 2001-2021 the original author or authors. + * Copyright 2001-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -24,6 +24,8 @@ import java.net.Socket; import java.net.SocketException; import java.net.SocketTimeoutException; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.function.Supplier; import javax.net.ssl.SSLSession; @@ -43,12 +45,15 @@ * * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov * * @since 2.0 * */ public class TcpNetConnection extends TcpConnectionSupport implements SchedulingAwareRunnable { + private final Lock lock = new ReentrantLock(); + private final Socket socket; private volatile OutputStream socketOutputStream; @@ -102,7 +107,8 @@ public boolean isOpen() { @Override @SuppressWarnings("unchecked") - public synchronized void send(Message message) { + public void send(Message message) { + this.lock.lock(); try { if (this.socketOutputStream == null) { int writeBufferSize = this.socket.getSendBufferSize(); @@ -121,6 +127,9 @@ public synchronized void send(Message message) { closeConnection(true); throw mex; } + finally { + this.lock.unlock(); + } if (logger.isDebugEnabled()) { logger.debug(getConnectionId() + " Message sent " + message); } diff --git a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpNioClientConnectionFactory.java b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpNioClientConnectionFactory.java index 6764db6d349..446d335f38d 100644 --- a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpNioClientConnectionFactory.java +++ b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpNioClientConnectionFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -39,6 +39,7 @@ * * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov * * @since 2.0 * @@ -170,12 +171,16 @@ public void stop() { @Override public void start() { - synchronized (this.lifecycleMonitor) { + this.lifecycleMonitor.lock(); + try { if (!isActive()) { setActive(true); getTaskExecutor().execute(this); } } + finally { + this.lifecycleMonitor.unlock(); + } super.start(); } diff --git a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpNioConnection.java b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpNioConnection.java index fa7c2df42c4..21afc3979d1 100644 --- a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpNioConnection.java +++ b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpNioConnection.java @@ -37,6 +37,8 @@ import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import javax.net.ssl.SSLSession; @@ -56,6 +58,7 @@ * @author John Anderson * @author Artem Bilan * @author David Herschler Shvo + * @author Christian Tzolov * * @since 2.0 * @@ -72,14 +75,20 @@ public class TcpNioConnection extends TcpConnectionSupport { private static final byte[] EOF = new byte[0]; // EOF marker buffer + private final Lock lock = new ReentrantLock(); + private final SocketChannel socketChannel; + private final Lock socketChannelMonitor = new ReentrantLock(); + private final ChannelOutputStream channelOutputStream = new ChannelOutputStream(); private final ChannelInputStream channelInputStream = new ChannelInputStream(); private final AtomicInteger executionControl = new AtomicInteger(); + private final Lock executionControlMonitor = new ReentrantLock(); + private boolean usingDirectBuffers; private long pipeTimeout = DEFAULT_PIPE_TIMEOUT; @@ -154,7 +163,8 @@ public boolean isOpen() { @Override @SuppressWarnings("unchecked") public void send(Message message) { - synchronized (this.socketChannel) { + this.socketChannelMonitor.lock(); + try { try { if (this.bufferedOutputStream == null) { int writeBufferSize = this.socketChannel.socket().getSendBufferSize(); @@ -177,6 +187,9 @@ public void send(Message message) { logger.debug(getConnectionId() + " Message sent " + message); } } + finally { + this.socketChannelMonitor.unlock(); + } } @Override @@ -311,7 +324,8 @@ private boolean checkForMoreData() { // timing was such that we were the last assembler and // a new one wasn't run if (dataAvailable()) { - synchronized (this.executionControl) { + this.executionControlMonitor.lock(); + try { if (this.executionControl.incrementAndGet() <= 1) { // only continue if we don't already have another assembler running this.executionControl.set(1); @@ -322,6 +336,9 @@ private boolean checkForMoreData() { this.executionControl.decrementAndGet(); } } + finally { + this.executionControlMonitor.unlock(); + } } if (moreDataAvailable) { if (logger.isTraceEnabled()) { @@ -352,43 +369,50 @@ private boolean dataAvailable() { * @throws IOException an IO exception */ @Nullable - private synchronized Message convert() throws IOException { - if (logger.isTraceEnabled()) { - logger.trace(getConnectionId() + " checking data avail (convert): " + this.channelInputStream.available() + - " pending: " + (this.writingToPipe)); - } - if (this.channelInputStream.available() <= 0) { - try { - if (this.writingLatch.await(SIXTY, TimeUnit.SECONDS)) { - if (this.channelInputStream.available() <= 0) { - return null; + private Message convert() throws IOException { + this.lock.lock(); + try { + if (logger.isTraceEnabled()) { + logger.trace( + getConnectionId() + " checking data avail (convert): " + this.channelInputStream.available() + + " pending: " + (this.writingToPipe)); + } + if (this.channelInputStream.available() <= 0) { + try { + if (this.writingLatch.await(SIXTY, TimeUnit.SECONDS)) { + if (this.channelInputStream.available() <= 0) { + return null; + } + } + else { // should never happen + throw new IOException("Timed out waiting for IO"); } } - else { // should never happen - throw new IOException("Timed out waiting for IO"); + catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IOException("Interrupted waiting for IO", e); } } - catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw new IOException("Interrupted waiting for IO", e); + try { + return getMapper().toMessage(this); } - } - try { - return getMapper().toMessage(this); - } - catch (Exception e) { - closeConnection(true); - if (e instanceof SocketTimeoutException) { // NOSONAR instanceof - if (logger.isDebugEnabled()) { - logger.debug("Closing socket after timeout " + getConnectionId()); + catch (Exception e) { + closeConnection(true); + if (e instanceof SocketTimeoutException) { // NOSONAR instanceof + if (logger.isDebugEnabled()) { + logger.debug("Closing socket after timeout " + getConnectionId()); + } } - } - else { - if (!(e instanceof SoftEndOfStreamException)) { // NOSONAR instanceof - throw e; + else { + if (!(e instanceof SoftEndOfStreamException)) { // NOSONAR instanceof + throw e; + } } + return null; } - return null; + } + finally { + this.lock.unlock(); } } @@ -468,7 +492,8 @@ protected void sendToPipe(ByteBuffer rawBufferToSend) throws IOException { } private void checkForAssembler() { - synchronized (this.executionControl) { + this.executionControlMonitor.lock(); + try { if (this.executionControl.incrementAndGet() <= 1) { // only execute run() if we don't already have one running this.executionControl.set(1); @@ -489,6 +514,9 @@ private void checkForAssembler() { this.executionControl.decrementAndGet(); } } + finally { + this.executionControlMonitor.unlock(); + } } /** @@ -605,6 +633,8 @@ class ChannelOutputStream extends OutputStream { private int soTimeout; + private final Lock innerLock = new ReentrantLock(); + @Override public void write(int b) throws IOException { byte[] bytes = new byte[1]; @@ -632,28 +662,35 @@ public void write(byte[] b) throws IOException { doWrite(buffer); } - protected synchronized void doWrite(ByteBuffer buffer) throws IOException { - if (logger.isDebugEnabled()) { - logger.debug(getConnectionId() + " writing " + buffer.remaining()); - } - TcpNioConnection.this.socketChannel.write(buffer); - int remaining = buffer.remaining(); - if (remaining == 0) { - return; - } - if (this.selector == null) { - this.selector = Selector.open(); - this.soTimeout = TcpNioConnection.this.socketChannel.socket().getSoTimeout(); - } - TcpNioConnection.this.socketChannel.register(this.selector, SelectionKey.OP_WRITE); - while (remaining > 0) { - int selectionCount = this.selector.select(this.soTimeout); - if (selectionCount == 0) { - throw new SocketTimeoutException("Timeout on write"); + protected void doWrite(ByteBuffer buffer) throws IOException { + this.innerLock.lock(); + try { + + if (logger.isDebugEnabled()) { + logger.debug(getConnectionId() + " writing " + buffer.remaining()); } - this.selector.selectedKeys().clear(); TcpNioConnection.this.socketChannel.write(buffer); - remaining = buffer.remaining(); + int remaining = buffer.remaining(); + if (remaining == 0) { + return; + } + if (this.selector == null) { + this.selector = Selector.open(); + this.soTimeout = TcpNioConnection.this.socketChannel.socket().getSoTimeout(); + } + TcpNioConnection.this.socketChannel.register(this.selector, SelectionKey.OP_WRITE); + while (remaining > 0) { + int selectionCount = this.selector.select(this.soTimeout); + if (selectionCount == 0) { + throw new SocketTimeoutException("Timeout on write"); + } + this.selector.selectedKeys().clear(); + TcpNioConnection.this.socketChannel.write(buffer); + remaining = buffer.remaining(); + } + } + finally { + this.innerLock.unlock(); } } @@ -680,6 +717,8 @@ class ChannelInputStream extends InputStream { private volatile boolean isClosed; + private final Lock innerLock = new ReentrantLock(); + @Override public int read(byte[] b, int off, int len) throws IOException { Assert.notNull(b, "byte[] cannot be null"); @@ -708,30 +747,36 @@ else if (len == 0) { } @Override - public synchronized int read() throws IOException { - if (this.isClosed && this.available.get() == 0) { - if (TcpNioConnection.this.timedOut) { - throw new SocketTimeoutException("Connection has timed out"); - } - return -1; - } - if (this.currentBuffer == null) { - this.currentBuffer = getNextBuffer(); - this.currentOffset = 0; - if (this.currentBuffer == null) { + public int read() throws IOException { + this.innerLock.lock(); + try { + if (this.isClosed && this.available.get() == 0) { if (TcpNioConnection.this.timedOut) { throw new SocketTimeoutException("Connection has timed out"); } return -1; } + if (this.currentBuffer == null) { + this.currentBuffer = getNextBuffer(); + this.currentOffset = 0; + if (this.currentBuffer == null) { + if (TcpNioConnection.this.timedOut) { + throw new SocketTimeoutException("Connection has timed out"); + } + return -1; + } + } + int bite; + bite = this.currentBuffer[this.currentOffset++] & 0xff; // NOSONAR + this.available.decrementAndGet(); + if (this.currentOffset >= this.currentBuffer.length) { + this.currentBuffer = null; + } + return bite; } - int bite; - bite = this.currentBuffer[this.currentOffset++] & 0xff; // NOSONAR - this.available.decrementAndGet(); - if (this.currentOffset >= this.currentBuffer.length) { - this.currentBuffer = null; + finally { + this.innerLock.unlock(); } - return bite; } @Nullable diff --git a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpNioSSLConnection.java b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpNioSSLConnection.java index 48c36881d90..0486f9dd43c 100644 --- a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpNioSSLConnection.java +++ b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpNioSSLConnection.java @@ -21,6 +21,8 @@ import java.nio.channels.SocketChannel; import java.util.concurrent.Semaphore; import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLEngineResult; @@ -51,6 +53,7 @@ * * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov * * @since 2.2 * @@ -67,7 +70,7 @@ public class TcpNioSSLConnection extends TcpNioConnection { private final Semaphore semaphore = new Semaphore(0); - private final Object monitorLock = new Object(); + private final Lock monitorLock = new ReentrantLock(); private int handshakeTimeout = DEFAULT_HANDSHAKE_TIMEOUT; @@ -285,12 +288,16 @@ private void initializeEngine() { @Override protected ChannelOutputStream getChannelOutputStream() { - synchronized (this.monitorLock) { + this.monitorLock.lock(); + try { if (this.sslChannelOutputStream == null) { this.sslChannelOutputStream = new SSLChannelOutputStream(super.getChannelOutputStream()); } return this.sslChannelOutputStream; } + finally { + this.monitorLock.unlock(); + } } protected SSLChannelOutputStream getSSLChannelOutputStream() { @@ -318,6 +325,8 @@ public void close() { */ final class SSLChannelOutputStream extends ChannelOutputStream { + private final Lock lock = new ReentrantLock(); + private final ChannelOutputStream channelOutputStream; SSLChannelOutputStream(ChannelOutputStream channelOutputStream) { @@ -331,7 +340,8 @@ final class SSLChannelOutputStream extends ChannelOutputStream { * and multiple writes will be necessary. */ @Override - protected synchronized void doWrite(ByteBuffer plainText) throws IOException { + protected void doWrite(ByteBuffer plainText) throws IOException { + this.lock.lock(); try { TcpNioSSLConnection.this.writerActive = true; int remaining = plainText.remaining(); @@ -357,6 +367,7 @@ protected synchronized void doWrite(ByteBuffer plainText) throws IOException { } finally { TcpNioSSLConnection.this.writerActive = false; + this.lock.unlock(); } } diff --git a/spring-integration-ip/src/main/java/org/springframework/integration/ip/udp/MulticastReceivingChannelAdapter.java b/spring-integration-ip/src/main/java/org/springframework/integration/ip/udp/MulticastReceivingChannelAdapter.java index 74552990099..0dbaa0b064e 100644 --- a/spring-integration-ip/src/main/java/org/springframework/integration/ip/udp/MulticastReceivingChannelAdapter.java +++ b/spring-integration-ip/src/main/java/org/springframework/integration/ip/udp/MulticastReceivingChannelAdapter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -32,6 +32,7 @@ * @author Gary Russell * @author Marcin Pilaczynski * @author Artem Bilan + * @author Christian Tzolov * * @since 2.0 */ @@ -39,7 +40,6 @@ public class MulticastReceivingChannelAdapter extends UnicastReceivingChannelAda private final String group; - /** * Constructs a MulticastReceivingChannelAdapter that listens for packets on the * specified multichannel address (group) and port. @@ -65,24 +65,31 @@ public MulticastReceivingChannelAdapter(String group, int port, boolean lengthCh } @Override - public synchronized DatagramSocket getSocket() { - if (getTheSocket() == null) { - try { - int port = getPort(); - MulticastSocket socket = port == 0 ? new MulticastSocket() : new MulticastSocket(port); - String localAddress = getLocalAddress(); - if (localAddress != null) { - socket.setNetworkInterface(NetworkInterface.getByInetAddress(InetAddress.getByName(localAddress))); + public DatagramSocket getSocket() { + this.lock.lock(); + try { + if (getTheSocket() == null) { + try { + int port = getPort(); + MulticastSocket socket = port == 0 ? new MulticastSocket() : new MulticastSocket(port); + String localAddress = getLocalAddress(); + if (localAddress != null) { + socket.setNetworkInterface( + NetworkInterface.getByInetAddress(InetAddress.getByName(localAddress))); + } + setSocketAttributes(socket); + socket.joinGroup(new InetSocketAddress(this.group, 0), null); + setSocket(socket); + } + catch (IOException e) { + throw new MessagingException("failed to create DatagramSocket", e); } - setSocketAttributes(socket); - socket.joinGroup(new InetSocketAddress(this.group, 0), null); - setSocket(socket); - } - catch (IOException e) { - throw new MessagingException("failed to create DatagramSocket", e); } + return super.getSocket(); + } + finally { + this.lock.unlock(); } - return super.getSocket(); } } diff --git a/spring-integration-ip/src/main/java/org/springframework/integration/ip/udp/MulticastSendingMessageHandler.java b/spring-integration-ip/src/main/java/org/springframework/integration/ip/udp/MulticastSendingMessageHandler.java index e863c3b8821..b6b57190fc1 100644 --- a/spring-integration-ip/src/main/java/org/springframework/integration/ip/udp/MulticastSendingMessageHandler.java +++ b/spring-integration-ip/src/main/java/org/springframework/integration/ip/udp/MulticastSendingMessageHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2001-2020 the original author or authors. + * Copyright 2001-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -37,6 +37,7 @@ * * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov * * @since 2.0 */ @@ -129,11 +130,17 @@ public MulticastSendingMessageHandler(String destinationExpression) { } @Override - protected synchronized DatagramSocket getSocket() throws IOException { - if (getTheSocket() == null) { - createSocket(); + protected DatagramSocket getSocket() throws IOException { + this.lock.lock(); + try { + if (getTheSocket() == null) { + createSocket(); + } + return super.getSocket(); + } + finally { + this.lock.unlock(); } - return super.getSocket(); } private void createSocket() throws IOException { diff --git a/spring-integration-ip/src/main/java/org/springframework/integration/ip/udp/UnicastReceivingChannelAdapter.java b/spring-integration-ip/src/main/java/org/springframework/integration/ip/udp/UnicastReceivingChannelAdapter.java index 1f045c7e70b..db8b380c3e1 100644 --- a/spring-integration-ip/src/main/java/org/springframework/integration/ip/udp/UnicastReceivingChannelAdapter.java +++ b/spring-integration-ip/src/main/java/org/springframework/integration/ip/udp/UnicastReceivingChannelAdapter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -25,6 +25,8 @@ import java.net.SocketTimeoutException; import java.util.concurrent.Executor; import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -44,6 +46,7 @@ * * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov * * @since 2.0 */ @@ -53,6 +56,8 @@ public class UnicastReceivingChannelAdapter extends AbstractInternetProtocolRece private final DatagramPacketMessageMapper mapper = new DatagramPacketMessageMapper(); + protected final Lock lock = new ReentrantLock(); + private DatagramSocket socket; private boolean socketExplicitlySet; @@ -248,27 +253,33 @@ protected DatagramSocket getTheSocket() { return this.socket; } - public synchronized DatagramSocket getSocket() { - if (this.socket == null) { - try { - DatagramSocket datagramSocket; - String localAddress = getLocalAddress(); - int port = super.getPort(); - if (localAddress == null) { - datagramSocket = port == 0 ? new DatagramSocket() : new DatagramSocket(port); + public DatagramSocket getSocket() { + this.lock.lock(); + try { + if (this.socket == null) { + try { + DatagramSocket datagramSocket; + String localAddress = getLocalAddress(); + int port = super.getPort(); + if (localAddress == null) { + datagramSocket = port == 0 ? new DatagramSocket() : new DatagramSocket(port); + } + else { + InetAddress whichNic = InetAddress.getByName(localAddress); + datagramSocket = new DatagramSocket(new InetSocketAddress(whichNic, port)); + } + setSocketAttributes(datagramSocket); + this.socket = datagramSocket; } - else { - InetAddress whichNic = InetAddress.getByName(localAddress); - datagramSocket = new DatagramSocket(new InetSocketAddress(whichNic, port)); + catch (IOException e) { + throw new MessagingException("failed to create DatagramSocket", e); } - setSocketAttributes(datagramSocket); - this.socket = datagramSocket; - } - catch (IOException e) { - throw new MessagingException("failed to create DatagramSocket", e); } + return this.socket; + } + finally { + this.lock.unlock(); } - return this.socket; } /** diff --git a/spring-integration-ip/src/main/java/org/springframework/integration/ip/udp/UnicastSendingMessageHandler.java b/spring-integration-ip/src/main/java/org/springframework/integration/ip/udp/UnicastSendingMessageHandler.java index a5a8c40aa92..ba638af6ae0 100644 --- a/spring-integration-ip/src/main/java/org/springframework/integration/ip/udp/UnicastSendingMessageHandler.java +++ b/spring-integration-ip/src/main/java/org/springframework/integration/ip/udp/UnicastSendingMessageHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2001-2022 the original author or authors. + * Copyright 2001-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -34,6 +34,8 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.springframework.expression.EvaluationContext; import org.springframework.expression.Expression; @@ -58,6 +60,7 @@ * @author Gary Russell * @author Marcin Pilaczynski * @author Artem Bilan + * @author Christian Tzolov * * @since 2.0 */ @@ -66,6 +69,8 @@ public class UnicastSendingMessageHandler extends private static final int DEFAULT_ACK_TIMEOUT = 5000; + protected final Lock lock = new ReentrantLock(); + private final DatagramPacketMessageMapper mapper = new DatagramPacketMessageMapper(); private final Map ackControl = Collections.synchronizedMap(new HashMap<>()); @@ -316,7 +321,8 @@ public void handleMessageInternal(Message message) { public void startAckThread() { if (!this.ackThreadRunning) { - synchronized (this) { + this.lock.lock(); + try { if (!this.ackThreadRunning) { try { getSocket(); @@ -334,6 +340,9 @@ public void startAckThread() { } } } + finally { + this.lock.unlock(); + } } } @@ -386,28 +395,34 @@ protected DatagramSocket getTheSocket() { return this.socket; } - protected synchronized DatagramSocket getSocket() throws IOException { - if (this.socket == null) { - if (this.acknowledge) { - if (this.localAddress == null) { - this.socket = this.ackPort == 0 ? new DatagramSocket() : new DatagramSocket(this.ackPort); + protected DatagramSocket getSocket() throws IOException { + this.lock.lock(); + try { + if (this.socket == null) { + if (this.acknowledge) { + if (this.localAddress == null) { + this.socket = this.ackPort == 0 ? new DatagramSocket() : new DatagramSocket(this.ackPort); + } + else { + InetAddress whichNic = InetAddress.getByName(this.localAddress); + this.socket = new DatagramSocket(new InetSocketAddress(whichNic, this.ackPort)); + } + if (this.soReceiveBufferSize > 0) { + this.socket.setReceiveBufferSize(this.soReceiveBufferSize); + } + logger.debug(() -> "Listening for acks on port: " + getAckPort()); + updateAckAddress(); } else { - InetAddress whichNic = InetAddress.getByName(this.localAddress); - this.socket = new DatagramSocket(new InetSocketAddress(whichNic, this.ackPort)); + this.socket = new DatagramSocket(); } - if (this.soReceiveBufferSize > 0) { - this.socket.setReceiveBufferSize(this.soReceiveBufferSize); - } - logger.debug(() -> "Listening for acks on port: " + getAckPort()); - updateAckAddress(); + setSocketAttributes(this.socket); } - else { - this.socket = new DatagramSocket(); - } - setSocketAttributes(this.socket); + return this.socket; + } + finally { + this.lock.unlock(); } - return this.socket; } protected void updateAckAddress() { @@ -424,8 +439,14 @@ public void setSoReceiveBufferSize(int size) { } @Override - public synchronized void setLocalAddress(String localAddress) { - this.localAddress = localAddress; + public void setLocalAddress(String localAddress) { + this.lock.lock(); + try { + this.localAddress = localAddress; + } + finally { + this.lock.unlock(); + } } public void setTaskExecutor(Executor taskExecutor) { diff --git a/spring-integration-jdbc/src/main/java/org/springframework/integration/jdbc/StoredProcExecutor.java b/spring-integration-jdbc/src/main/java/org/springframework/integration/jdbc/StoredProcExecutor.java index 294e0c789d1..7b2b95e2cbf 100644 --- a/spring-integration-jdbc/src/main/java/org/springframework/integration/jdbc/StoredProcExecutor.java +++ b/spring-integration-jdbc/src/main/java/org/springframework/integration/jdbc/StoredProcExecutor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -23,6 +23,8 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import javax.sql.DataSource; @@ -51,6 +53,7 @@ * @author Gunnar Hillert * @author Artem Bilan * @author Gary Russell + * @author Christian Tzolov * * @since 2.1 * @@ -63,7 +66,7 @@ public class StoredProcExecutor implements BeanFactoryAware, InitializingBean { private final DataSource dataSource; - private final Object jdbcCallOperationsMapMonitor = new Object(); + private final Lock jdbcCallOperationsMapMonitor = new ReentrantLock(); private Map> returningResultSetRowMappers = new HashMap<>(0); @@ -301,10 +304,14 @@ private Map executeStoredProcedureInternal(Object input, String private SimpleJdbcCallOperations obtainSimpleJdbcCall(String storedProcedureName) { SimpleJdbcCallOperations operations = this.jdbcCallOperationsMap.get(storedProcedureName); if (operations == null) { - synchronized (this.jdbcCallOperationsMapMonitor) { + this.jdbcCallOperationsMapMonitor.lock(); + try { operations = this.jdbcCallOperationsMap.computeIfAbsent(storedProcedureName, this::createSimpleJdbcCall); } + finally { + this.jdbcCallOperationsMapMonitor.unlock(); + } } return operations; } diff --git a/spring-integration-jdbc/src/main/java/org/springframework/integration/jdbc/channel/PostgresChannelMessageTableSubscriber.java b/spring-integration-jdbc/src/main/java/org/springframework/integration/jdbc/channel/PostgresChannelMessageTableSubscriber.java index 08d0c74b4f0..d0a5cd82902 100644 --- a/spring-integration-jdbc/src/main/java/org/springframework/integration/jdbc/channel/PostgresChannelMessageTableSubscriber.java +++ b/spring-integration-jdbc/src/main/java/org/springframework/integration/jdbc/channel/PostgresChannelMessageTableSubscriber.java @@ -26,6 +26,8 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.postgresql.PGNotification; import org.postgresql.jdbc.PgConnection; @@ -61,6 +63,7 @@ * @author Rafael Winterhalter * @author Artem Bilan * @author Igor Lovich + * @author Christian Tzolov * * @since 6.0 */ @@ -68,6 +71,8 @@ public final class PostgresChannelMessageTableSubscriber implements SmartLifecyc private static final LogAccessor LOGGER = new LogAccessor(PostgresChannelMessageTableSubscriber.class); + private final Lock lock = new ReentrantLock(); + private final Map> subscriptionsMap = new ConcurrentHashMap<>(); private final PgConnectionSupplier connectionSupplier; @@ -111,8 +116,14 @@ public PostgresChannelMessageTableSubscriber(PgConnectionSupplier connectionSupp * @deprecated since 6.2 in favor of {@link #setTaskExecutor(AsyncTaskExecutor)} */ @Deprecated(since = "6.2", forRemoval = true) - public synchronized void setExecutor(ExecutorService executor) { - setTaskExecutor(new TaskExecutorAdapter(executor)); + public void setExecutor(ExecutorService executor) { + this.lock.lock(); + try { + setTaskExecutor(new TaskExecutorAdapter(executor)); + } + finally { + this.lock.unlock(); + } } /** @@ -149,85 +160,96 @@ public boolean unsubscribe(Subscription subscription) { } @Override - public synchronized void start() { - if (this.latch.getCount() > 0) { - return; - } + public void start() { + this.lock.lock(); + try { + if (this.latch.getCount() > 0) { + return; + } - this.latch = new CountDownLatch(1); + this.latch = new CountDownLatch(1); + + CountDownLatch startingLatch = new CountDownLatch(1); + this.future = this.taskExecutor.submit(() -> { + doStart(startingLatch); + }); - CountDownLatch startingLatch = new CountDownLatch(1); - this.future = this.taskExecutor.submit(() -> { try { - while (isActive()) { - try { - PgConnection conn = this.connectionSupplier.get(); - try (Statement stmt = conn.createStatement()) { - stmt.execute("LISTEN " + this.tablePrefix.toLowerCase() + "channel_message_notify"); + if (!startingLatch.await(5, TimeUnit.SECONDS)) { + throw new IllegalStateException("Failed to start " + this); + } + } + catch (InterruptedException ex) { + Thread.currentThread().interrupt(); + throw new IllegalStateException("Failed to start " + this, ex); + } + } + finally { + this.lock.unlock(); + } + } + + private void doStart(CountDownLatch startingLatch) { + try { + while (isActive()) { + try { + PgConnection conn = this.connectionSupplier.get(); + try (Statement stmt = conn.createStatement()) { + stmt.execute("LISTEN " + this.tablePrefix.toLowerCase() + "channel_message_notify"); + } + catch (Exception ex) { + try { + conn.close(); } - catch (Exception ex) { - try { - conn.close(); - } - catch (Exception suppressed) { - ex.addSuppressed(suppressed); - } - throw ex; + catch (Exception suppressed) { + ex.addSuppressed(suppressed); } - this.subscriptionsMap.values() - .forEach(subscriptions -> subscriptions.forEach(Subscription::notifyUpdate)); - try { - this.connection = conn; - while (isActive()) { - startingLatch.countDown(); - - PGNotification[] notifications = conn.getNotifications(0); - // Unfortunately, there is no good way of interrupting a notification - // poll but by closing its connection. - if (!isActive()) { - return; - } - if (notifications != null) { - for (PGNotification notification : notifications) { - String parameter = notification.getParameter(); - Set subscriptions = this.subscriptionsMap.get(parameter); - if (subscriptions == null) { - continue; - } - for (Subscription subscription : subscriptions) { - subscription.notifyUpdate(); - } + throw ex; + } + this.subscriptionsMap.values() + .forEach(subscriptions -> subscriptions.forEach(Subscription::notifyUpdate)); + try { + this.connection = conn; + while (isActive()) { + startingLatch.countDown(); + + PGNotification[] notifications = conn.getNotifications(0); + // Unfortunately, there is no good way of interrupting a notification + // poll but by closing its connection. + if (!isActive()) { + return; + } + if (notifications != null) { + for (PGNotification notification : notifications) { + String parameter = notification.getParameter(); + Set subscriptions = this.subscriptionsMap.get(parameter); + if (subscriptions == null) { + continue; + } + for (Subscription subscription : subscriptions) { + subscription.notifyUpdate(); } } } } - finally { - conn.close(); - } } - catch (Exception e) { - // The getNotifications method does not throw a meaningful message on interruption. - // Therefore, we do not log an error, unless it occurred while active. - if (isActive()) { - LOGGER.error(e, "Failed to poll notifications from Postgres database"); - } + finally { + conn.close(); + } + } + catch (Exception e) { + // The getNotifications method does not throw a meaningful message on interruption. + // Therefore, we do not log an error, unless it occurred while active. + if (isActive()) { + LOGGER.error(e, "Failed to poll notifications from Postgres database"); } } - } - finally { - this.latch.countDown(); - } - }); - - try { - if (!startingLatch.await(5, TimeUnit.SECONDS)) { - throw new IllegalStateException("Failed to start " + this); } } - catch (InterruptedException ex) { - Thread.currentThread().interrupt(); - throw new IllegalStateException("Failed to start " + this, ex); + finally { + this.latch.countDown(); } + } private boolean isActive() { @@ -239,25 +261,31 @@ private boolean isActive() { } @Override - public synchronized void stop() { - if (this.future.isDone()) { - return; - } - this.future.cancel(true); - PgConnection conn = this.connection; - if (conn != null) { - try { - conn.close(); + public void stop() { + this.lock.lock(); + try { + if (this.future.isDone()) { + return; + } + this.future.cancel(true); + PgConnection conn = this.connection; + if (conn != null) { + try { + conn.close(); + } + catch (SQLException ignored) { + } } - catch (SQLException ignored) { + try { + if (!this.latch.await(5, TimeUnit.SECONDS)) { + throw new IllegalStateException("Failed to stop " + this); + } } - } - try { - if (!this.latch.await(5, TimeUnit.SECONDS)) { - throw new IllegalStateException("Failed to stop " + this); + catch (InterruptedException ignored) { } } - catch (InterruptedException ignored) { + finally { + this.lock.unlock(); } } diff --git a/spring-integration-jdbc/src/main/java/org/springframework/integration/jdbc/lock/JdbcLockRegistry.java b/spring-integration-jdbc/src/main/java/org/springframework/integration/jdbc/lock/JdbcLockRegistry.java index f4f01af77a1..215f89875cc 100644 --- a/spring-integration-jdbc/src/main/java/org/springframework/integration/jdbc/lock/JdbcLockRegistry.java +++ b/spring-integration-jdbc/src/main/java/org/springframework/integration/jdbc/lock/JdbcLockRegistry.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2022 the original author or authors. + * Copyright 2016-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -54,6 +54,7 @@ * @author Olivier Hubaut * @author Fran Aranda * @author Unseok Kim + * @author Christian Tzolov * * @since 4.3 */ @@ -63,6 +64,8 @@ public class JdbcLockRegistry implements ExpirableLockRegistry, RenewableLockReg private static final int DEFAULT_CAPACITY = 100_000; + private final Lock lock = new ReentrantLock(); + private final Map locks = new LinkedHashMap(16, 0.75F, true) { @@ -111,9 +114,13 @@ public void setCacheCapacity(int cacheCapacity) { public Lock obtain(Object lockKey) { Assert.isInstanceOf(String.class, lockKey); String path = pathFor((String) lockKey); - synchronized (this.locks) { + this.lock.lock(); + try { return this.locks.computeIfAbsent(path, key -> new JdbcLock(this.client, this.idleBetweenTries, key)); } + finally { + this.lock.unlock(); + } } private String pathFor(String input) { @@ -123,13 +130,17 @@ private String pathFor(String input) { @Override public void expireUnusedOlderThan(long age) { long now = System.currentTimeMillis(); - synchronized (this.locks) { + this.lock.lock(); + try { this.locks.entrySet() .removeIf(entry -> { JdbcLock lock = entry.getValue(); return now - lock.getLastUsed() > age && !lock.isAcquiredInThisProcess(); }); } + finally { + this.lock.unlock(); + } } @Override @@ -137,9 +148,14 @@ public void renewLock(Object lockKey) { Assert.isInstanceOf(String.class, lockKey); String path = pathFor((String) lockKey); JdbcLock jdbcLock; - synchronized (this.locks) { + this.lock.lock(); + try { jdbcLock = this.locks.get(path); } + finally { + this.lock.unlock(); + } + if (jdbcLock == null) { throw new IllegalStateException("Could not found mutex at " + path); } diff --git a/spring-integration-jms/src/main/java/org/springframework/integration/jms/JmsOutboundGateway.java b/spring-integration-jms/src/main/java/org/springframework/integration/jms/JmsOutboundGateway.java index 59de0f8f644..8a07a7860bf 100644 --- a/spring-integration-jms/src/main/java/org/springframework/integration/jms/JmsOutboundGateway.java +++ b/spring-integration-jms/src/main/java/org/springframework/integration/jms/JmsOutboundGateway.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -29,6 +29,8 @@ import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import jakarta.jms.Connection; import jakarta.jms.ConnectionFactory; @@ -81,6 +83,7 @@ * @author Oleg Zhurakousky * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov */ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler implements ManageableLifecycle, MessageListener { @@ -90,7 +93,7 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler */ public static final long DEFAULT_RECEIVE_TIMEOUT = 5000L; - private final Object initializationMonitor = new Object(); + private final Lock initializationMonitor = new ReentrantLock(); private final AtomicLong correlationId = new AtomicLong(); @@ -100,10 +103,12 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler private final ConcurrentHashMap earlyOrLateReplies = new ConcurrentHashMap<>(); + private final Lock earlyOrLateRepliesMonitor = new ReentrantLock(); + private final Map>> futures = new ConcurrentHashMap<>(); - private final Object lifeCycleMonitor = new Object(); + private final Lock lifeCycleMonitor = new ReentrantLock(); private Destination requestDestination; @@ -512,7 +517,8 @@ private Destination resolveReplyDestination(String repDestinationName, Session s @Override protected void doInit() { - synchronized (this.initializationMonitor) { + this.initializationMonitor.lock(); + try { if (this.initialized) { return; } @@ -539,6 +545,9 @@ protected void doInit() { initializeReplyContainer(); this.initialized = true; } + finally { + this.initializationMonitor.unlock(); + } } private void initializeReplyContainer() { @@ -667,7 +676,8 @@ private void applyReplyContainerProperties(GatewayReplyListenerContainer contain @Override public void start() { - synchronized (this.lifeCycleMonitor) { + this.lifeCycleMonitor.lock(); + try { if (!this.active) { if (this.replyContainer != null) { TaskScheduler taskScheduler = getTaskScheduler(); @@ -689,11 +699,15 @@ public void start() { this.active = true; } } + finally { + this.lifeCycleMonitor.unlock(); + } } @Override public void stop() { - synchronized (this.lifeCycleMonitor) { + this.lifeCycleMonitor.lock(); + try { if (this.replyContainer != null) { this.replyContainer.shutdown(); this.wasStopped = true; @@ -708,6 +722,10 @@ public void stop() { } this.active = false; } + finally { + this.lifeCycleMonitor.unlock(); + } + } @Override @@ -727,7 +745,8 @@ protected Object handleRequestMessage(final Message requestMessage) { } else { if (this.idleReplyContainerTimeout > 0) { - synchronized (this.lifeCycleMonitor) { + this.lifeCycleMonitor.lock(); + try { this.lastSend = System.currentTimeMillis(); if (!this.replyContainer.isRunning()) { logger.debug(() -> getComponentName() + ": Starting reply container."); @@ -738,6 +757,9 @@ protected Object handleRequestMessage(final Message requestMessage) { Duration.ofMillis(this.idleReplyContainerTimeout / 2)); } } + finally { + this.lifeCycleMonitor.unlock(); + } } reply = sendAndReceiveWithContainer(requestMessage); } @@ -1121,13 +1143,17 @@ private jakarta.jms.Message doSendAndReceiveAsyncDefaultCorrelation(Destination /* * Check to see if the reply arrived before we obtained the correlationId */ - synchronized (this.earlyOrLateReplies) { + this.earlyOrLateRepliesMonitor.lock(); + try { TimedReply timedReply = this.earlyOrLateReplies.remove(correlation); if (timedReply != null) { logger.debug(() -> "Found early reply with correlationId " + correlationToLog); replyQueue.add(timedReply.getReply()); } } + finally { + this.earlyOrLateRepliesMonitor.unlock(); + } return obtainReplyFromContainer(correlation, replyQueue); } @@ -1298,13 +1324,17 @@ private void onMessageSync(jakarta.jms.Message message, String correlationId) { } throw new IllegalStateException("No sender waiting for reply"); } - synchronized (this.earlyOrLateReplies) { + this.earlyOrLateRepliesMonitor.lock(); + try { queue = this.replies.get(correlationId); if (queue == null) { logger.debug(() -> "Reply for correlationId " + correlationId + " received early or late"); this.earlyOrLateReplies.put(correlationId, new TimedReply(message)); } } + finally { + this.earlyOrLateRepliesMonitor.unlock(); + } } if (queue != null) { logger.debug(() -> "Received reply with correlationId " + correlationId); @@ -1466,7 +1496,8 @@ private class IdleContainerStopper implements Runnable { @Override public void run() { - synchronized (JmsOutboundGateway.this.lifeCycleMonitor) { + JmsOutboundGateway.this.lifeCycleMonitor.lock(); + try { if (System.currentTimeMillis() - JmsOutboundGateway.this.lastSend > JmsOutboundGateway.this.idleReplyContainerTimeout && JmsOutboundGateway.this.replies.size() == 0 && @@ -1478,6 +1509,9 @@ public void run() { JmsOutboundGateway.this.idleTask = null; } } + finally { + JmsOutboundGateway.this.lifeCycleMonitor.unlock(); + } } } diff --git a/spring-integration-kafka/src/main/java/org/springframework/integration/kafka/inbound/KafkaMessageSource.java b/spring-integration-kafka/src/main/java/org/springframework/integration/kafka/inbound/KafkaMessageSource.java index 7f621e0264a..96d1e18180e 100644 --- a/spring-integration-kafka/src/main/java/org/springframework/integration/kafka/inbound/KafkaMessageSource.java +++ b/spring-integration-kafka/src/main/java/org/springframework/integration/kafka/inbound/KafkaMessageSource.java @@ -1,5 +1,5 @@ /* - * Copyright 2018-2022 the original author or authors. + * Copyright 2018-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -30,6 +30,8 @@ import java.util.TreeSet; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -95,6 +97,7 @@ * @author Mark Norkin * @author Artem Bilan * @author Anshul Mehra + * @author Christian Tzolov * * @since 5.4 * @@ -111,11 +114,13 @@ public class KafkaMessageSource extends AbstractMessageSource impl */ public static final String REMAINING_RECORDS = KafkaHeaders.PREFIX + "remainingRecords"; + private final Lock lock = new ReentrantLock(); + private final ConsumerFactory consumerFactory; private final KafkaAckCallbackFactory ackCallbackFactory; - private final Object consumerMonitor = new Object(); + private final Lock consumerMonitor = new ReentrantLock(); private final Map>> inflightRecords = new ConcurrentHashMap<>(); @@ -385,31 +390,61 @@ private boolean maxPollStringGtr1(Object maxPoll) { } @Override - public synchronized boolean isRunning() { - return this.running; + public boolean isRunning() { + this.lock.lock(); + try { + return this.running; + } + finally { + this.lock.unlock(); + } } @Override - public synchronized void start() { - this.running = true; - this.stopped = false; + public void start() { + this.lock.lock(); + try { + this.running = true; + this.stopped = false; + } + finally { + this.lock.unlock(); + } } @Override - public synchronized void stop() { - stopConsumer(); - this.running = false; - this.stopped = true; + public void stop() { + this.lock.lock(); + try { + stopConsumer(); + this.running = false; + this.stopped = true; + } + finally { + this.lock.unlock(); + } } @Override - public synchronized void pause() { - this.pausing = true; + public void pause() { + this.lock.lock(); + try { + this.pausing = true; + } + finally { + this.lock.unlock(); + } } @Override - public synchronized void resume() { - this.pausing = false; + public void resume() { + this.lock.lock(); + try { + this.pausing = false; + } + finally { + this.lock.unlock(); + } } @Override @@ -418,35 +453,43 @@ public boolean isPaused() { } @Override // NOSONAR - not so complex - protected synchronized Object doReceive() { - if (this.stopped) { - this.logger.debug("Message source is stopped; no records will be returned"); - return null; - } - if (this.consumer == null) { - createConsumer(); - this.running = true; - } - if (this.pausing && !this.paused && this.assignedPartitions.size() > 0) { - this.consumer.pause(this.assignedPartitions); - this.paused = true; - } - else if (this.paused && !this.pausing) { - this.consumer.resume(this.assignedPartitions); - this.paused = false; + protected Object doReceive() { + this.lock.lock(); + try { + + if (this.stopped) { + this.logger.debug("Message source is stopped; no records will be returned"); + return null; + } + if (this.consumer == null) { + createConsumer(); + this.running = true; + } + if (this.pausing && !this.paused && this.assignedPartitions.size() > 0) { + this.consumer.pause(this.assignedPartitions); + this.paused = true; + } + else if (this.paused && !this.pausing) { + this.consumer.resume(this.assignedPartitions); + this.paused = false; + } + if (this.paused && this.recordsIterator == null) { + this.logger.debug("Consumer is paused; no records will be returned"); + } + ConsumerRecord record = pollRecord(); + + return record != null + ? recordToMessage(record) + : null; } - if (this.paused && this.recordsIterator == null) { - this.logger.debug("Consumer is paused; no records will be returned"); + finally { + this.lock.unlock(); } - ConsumerRecord record = pollRecord(); - - return record != null - ? recordToMessage(record) - : null; } protected void createConsumer() { - synchronized (this.consumerMonitor) { + this.consumerMonitor.lock(); + try { this.consumer = this.consumerFactory.createConsumer(this.consumerProperties.getGroupId(), this.consumerProperties.getClientId(), null, this.consumerProperties.getKafkaConsumerProperties()); @@ -466,6 +509,9 @@ else if (partitions != null) { rebalanceCallback); } } + finally { + this.consumerMonitor.unlock(); + } } private void assignAndSeekPartitions(TopicPartitionOffset[] partitions) { @@ -522,7 +568,8 @@ private ConsumerRecord pollRecord() { return nextRecord(); } else { - synchronized (this.consumerMonitor) { + this.consumerMonitor.lock(); + try { try { ConsumerRecords records = this.consumer .poll(this.assignedPartitions.isEmpty() ? this.assignTimeout : this.pollTimeout); @@ -545,6 +592,9 @@ private ConsumerRecord pollRecord() { return null; } } + finally { + this.consumerMonitor.unlock(); + } } } @@ -590,18 +640,28 @@ private Object recordToMessage(ConsumerRecord record) { } @Override - public synchronized void destroy() { - stopConsumer(); + public void destroy() { + this.lock.lock(); + try { + stopConsumer(); + } + finally { + this.lock.unlock(); + } } private void stopConsumer() { - synchronized (this.consumerMonitor) { + this.consumerMonitor.lock(); + try { if (this.consumer != null) { this.consumer.close(this.closeTimeout); this.consumer = null; this.assignedPartitions.clear(); } } + finally { + this.consumerMonitor.unlock(); + } } private class IntegrationConsumerRebalanceListener implements ConsumerRebalanceListener { diff --git a/spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/core/AbstractMqttClientManager.java b/spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/core/AbstractMqttClientManager.java index 2fd2e6b2060..0a5e2a9012a 100644 --- a/spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/core/AbstractMqttClientManager.java +++ b/spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/core/AbstractMqttClientManager.java @@ -19,6 +19,8 @@ import java.util.Collections; import java.util.HashSet; import java.util.Set; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -38,6 +40,7 @@ * * @author Artem Vozhdayenko * @author Artem Bilan + * @author Christian Tzolov * * @since 6.0 */ @@ -47,6 +50,8 @@ public abstract class AbstractMqttClientManager implements ClientManager connectCallbacks = Collections.synchronizedSet(new HashSet<>()); private final String clientId; @@ -92,8 +97,14 @@ protected ApplicationEventPublisher getApplicationEventPublisher() { return this.applicationEventPublisher; } - protected synchronized void setClient(T client) { - this.client = client; + protected void setClient(T client) { + this.lock.lock(); + try { + this.client = client; + } + finally { + this.lock.unlock(); + } } protected Set getCallbacks() { @@ -134,8 +145,14 @@ public boolean isManualAcks() { } @Override - public synchronized T getClient() { - return this.client; + public T getClient() { + this.lock.lock(); + try { + return this.client; + } + finally { + this.lock.unlock(); + } } @Override @@ -177,8 +194,14 @@ public boolean removeCallback(ConnectCallback connectCallback) { return this.connectCallbacks.remove(connectCallback); } - public synchronized boolean isRunning() { - return this.client != null; + public boolean isRunning() { + this.lock.lock(); + try { + return this.client != null; + } + finally { + this.lock.unlock(); + } } /** diff --git a/spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/core/Mqttv3ClientManager.java b/spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/core/Mqttv3ClientManager.java index b88416aa803..6a9a0249e14 100644 --- a/spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/core/Mqttv3ClientManager.java +++ b/spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/core/Mqttv3ClientManager.java @@ -36,6 +36,7 @@ * * @author Artem Vozhdayenko * @author Artem Bilan + * @author Christian Tzolov * * @since 6.0 */ @@ -87,40 +88,46 @@ public MqttConnectOptions getConnectionInfo() { } @Override - public synchronized void start() { - var client = getClient(); - if (client == null) { - try { - client = createClient(); - } - catch (MqttException e) { - throw new IllegalStateException("could not start client manager", e); - } - } - setClient(client); + public void start() { + this.lock.lock(); try { - client.connect(this.connectionOptions).waitForCompletion(getCompletionTimeout()); - } - catch (MqttException ex) { - // See GH-3822 - if (this.connectionOptions.isAutomaticReconnect()) { + var client = getClient(); + if (client == null) { try { - client.reconnect(); + client = createClient(); } - catch (MqttException re) { - logger.error("MQTT client failed to connect. Never happens.", re); + catch (MqttException e) { + throw new IllegalStateException("could not start client manager", e); } } - else { - var applicationEventPublisher = getApplicationEventPublisher(); - if (applicationEventPublisher != null) { - applicationEventPublisher.publishEvent(new MqttConnectionFailedEvent(this, ex)); + setClient(client); + try { + client.connect(this.connectionOptions).waitForCompletion(getCompletionTimeout()); + } + catch (MqttException ex) { + // See GH-3822 + if (this.connectionOptions.isAutomaticReconnect()) { + try { + client.reconnect(); + } + catch (MqttException re) { + logger.error("MQTT client failed to connect. Never happens.", re); + } } else { - logger.error("Could not start client manager, client_id=" + getClientId(), ex); + var applicationEventPublisher = getApplicationEventPublisher(); + if (applicationEventPublisher != null) { + applicationEventPublisher.publishEvent(new MqttConnectionFailedEvent(this, ex)); + } + else { + logger.error("Could not start client manager, client_id=" + getClientId(), ex); + } } } } + finally { + this.lock.unlock(); + } } private IMqttAsyncClient createClient() throws MqttException { @@ -133,31 +140,43 @@ private IMqttAsyncClient createClient() throws MqttException { } @Override - public synchronized void stop() { - var client = getClient(); - if (client == null) { - return; - } + public void stop() { + this.lock.lock(); try { - client.disconnectForcibly(getDisconnectCompletionTimeout()); - } - catch (MqttException e) { - logger.error("Could not disconnect from the client", e); - } - finally { + var client = getClient(); + if (client == null) { + return; + } try { - client.close(); + client.disconnectForcibly(getDisconnectCompletionTimeout()); } catch (MqttException e) { - logger.error("Could not close the client", e); + logger.error("Could not disconnect from the client", e); + } + finally { + try { + client.close(); + } + catch (MqttException e) { + logger.error("Could not close the client", e); + } + setClient(null); } - setClient(null); + } + finally { + this.lock.unlock(); } } @Override - public synchronized void connectionLost(Throwable cause) { - logger.error("Connection lost, client_id=" + getClientId(), cause); + public void connectionLost(Throwable cause) { + this.lock.lock(); + try { + logger.error("Connection lost, client_id=" + getClientId(), cause); + } + finally { + this.lock.unlock(); + } } @Override diff --git a/spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/core/Mqttv5ClientManager.java b/spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/core/Mqttv5ClientManager.java index 72ec62472d9..a89b34aa609 100644 --- a/spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/core/Mqttv5ClientManager.java +++ b/spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/core/Mqttv5ClientManager.java @@ -38,6 +38,7 @@ * * @author Artem Vozhdayenko * @author Artem Bilan + * @author Christian Tzolov * * @since 6.0 */ @@ -89,39 +90,45 @@ public MqttConnectionOptions getConnectionInfo() { } @Override - public synchronized void start() { - var client = getClient(); - if (client == null) { - try { - client = createClient(); - } - catch (MqttException e) { - throw new IllegalStateException("Could not start client manager", e); - } - } - setClient(client); + public void start() { + this.lock.lock(); try { - client.connect(this.connectionOptions).waitForCompletion(getCompletionTimeout()); - } - catch (MqttException ex) { - if (this.connectionOptions.isAutomaticReconnect()) { + var client = getClient(); + if (client == null) { try { - client.reconnect(); + client = createClient(); } - catch (MqttException re) { - logger.error("MQTT client failed to connect. Never happens.", re); + catch (MqttException e) { + throw new IllegalStateException("Could not start client manager", e); } } - else { - var applicationEventPublisher = getApplicationEventPublisher(); - if (applicationEventPublisher != null) { - applicationEventPublisher.publishEvent(new MqttConnectionFailedEvent(this, ex)); + setClient(client); + try { + client.connect(this.connectionOptions).waitForCompletion(getCompletionTimeout()); + } + catch (MqttException ex) { + if (this.connectionOptions.isAutomaticReconnect()) { + try { + client.reconnect(); + } + catch (MqttException re) { + logger.error("MQTT client failed to connect. Never happens.", re); + } } else { - logger.error("Could not start client manager, client_id=" + getClientId(), ex); + var applicationEventPublisher = getApplicationEventPublisher(); + if (applicationEventPublisher != null) { + applicationEventPublisher.publishEvent(new MqttConnectionFailedEvent(this, ex)); + } + else { + logger.error("Could not start client manager, client_id=" + getClientId(), ex); + } } } } + finally { + this.lock.unlock(); + } } private MqttAsyncClient createClient() throws MqttException { @@ -134,26 +141,32 @@ private MqttAsyncClient createClient() throws MqttException { } @Override - public synchronized void stop() { - var client = getClient(); - if (client == null) { - return; - } - + public void stop() { + this.lock.lock(); try { - client.disconnectForcibly(getDisconnectCompletionTimeout()); - } - catch (MqttException e) { - logger.error("Could not disconnect from the client", e); - } - finally { + var client = getClient(); + if (client == null) { + return; + } + try { - client.close(); + client.disconnectForcibly(getDisconnectCompletionTimeout()); } catch (MqttException e) { - logger.error("Could not close the client", e); + logger.error("Could not disconnect from the client", e); } - setClient(null); + finally { + try { + client.close(); + } + catch (MqttException e) { + logger.error("Could not close the client", e); + } + setClient(null); + } + } + finally { + this.lock.unlock(); } } diff --git a/spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/inbound/MqttPahoMessageDrivenChannelAdapter.java b/spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/inbound/MqttPahoMessageDrivenChannelAdapter.java index 26a8f29c89e..f28df29eb99 100644 --- a/spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/inbound/MqttPahoMessageDrivenChannelAdapter.java +++ b/spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/inbound/MqttPahoMessageDrivenChannelAdapter.java @@ -17,6 +17,8 @@ package org.springframework.integration.mqtt.inbound; import java.util.Arrays; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.stream.Stream; import org.eclipse.paho.client.mqttv3.IMqttAsyncClient; @@ -64,6 +66,8 @@ public class MqttPahoMessageDrivenChannelAdapter extends AbstractMqttMessageDrivenChannelAdapter implements MqttCallbackExtended, MqttPahoComponent { + private final Lock lock = new ReentrantLock(); + private final MqttPahoClientFactory clientFactory; private volatile IMqttAsyncClient client; @@ -179,46 +183,58 @@ protected void doStart() { } @SuppressWarnings("deprecation") - private synchronized void connect() throws MqttException { - MqttConnectOptions connectionOptions = this.clientFactory.getConnectionOptions(); - var clientManager = getClientManager(); - if (clientManager == null) { - Assert.state(getUrl() != null || connectionOptions.getServerURIs() != null, - "If no 'url' provided, connectionOptions.getServerURIs() must not be null"); - this.client = this.clientFactory.getAsyncClientInstance(getUrl(), getClientId()); - this.client.setCallback(this); - this.client.connect(connectionOptions).waitForCompletion(getCompletionTimeout()); - this.client.setManualAcks(isManualAcks()); + private void connect() throws MqttException { + this.lock.lock(); + try { + MqttConnectOptions connectionOptions = this.clientFactory.getConnectionOptions(); + var clientManager = getClientManager(); + if (clientManager == null) { + Assert.state(getUrl() != null || connectionOptions.getServerURIs() != null, + "If no 'url' provided, connectionOptions.getServerURIs() must not be null"); + this.client = this.clientFactory.getAsyncClientInstance(getUrl(), getClientId()); + this.client.setCallback(this); + this.client.connect(connectionOptions).waitForCompletion(getCompletionTimeout()); + this.client.setManualAcks(isManualAcks()); + } + else { + this.client = clientManager.getClient(); + } } - else { - this.client = clientManager.getClient(); + finally { + this.lock.unlock(); } } @Override - protected synchronized void doStop() { - this.readyToSubscribeOnStart = false; + protected void doStop() { + this.lock.lock(); try { - if (this.clientFactory.getConnectionOptions().isCleanSession()) { - this.client.unsubscribe(getTopic()); - // Have to re-subscribe on next start if connection is not lost. - this.readyToSubscribeOnStart = true; + this.readyToSubscribeOnStart = false; + try { + if (this.clientFactory.getConnectionOptions().isCleanSession()) { + this.client.unsubscribe(getTopic()); + // Have to re-subscribe on next start if connection is not lost. + this.readyToSubscribeOnStart = true; + } + } + catch (MqttException ex1) { + logger.error(ex1, "Exception while unsubscribing"); } - } - catch (MqttException ex1) { - logger.error(ex1, "Exception while unsubscribing"); - } - if (getClientManager() != null) { - return; - } + if (getClientManager() != null) { + return; + } - try { - this.client.disconnectForcibly(getDisconnectCompletionTimeout()); + try { + this.client.disconnectForcibly(getDisconnectCompletionTimeout()); + } + catch (MqttException ex) { + logger.error(ex, "Exception while disconnecting"); + } } - catch (MqttException ex) { - logger.error(ex, "Exception while disconnecting"); + finally { + this.lock.unlock(); } } @@ -322,17 +338,23 @@ private void warnInvalidQosForSubscription(String[] topics, int[] requestedQos, } @Override - public synchronized void connectionLost(Throwable cause) { - if (isRunning()) { - this.logger.error(() -> "Lost connection: " + cause.getMessage()); - ApplicationEventPublisher applicationEventPublisher = getApplicationEventPublisher(); - if (applicationEventPublisher != null) { - applicationEventPublisher.publishEvent(new MqttConnectionFailedEvent(this, cause)); + public void connectionLost(Throwable cause) { + this.lock.lock(); + try { + if (isRunning()) { + this.logger.error(() -> "Lost connection: " + cause.getMessage()); + ApplicationEventPublisher applicationEventPublisher = getApplicationEventPublisher(); + if (applicationEventPublisher != null) { + applicationEventPublisher.publishEvent(new MqttConnectionFailedEvent(this, cause)); + } + } + else { + // The 'connectComplete()' re-subscribes or sets this flag otherwise. + this.readyToSubscribeOnStart = false; } } - else { - // The 'connectComplete()' re-subscribes or sets this flag otherwise. - this.readyToSubscribeOnStart = false; + finally { + this.lock.unlock(); } } diff --git a/spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/inbound/Mqttv5PahoMessageDrivenChannelAdapter.java b/spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/inbound/Mqttv5PahoMessageDrivenChannelAdapter.java index cab526ddc5b..b589da35fc6 100644 --- a/spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/inbound/Mqttv5PahoMessageDrivenChannelAdapter.java +++ b/spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/inbound/Mqttv5PahoMessageDrivenChannelAdapter.java @@ -18,6 +18,8 @@ import java.util.Arrays; import java.util.Map; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.stream.IntStream; import org.eclipse.paho.mqttv5.client.IMqttAsyncClient; @@ -81,6 +83,8 @@ public class Mqttv5PahoMessageDrivenChannelAdapter extends AbstractMqttMessageDrivenChannelAdapter implements MqttCallback, MqttComponent { + private final Lock lock = new ReentrantLock(); + private final MqttConnectionOptions connectionOptions; private IMqttAsyncClient mqttClient; @@ -211,13 +215,19 @@ protected void doStart() { } } - private synchronized void connect() throws MqttException { - var clientManager = getClientManager(); - if (clientManager == null) { - this.mqttClient.connect(this.connectionOptions).waitForCompletion(getCompletionTimeout()); + private void connect() throws MqttException { + this.lock.lock(); + try { + var clientManager = getClientManager(); + if (clientManager == null) { + this.mqttClient.connect(this.connectionOptions).waitForCompletion(getCompletionTimeout()); + } + else { + this.mqttClient = clientManager.getClient(); + } } - else { - this.mqttClient = clientManager.getClient(); + finally { + this.lock.unlock(); } } diff --git a/spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/outbound/AbstractMqttMessageHandler.java b/spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/outbound/AbstractMqttMessageHandler.java index 387db0497fe..a17f7f63a5b 100644 --- a/spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/outbound/AbstractMqttMessageHandler.java +++ b/spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/outbound/AbstractMqttMessageHandler.java @@ -17,6 +17,8 @@ package org.springframework.integration.mqtt.outbound; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.springframework.beans.factory.BeanFactoryAware; import org.springframework.context.ApplicationEventPublisher; @@ -63,6 +65,8 @@ public abstract class AbstractMqttMessageHandler extends AbstractMessageHa private static final MessageProcessor DEFAULT_TOPIC_PROCESSOR = (message) -> message.getHeaders().get(MqttHeaders.TOPIC, String.class); + protected final Lock lock = new ReentrantLock(); + private final AtomicBoolean running = new AtomicBoolean(); private final String url; diff --git a/spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/outbound/MqttPahoMessageHandler.java b/spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/outbound/MqttPahoMessageHandler.java index a9d5f3ab7f2..317a9b9604a 100644 --- a/spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/outbound/MqttPahoMessageHandler.java +++ b/spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/outbound/MqttPahoMessageHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -50,6 +50,7 @@ * @author Gary Russell * @author Artem Bilan * @author Artem Vozhdayenko + * @author Christian Tzolov * * @since 4.0 * @@ -184,41 +185,47 @@ protected void doStop() { } } - private synchronized IMqttAsyncClient checkConnection() throws MqttException { - var theClientManager = getClientManager(); - if (theClientManager != null) { - return theClientManager.getClient(); - } + private IMqttAsyncClient checkConnection() throws MqttException { + this.lock.lock(); + try { + var theClientManager = getClientManager(); + if (theClientManager != null) { + return theClientManager.getClient(); + } - if (this.client != null && !this.client.isConnected()) { - this.client.setCallback(null); - this.client.close(); - this.client = null; - } - if (this.client == null) { - try { - MqttConnectOptions connectionOptions = this.clientFactory.getConnectionOptions(); - Assert.state(this.getUrl() != null || connectionOptions.getServerURIs() != null, - "If no 'url' provided, connectionOptions.getServerURIs() must not be null"); - this.client = this.clientFactory.getAsyncClientInstance(this.getUrl(), this.getClientId()); - incrementClientInstance(); - this.client.setCallback(this); - this.client.connect(connectionOptions).waitForCompletion(getCompletionTimeout()); - logger.debug("Client connected"); + if (this.client != null && !this.client.isConnected()) { + this.client.setCallback(null); + this.client.close(); + this.client = null; } - catch (MqttException e) { - if (this.client != null) { - this.client.close(); - this.client = null; + if (this.client == null) { + try { + MqttConnectOptions connectionOptions = this.clientFactory.getConnectionOptions(); + Assert.state(this.getUrl() != null || connectionOptions.getServerURIs() != null, + "If no 'url' provided, connectionOptions.getServerURIs() must not be null"); + this.client = this.clientFactory.getAsyncClientInstance(this.getUrl(), this.getClientId()); + incrementClientInstance(); + this.client.setCallback(this); + this.client.connect(connectionOptions).waitForCompletion(getCompletionTimeout()); + logger.debug("Client connected"); } - ApplicationEventPublisher applicationEventPublisher = getApplicationEventPublisher(); - if (applicationEventPublisher != null) { - applicationEventPublisher.publishEvent(new MqttConnectionFailedEvent(this, e)); + catch (MqttException e) { + if (this.client != null) { + this.client.close(); + this.client = null; + } + ApplicationEventPublisher applicationEventPublisher = getApplicationEventPublisher(); + if (applicationEventPublisher != null) { + applicationEventPublisher.publishEvent(new MqttConnectionFailedEvent(this, e)); + } + throw new MessagingException("Failed to connect", e); } - throw new MessagingException("Failed to connect", e); } + return this.client; + } + finally { + this.lock.unlock(); } - return this.client; } @Override @@ -252,22 +259,28 @@ private void sendDeliveryComplete(IMqttDeliveryToken token) { } @Override - public synchronized void connectionLost(Throwable cause) { - logger.error("Lost connection; will attempt reconnect on next request"); - if (this.client != null) { - try { - this.client.setCallback(null); - this.client.close(); - } - catch (MqttException e) { - // NOSONAR - } - this.client = null; - ApplicationEventPublisher applicationEventPublisher = getApplicationEventPublisher(); - if (applicationEventPublisher != null) { - applicationEventPublisher.publishEvent(new MqttConnectionFailedEvent(this, cause)); + public void connectionLost(Throwable cause) { + this.lock.lock(); + try { + logger.error("Lost connection; will attempt reconnect on next request"); + if (this.client != null) { + try { + this.client.setCallback(null); + this.client.close(); + } + catch (MqttException e) { + // NOSONAR + } + this.client = null; + ApplicationEventPublisher applicationEventPublisher = getApplicationEventPublisher(); + if (applicationEventPublisher != null) { + applicationEventPublisher.publishEvent(new MqttConnectionFailedEvent(this, cause)); + } } } + finally { + this.lock.unlock(); + } } @Override diff --git a/spring-integration-redis/src/main/java/org/springframework/integration/redis/util/RedisLockRegistry.java b/spring-integration-redis/src/main/java/org/springframework/integration/redis/util/RedisLockRegistry.java index 2f6e3ecddc6..828c18701d5 100644 --- a/spring-integration-redis/src/main/java/org/springframework/integration/redis/util/RedisLockRegistry.java +++ b/spring-integration-redis/src/main/java/org/springframework/integration/redis/util/RedisLockRegistry.java @@ -82,6 +82,7 @@ * @author Vedran Pavic * @author Unseok Kim * @author Anton Gabov + * @author Christian Tzolov * * @since 4.0 * @@ -94,6 +95,8 @@ public final class RedisLockRegistry implements ExpirableLockRegistry, Disposabl private static final int DEFAULT_CAPACITY = 100_000; + private final Lock lock = new ReentrantLock(); + private final Map locks = new LinkedHashMap(16, 0.75F, true) { @@ -224,15 +227,20 @@ public void setRedisLockType(RedisLockType redisLockType) { public Lock obtain(Object lockKey) { Assert.isInstanceOf(String.class, lockKey); String path = (String) lockKey; - synchronized (this.locks) { + this.lock.lock(); + try { return this.locks.computeIfAbsent(path, getRedisLockConstructor(this.redisLockType)); } + finally { + this.lock.unlock(); + } } @Override public void expireUnusedOlderThan(long age) { long now = System.currentTimeMillis(); - synchronized (this.locks) { + this.lock.lock(); + try { this.locks.entrySet() .removeIf(entry -> { RedisLock lock = entry.getValue(); @@ -243,6 +251,9 @@ public void expireUnusedOlderThan(long age) { && !lock.isAcquiredInThisProcess(); }); } + finally { + this.lock.unlock(); + } } @Override @@ -631,7 +642,8 @@ private boolean subscribeLock(long time) throws ExecutionException, InterruptedE } private void runRedisMessageListenerContainer() { - synchronized (RedisLockRegistry.this.locks) { + RedisLockRegistry.this.lock.tryLock(); + try { if (!(RedisLockRegistry.this.isRunningRedisMessageListenerContainer && RedisLockRegistry.this.redisMessageListenerContainer != null && RedisLockRegistry.this.redisMessageListenerContainer.isRunning())) { @@ -645,6 +657,9 @@ private void runRedisMessageListenerContainer() { RedisLockRegistry.this.isRunningRedisMessageListenerContainer = true; } } + finally { + RedisLockRegistry.this.lock.unlock(); + } } private static final class RedisUnLockNotifyMessageListener implements MessageListener { diff --git a/spring-integration-sftp/src/main/java/org/springframework/integration/sftp/session/DefaultSftpSessionFactory.java b/spring-integration-sftp/src/main/java/org/springframework/integration/sftp/session/DefaultSftpSessionFactory.java index b74c1312ab2..782223a87e8 100644 --- a/spring-integration-sftp/src/main/java/org/springframework/integration/sftp/session/DefaultSftpSessionFactory.java +++ b/spring-integration-sftp/src/main/java/org/springframework/integration/sftp/session/DefaultSftpSessionFactory.java @@ -62,11 +62,14 @@ * @author Artem Bilan * @author Krzysztof Debski * @author Auke Zaaiman + * @author Christian Tzolov * * @since 2.0 */ public class DefaultSftpSessionFactory implements SessionFactory, SharedSessionCapable { + private final Lock lock = new ReentrantLock(); + private final SshClient sshClient; private volatile boolean initialized; @@ -323,12 +326,16 @@ private ClientSession initClientSession() throws IOException { private void initClient() throws IOException { if (!this.initialized) { - synchronized (this) { + this.lock.lock(); + try { if (!this.initialized) { doInitClient(); this.initialized = true; } } + finally { + this.lock.unlock(); + } } } diff --git a/spring-integration-sftp/src/main/java/org/springframework/integration/sftp/session/SftpSession.java b/spring-integration-sftp/src/main/java/org/springframework/integration/sftp/session/SftpSession.java index 819d257b125..e26772c418e 100644 --- a/spring-integration-sftp/src/main/java/org/springframework/integration/sftp/session/SftpSession.java +++ b/spring-integration-sftp/src/main/java/org/springframework/integration/sftp/session/SftpSession.java @@ -22,6 +22,8 @@ import java.io.UncheckedIOException; import java.net.SocketAddress; import java.time.Duration; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.stream.Stream; import java.util.stream.StreamSupport; @@ -46,11 +48,14 @@ * @author Oleg Zhurakousky * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov * * @since 2.0 */ public class SftpSession implements Session { + private final Lock lock = new ReentrantLock(); + private final SftpClient sftpClient; public SftpSession(SftpClient sftpClient) { @@ -122,15 +127,20 @@ public boolean finalizeRaw() { @Override public void write(InputStream inputStream, String destination) throws IOException { - synchronized (this.sftpClient) { + this.lock.lock(); + try { OutputStream outputStream = this.sftpClient.write(destination); FileCopyUtils.copy(inputStream, outputStream); } + finally { + this.lock.unlock(); + } } @Override public void append(InputStream inputStream, String destination) throws IOException { - synchronized (this.sftpClient) { + this.lock.lock(); + try { OutputStream outputStream = this.sftpClient.write(destination, SftpClient.OpenMode.Create, @@ -138,6 +148,9 @@ public void append(InputStream inputStream, String destination) throws IOExcepti SftpClient.OpenMode.Append); FileCopyUtils.copy(inputStream, outputStream); } + finally { + this.lock.unlock(); + } } @Override diff --git a/spring-integration-smb/src/main/java/org/springframework/integration/smb/session/SmbShare.java b/spring-integration-smb/src/main/java/org/springframework/integration/smb/session/SmbShare.java index 3878f0223e5..e3cb4bb9d12 100644 --- a/spring-integration-smb/src/main/java/org/springframework/integration/smb/session/SmbShare.java +++ b/spring-integration-smb/src/main/java/org/springframework/integration/smb/session/SmbShare.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2022 the original author or authors. + * Copyright 2012-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,6 +19,8 @@ import java.io.IOException; import java.util.Properties; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import jcifs.CIFSContext; import jcifs.CIFSException; @@ -41,6 +43,7 @@ * @author Gregory Bragg * @author Adam Jones * @author Artem Bilan + * @author Christian Tzolov * * @since 6.0 */ @@ -48,6 +51,8 @@ public class SmbShare extends SmbFile { private static final Log logger = LogFactory.getLog(SmbShare.class); + private final Lock lock = new ReentrantLock(); + private final AtomicBoolean open = new AtomicBoolean(false); private final AtomicBoolean closeContext = new AtomicBoolean(false); @@ -126,17 +131,23 @@ boolean isOpened() { } @Override - public synchronized void close() { - this.open.set(false); - if (this.closeContext.get()) { - try { - getContext().close(); - } - catch (CIFSException e) { - logger.error("Unable to close share: " + this); + public void close() { + this.lock.lock(); + try { + this.open.set(false); + if (this.closeContext.get()) { + try { + getContext().close(); + } + catch (CIFSException e) { + logger.error("Unable to close share: " + this); + } } + super.close(); + } + finally { + this.lock.unlock(); } - super.close(); } /** diff --git a/spring-integration-stomp/src/main/java/org/springframework/integration/stomp/AbstractStompSessionManager.java b/spring-integration-stomp/src/main/java/org/springframework/integration/stomp/AbstractStompSessionManager.java index 7ef57952e6d..13ae624efcd 100644 --- a/spring-integration-stomp/src/main/java/org/springframework/integration/stomp/AbstractStompSessionManager.java +++ b/spring-integration-stomp/src/main/java/org/springframework/integration/stomp/AbstractStompSessionManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2015-2022 the original author or authors. + * Copyright 2015-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -25,6 +25,8 @@ import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.function.BiConsumer; import org.apache.commons.logging.Log; @@ -66,6 +68,7 @@ * * @author Artem Bilan * @author Gary Russell + * @author Christian Tzolov * * @since 4.2 */ @@ -80,7 +83,9 @@ public abstract class AbstractStompSessionManager implements StompSessionManager private final CompositeStompSessionHandler compositeStompSessionHandler = new CompositeStompSessionHandler(); - private final Object lifecycleMonitor = new Object(); + private final Lock lifecycleMonitor = new ReentrantLock(); + + private final Lock lock = new ReentrantLock(); private final AtomicInteger epoch = new AtomicInteger(); @@ -177,41 +182,47 @@ public int getPhase() { return this.phase; } - private synchronized void connect() { - if (this.connecting || this.connected) { - this.logger.debug("Aborting connect; another thread is connecting."); - return; - } - final int currentEpoch = this.epoch.get(); - this.connecting = true; - if (this.logger.isDebugEnabled()) { - this.logger.debug("Connecting " + this); - } + private void connect() { + this.lock.lock(); try { - this.stompSessionFuture = doConnect(this.compositeStompSessionHandler); - } - catch (Exception e) { - if (currentEpoch == this.epoch.get()) { - scheduleReconnect(e); + if (this.connecting || this.connected) { + this.logger.debug("Aborting connect; another thread is connecting."); + return; } - else { - this.logger.error("STOMP doConnect() error for " + this, e); + final int currentEpoch = this.epoch.get(); + this.connecting = true; + if (this.logger.isDebugEnabled()) { + this.logger.debug("Connecting " + this); } - return; - } - CountDownLatch connectLatch = addStompSessionCallback(currentEpoch); - - try { - if (!connectLatch.await(30, TimeUnit.SECONDS)) { // NOSONAR magic number - this.logger.error("No response to connection attempt"); + try { + this.stompSessionFuture = doConnect(this.compositeStompSessionHandler); + } + catch (Exception e) { if (currentEpoch == this.epoch.get()) { - scheduleReconnect(null); + scheduleReconnect(e); + } + else { + this.logger.error("STOMP doConnect() error for " + this, e); + } + return; + } + CountDownLatch connectLatch = addStompSessionCallback(currentEpoch); + + try { + if (!connectLatch.await(30, TimeUnit.SECONDS)) { // NOSONAR magic number + this.logger.error("No response to connection attempt"); + if (currentEpoch == this.epoch.get()) { + scheduleReconnect(null); + } } } + catch (InterruptedException e1) { + this.logger.error("Interrupted while waiting for connection attempt"); + Thread.currentThread().interrupt(); + } } - catch (InterruptedException e1) { - this.logger.error("Interrupted while waiting for connection attempt"); - Thread.currentThread().interrupt(); + finally { + this.lock.unlock(); } } @@ -294,7 +305,8 @@ public void accept(StompSession session, Throwable throwable) { @Override public void start() { - synchronized (this.lifecycleMonitor) { + this.lifecycleMonitor.lock(); + try { if (!isRunning()) { if (this.logger.isInfoEnabled()) { this.logger.info("Starting " + this); @@ -303,11 +315,15 @@ public void start() { this.running = true; } } + finally { + this.lifecycleMonitor.unlock(); + } } @Override public void stop() { - synchronized (this.lifecycleMonitor) { + this.lifecycleMonitor.lock(); + try { if (isRunning()) { this.running = false; if (this.logger.isInfoEnabled()) { @@ -316,6 +332,9 @@ public void stop() { destroy(); } } + finally { + this.lifecycleMonitor.unlock(); + } } @Override @@ -355,18 +374,24 @@ private class CompositeStompSessionHandler extends StompSessionHandlerAdapter { private final List delegates = Collections.synchronizedList(new ArrayList<>()); + private final Lock delegatesMonitor = new ReentrantLock(); + private volatile StompSession session; CompositeStompSessionHandler() { } void addHandler(StompSessionHandler delegate) { - synchronized (this.delegates) { + this.delegatesMonitor.lock(); + try { if (this.session != null) { delegate.afterConnected(this.session, getConnectHeaders()); } this.delegates.add(delegate); } + finally { + this.delegatesMonitor.unlock(); + } } void removeHandler(StompSessionHandler delegate) { @@ -375,23 +400,31 @@ void removeHandler(StompSessionHandler delegate) { @Override public void afterConnected(StompSession session, StompHeaders connectedHeaders) { - synchronized (this.delegates) { + this.delegatesMonitor.lock(); + try { this.session = session; for (StompSessionHandler delegate : this.delegates) { delegate.afterConnected(session, connectedHeaders); } } + finally { + this.delegatesMonitor.unlock(); + } } @Override public void handleException(StompSession session, @Nullable StompCommand command, StompHeaders headers, byte[] payload, Throwable exception) { - synchronized (this.delegates) { + this.delegatesMonitor.lock(); + try { for (StompSessionHandler delegate : this.delegates) { delegate.handleException(session, command, headers, payload, exception); } } + finally { + this.delegatesMonitor.unlock(); + } } @Override @@ -400,20 +433,28 @@ public void handleTransportError(StompSession session, Throwable exception) { exception); this.session = null; scheduleReconnect(exception); - synchronized (this.delegates) { + this.delegatesMonitor.lock(); + try { for (StompSessionHandler delegate : this.delegates) { delegate.handleTransportError(session, exception); } } + finally { + this.delegatesMonitor.unlock(); + } } @Override public void handleFrame(StompHeaders headers, Object payload) { - synchronized (this.delegates) { + this.delegatesMonitor.lock(); + try { for (StompSessionHandler delegate : this.delegates) { delegate.handleFrame(headers, payload); } } + finally { + this.delegatesMonitor.unlock(); + } } } diff --git a/spring-integration-stomp/src/main/java/org/springframework/integration/stomp/outbound/StompMessageHandler.java b/spring-integration-stomp/src/main/java/org/springframework/integration/stomp/outbound/StompMessageHandler.java index 7c93fa2de03..9f017d8926e 100644 --- a/spring-integration-stomp/src/main/java/org/springframework/integration/stomp/outbound/StompMessageHandler.java +++ b/spring-integration-stomp/src/main/java/org/springframework/integration/stomp/outbound/StompMessageHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2015-2020 the original author or authors. + * Copyright 2015-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,6 +18,8 @@ import java.util.concurrent.Semaphore; import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.ApplicationEventPublisherAware; @@ -51,6 +53,7 @@ * * @author Artem Bilan * @author Gary Russell + * @author Christian Tzolov * * @since 4.2 */ @@ -59,6 +62,8 @@ public class StompMessageHandler extends AbstractMessageHandler private static final int DEFAULT_CONNECT_TIMEOUT = 3000; + private final Lock lock = new ReentrantLock(); + private final StompSessionHandler sessionHandler = new IntegrationOutboundStompSessionHandler(); private final StompSessionManager stompSessionManager; @@ -178,7 +183,8 @@ protected void handleMessageInternal(final Message message) { } private void connectIfNecessary() throws InterruptedException { - synchronized (this.connectSemaphore) { + this.lock.lock(); + try { if (this.stompSession == null || !this.stompSessionManager.isConnected()) { this.stompSessionManager.disconnect(this.sessionHandler); this.stompSessionManager.connect(this.sessionHandler); @@ -199,6 +205,9 @@ private void connectIfNecessary() throws InterruptedException { } } } + finally { + this.lock.unlock(); + } } @Override diff --git a/spring-integration-stomp/src/test/java/org/springframework/integration/stomp/inbound/StompInboundChannelAdapterWebSocketIntegrationTests.java b/spring-integration-stomp/src/test/java/org/springframework/integration/stomp/inbound/StompInboundChannelAdapterWebSocketIntegrationTests.java index d10ca04b148..8d11caa1006 100644 --- a/spring-integration-stomp/src/test/java/org/springframework/integration/stomp/inbound/StompInboundChannelAdapterWebSocketIntegrationTests.java +++ b/spring-integration-stomp/src/test/java/org/springframework/integration/stomp/inbound/StompInboundChannelAdapterWebSocketIntegrationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2015-2022 the original author or authors. + * Copyright 2015-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -247,7 +247,8 @@ public WebSocketClient webSocketClient() { } @Bean - public WebSocketStompClient stompClient(TaskScheduler taskScheduler) { + public WebSocketStompClient stompClient( + @Qualifier("taskScheduler") TaskScheduler taskScheduler) { WebSocketStompClient webSocketStompClient = new WebSocketStompClient(webSocketClient()); webSocketStompClient.setMessageConverter(new MappingJackson2MessageConverter()); webSocketStompClient.setTaskScheduler(taskScheduler); @@ -347,6 +348,7 @@ public void configureMessageBroker(MessageBrokerRegistry configurer) { //SimpleBrokerMessageHandler doesn't support RECEIPT frame, hence we emulate it this way @Bean public ApplicationListener webSocketEventListener( + @Qualifier("clientOutboundChannel") final AbstractSubscribableChannel clientOutboundChannel) { return event -> { Message message = event.getMessage(); diff --git a/spring-integration-stream/src/main/java/org/springframework/integration/stream/ByteStreamReadingMessageSource.java b/spring-integration-stream/src/main/java/org/springframework/integration/stream/ByteStreamReadingMessageSource.java index b5a15f814eb..edeaef71475 100644 --- a/spring-integration-stream/src/main/java/org/springframework/integration/stream/ByteStreamReadingMessageSource.java +++ b/spring-integration-stream/src/main/java/org/springframework/integration/stream/ByteStreamReadingMessageSource.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,6 +19,8 @@ import java.io.BufferedInputStream; import java.io.IOException; import java.io.InputStream; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.springframework.integration.endpoint.AbstractMessageSource; import org.springframework.messaging.MessagingException; @@ -28,9 +30,12 @@ * * @author Mark Fisher * @author Artem Bilan + * @author Christian Tzolov */ public class ByteStreamReadingMessageSource extends AbstractMessageSource { + private final Lock lock = new ReentrantLock(); + private final BufferedInputStream stream; private int bytesPerMessage = 1024; // NOSONAR magic number @@ -73,13 +78,17 @@ protected byte[] doReceive() { try { byte[] bytes; int bytesRead = 0; - synchronized (this.stream) { + this.lock.lock(); + try { if (this.stream.available() == 0) { return null; } bytes = new byte[this.bytesPerMessage]; bytesRead = this.stream.read(bytes, 0, bytes.length); } + finally { + this.lock.unlock(); + } if (bytesRead <= 0) { return null; } diff --git a/spring-integration-stream/src/main/java/org/springframework/integration/stream/CharacterStreamReadingMessageSource.java b/spring-integration-stream/src/main/java/org/springframework/integration/stream/CharacterStreamReadingMessageSource.java index 450394047ff..b577d20f559 100644 --- a/spring-integration-stream/src/main/java/org/springframework/integration/stream/CharacterStreamReadingMessageSource.java +++ b/spring-integration-stream/src/main/java/org/springframework/integration/stream/CharacterStreamReadingMessageSource.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,6 +21,8 @@ import java.io.InputStreamReader; import java.io.Reader; import java.io.UnsupportedEncodingException; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.ApplicationEventPublisherAware; @@ -34,10 +36,13 @@ * @author Mark Fisher * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov */ public class CharacterStreamReadingMessageSource extends AbstractMessageSource implements ApplicationEventPublisherAware { + private final Lock lock = new ReentrantLock(); + private final BufferedReader reader; private final boolean blockToDetectEOF; @@ -112,7 +117,8 @@ public String getComponentType() { @Override public String doReceive() { try { - synchronized (this.reader) { + this.lock.lock(); + try { if (!this.blockToDetectEOF && !this.reader.ready()) { return null; } @@ -122,6 +128,9 @@ public String doReceive() { } return line; } + finally { + this.lock.unlock(); + } } catch (IOException e) { throw new MessagingException("IO failure occurred in adapter", e); diff --git a/spring-integration-test/src/main/java/org/springframework/integration/test/mock/MockMessageHandler.java b/spring-integration-test/src/main/java/org/springframework/integration/test/mock/MockMessageHandler.java index 646d97c72b6..ab2a5d931b6 100644 --- a/spring-integration-test/src/main/java/org/springframework/integration/test/mock/MockMessageHandler.java +++ b/spring-integration-test/src/main/java/org/springframework/integration/test/mock/MockMessageHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2017-2019 the original author or authors. + * Copyright 2017-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,6 +19,8 @@ import java.util.Iterator; import java.util.LinkedList; import java.util.List; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.function.Consumer; import java.util.function.Function; @@ -51,11 +53,14 @@ * * * @author Artem Bilan + * @author Christian Tzolov * * @since 5.0 */ public class MockMessageHandler extends AbstractMessageProducingHandler { + private final Lock lock = new ReentrantLock(); + protected final List, ?>> messageFunctions = new LinkedList<>(); // NOSONAR final private final CapturingMatcher> capturingMatcher; @@ -110,13 +115,17 @@ protected void handleMessageInternal(Message message) { Function, ?> function = this.lastFunction; - synchronized (this) { + this.lock.lock(); + try { Iterator, ?>> iterator = this.messageFunctions.iterator(); if (iterator.hasNext()) { function = iterator.next(); iterator.remove(); } } + finally { + this.lock.unlock(); + } Object result = function.apply(message); diff --git a/spring-integration-websocket/src/main/java/org/springframework/integration/websocket/ClientWebSocketContainer.java b/spring-integration-websocket/src/main/java/org/springframework/integration/websocket/ClientWebSocketContainer.java index ac7f9d3b4ac..b89db27cc31 100644 --- a/spring-integration-websocket/src/main/java/org/springframework/integration/websocket/ClientWebSocketContainer.java +++ b/spring-integration-websocket/src/main/java/org/springframework/integration/websocket/ClientWebSocketContainer.java @@ -46,6 +46,7 @@ * * @author Artem Bilan * @author Gary Russell + * @author Christian Tzolov * * @since 4.1 */ @@ -183,12 +184,18 @@ public boolean isRunning() { } @Override - public synchronized void start() { - if (!isRunning()) { - this.clientSession = null; - this.openConnectionException = null; - this.connectionLatch = new CountDownLatch(1); - this.connectionManager.start(); + public void start() { + this.lock.lock(); + try { + if (!isRunning()) { + this.clientSession = null; + this.openConnectionException = null; + this.connectionLatch = new CountDownLatch(1); + this.connectionManager.start(); + } + } + finally { + this.lock.unlock(); } } diff --git a/spring-integration-websocket/src/main/java/org/springframework/integration/websocket/IntegrationWebSocketContainer.java b/spring-integration-websocket/src/main/java/org/springframework/integration/websocket/IntegrationWebSocketContainer.java index fab87e2fd2e..dfe8d67e9c9 100644 --- a/spring-integration-websocket/src/main/java/org/springframework/integration/websocket/IntegrationWebSocketContainer.java +++ b/spring-integration-websocket/src/main/java/org/springframework/integration/websocket/IntegrationWebSocketContainer.java @@ -21,6 +21,8 @@ import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -67,6 +69,8 @@ public abstract class IntegrationWebSocketContainer implements DisposableBean { protected final Log logger = LogFactory.getLog(getClass()); // NOSONAR + protected final Lock lock = new ReentrantLock(); + private WebSocketHandler webSocketHandler = new IntegrationWebSocketHandler(); protected final Map sessions = new ConcurrentHashMap<>(); // NOSONAR diff --git a/spring-integration-websocket/src/main/java/org/springframework/integration/websocket/ServerWebSocketContainer.java b/spring-integration-websocket/src/main/java/org/springframework/integration/websocket/ServerWebSocketContainer.java index 792e36b4207..fb103b8aa36 100644 --- a/spring-integration-websocket/src/main/java/org/springframework/integration/websocket/ServerWebSocketContainer.java +++ b/spring-integration-websocket/src/main/java/org/springframework/integration/websocket/ServerWebSocketContainer.java @@ -47,6 +47,7 @@ * * @author Artem Bilan * @author Gary Russell + * @author Christian Tzolov * * @since 4.1 */ @@ -224,9 +225,15 @@ public boolean isRunning() { } @Override - public synchronized void start() { - if (this.handshakeHandler instanceof Lifecycle && !isRunning()) { - ((Lifecycle) this.handshakeHandler).start(); + public void start() { + this.lock.lock(); + try { + if (this.handshakeHandler instanceof Lifecycle && !isRunning()) { + ((Lifecycle) this.handshakeHandler).start(); + } + } + finally { + this.lock.unlock(); } } diff --git a/spring-integration-ws/src/main/java/org/springframework/integration/ws/AbstractWebServiceOutboundGateway.java b/spring-integration-ws/src/main/java/org/springframework/integration/ws/AbstractWebServiceOutboundGateway.java index 53fc14ea801..50f8338b9e7 100644 --- a/spring-integration-ws/src/main/java/org/springframework/integration/ws/AbstractWebServiceOutboundGateway.java +++ b/spring-integration-ws/src/main/java/org/springframework/integration/ws/AbstractWebServiceOutboundGateway.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,6 +20,8 @@ import java.net.URI; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import javax.xml.transform.TransformerException; @@ -55,9 +57,12 @@ * @author Oleg Zhurakousky * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov */ public abstract class AbstractWebServiceOutboundGateway extends AbstractReplyProducingMessageHandler { + private final Lock lock = new ReentrantLock(); + protected final DefaultUriBuilderFactory uriFactory = new DefaultUriBuilderFactory(); // NOSONAR - final private final String uri; @@ -108,10 +113,14 @@ public void setHeaderMapper(SoapHeaderMapper headerMapper) { * @param uriVariableExpressions The URI variable expressions. */ public void setUriVariableExpressions(Map uriVariableExpressions) { - synchronized (this.uriVariableExpressions) { + this.lock.lock(); + try { this.uriVariableExpressions.clear(); this.uriVariableExpressions.putAll(uriVariableExpressions); } + finally { + this.lock.unlock(); + } } /** diff --git a/spring-integration-xml/src/main/java/org/springframework/integration/xml/DefaultXmlPayloadConverter.java b/spring-integration-xml/src/main/java/org/springframework/integration/xml/DefaultXmlPayloadConverter.java index 3b7ea3853b8..be48e562bdb 100644 --- a/spring-integration-xml/src/main/java/org/springframework/integration/xml/DefaultXmlPayloadConverter.java +++ b/spring-integration-xml/src/main/java/org/springframework/integration/xml/DefaultXmlPayloadConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,6 +20,8 @@ import java.io.File; import java.io.InputStream; import java.io.StringReader; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; @@ -43,11 +45,13 @@ * * @author Jonas Partner * @author Artem Bilan + * @author Christian Tzolov */ public class DefaultXmlPayloadConverter implements XmlPayloadConverter { private final DocumentBuilderFactory documentBuilderFactory; + private final Lock lock = new ReentrantLock(); public DefaultXmlPayloadConverter() { this(DocumentBuilderFactoryUtils.newInstance()); @@ -142,13 +146,17 @@ else if (object instanceof Document) { } } - protected synchronized DocumentBuilder getDocumentBuilder() { + protected DocumentBuilder getDocumentBuilder() { + this.lock.lock(); try { return this.documentBuilderFactory.newDocumentBuilder(); } catch (ParserConfigurationException e) { throw new MessagingException("failed to create a new DocumentBuilder", e); } + finally { + this.lock.unlock(); + } } } diff --git a/spring-integration-xml/src/main/java/org/springframework/integration/xml/result/DomResultFactory.java b/spring-integration-xml/src/main/java/org/springframework/integration/xml/result/DomResultFactory.java index 76bd1ac3224..4ee51dab409 100644 --- a/spring-integration-xml/src/main/java/org/springframework/integration/xml/result/DomResultFactory.java +++ b/spring-integration-xml/src/main/java/org/springframework/integration/xml/result/DomResultFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,9 @@ package org.springframework.integration.xml.result; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; + import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; @@ -30,11 +33,16 @@ * @author Jonas Partner * @author Artem Bilan * @author Gary Russell + * @author Christian Tzolov */ public class DomResultFactory implements ResultFactory { private final DocumentBuilderFactory documentBuilderFactory; + private final Lock documentBuilderFactoryMonitor = new ReentrantLock(); + + private final Lock lock = new ReentrantLock(); + public DomResultFactory() { this(DocumentBuilderFactoryUtils.newInstance()); @@ -48,7 +56,8 @@ public DomResultFactory(DocumentBuilderFactory documentBuilderFactory) { @Override - public synchronized Result createResult(Object payload) { + public Result createResult(Object payload) { + this.lock.lock(); try { return new DOMResult(getNewDocumentBuilder().newDocument()); } @@ -56,12 +65,19 @@ public synchronized Result createResult(Object payload) { throw new MessagingException("failed to create Result for payload type [" + payload.getClass().getName() + "]", e); } + finally { + this.lock.unlock(); + } } protected DocumentBuilder getNewDocumentBuilder() throws ParserConfigurationException { - synchronized (this.documentBuilderFactory) { + this.documentBuilderFactoryMonitor.lock(); + try { return this.documentBuilderFactory.newDocumentBuilder(); } + finally { + this.documentBuilderFactoryMonitor.unlock(); + } } } diff --git a/spring-integration-xml/src/main/java/org/springframework/integration/xml/source/DomSourceFactory.java b/spring-integration-xml/src/main/java/org/springframework/integration/xml/source/DomSourceFactory.java index ab2593ce47a..4714e1bbfc2 100644 --- a/spring-integration-xml/src/main/java/org/springframework/integration/xml/source/DomSourceFactory.java +++ b/spring-integration-xml/src/main/java/org/springframework/integration/xml/source/DomSourceFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,6 +18,8 @@ import java.io.File; import java.io.StringReader; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; @@ -39,9 +41,12 @@ * @author Jonas Partner * @author Mark Fisher * @author Artem Bilan + * @author Christian Tzolov */ public class DomSourceFactory implements SourceFactory { + private final Lock lock = new ReentrantLock(); + private final DocumentBuilderFactory documentBuilderFactory; @@ -99,9 +104,13 @@ private DOMSource createDomSourceForFile(File file) { } private DocumentBuilder getNewDocumentBuilder() throws ParserConfigurationException { - synchronized (this.documentBuilderFactory) { + this.lock.lock(); + try { return this.documentBuilderFactory.newDocumentBuilder(); } + finally { + this.lock.unlock(); + } } } diff --git a/spring-integration-xml/src/main/java/org/springframework/integration/xml/source/StringSourceFactory.java b/spring-integration-xml/src/main/java/org/springframework/integration/xml/source/StringSourceFactory.java index 16599180d02..b1317fdc1ba 100644 --- a/spring-integration-xml/src/main/java/org/springframework/integration/xml/source/StringSourceFactory.java +++ b/spring-integration-xml/src/main/java/org/springframework/integration/xml/source/StringSourceFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,6 +18,8 @@ import java.io.File; import java.io.FileReader; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import javax.xml.transform.Source; import javax.xml.transform.Transformer; @@ -40,9 +42,12 @@ * @author Jonas Partner * @author Mark Fisher * @author Artem Bilan + * @author Christian Tzolov */ public class StringSourceFactory implements SourceFactory { + private final Lock lock = new ReentrantLock(); + private final TransformerFactory transformerFactory; @@ -96,13 +101,17 @@ private StringSource createStringSourceForFile(File file) { } } - private synchronized Transformer getTransformer() { + private Transformer getTransformer() { + this.lock.lock(); try { return this.transformerFactory.newTransformer(); } catch (Exception e) { throw new MessagingException("Exception creating transformer", e); } + finally { + this.lock.unlock(); + } } } diff --git a/spring-integration-xml/src/main/java/org/springframework/integration/xml/splitter/XPathMessageSplitter.java b/spring-integration-xml/src/main/java/org/springframework/integration/xml/splitter/XPathMessageSplitter.java index 3d615024fa3..13eb3653882 100644 --- a/spring-integration-xml/src/main/java/org/springframework/integration/xml/splitter/XPathMessageSplitter.java +++ b/spring-integration-xml/src/main/java/org/springframework/integration/xml/splitter/XPathMessageSplitter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,6 +22,8 @@ import java.util.List; import java.util.Map; import java.util.Properties; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.function.Function; import javax.xml.parsers.DocumentBuilder; @@ -67,12 +69,15 @@ * @author Mark Fisher * @author Artem Bilan * @author Gary Russell + * @author Christian Tzolov */ public class XPathMessageSplitter extends AbstractMessageSplitter { private final TransformerFactory transformerFactory; - private final Object documentBuilderFactoryMonitor = new Object(); + private final Lock documentBuilderFactoryMonitor = new ReentrantLock(); + + private final Lock transformerFactoryMonitor = new ReentrantLock(); private final XPathExpression xpathExpression; @@ -249,9 +254,13 @@ protected int obtainSizeIfPossible(Iterator iterator) { private Object splitDocument(Document document) throws ParserConfigurationException, TransformerException { Object nodes = splitNode(document); final Transformer transformer; - synchronized (this.transformerFactory) { + this.transformerFactoryMonitor.lock(); + try { transformer = this.transformerFactory.newTransformer(); } + finally { + this.transformerFactoryMonitor.unlock(); + } if (this.outputProperties != null) { transformer.setOutputProperties(this.outputProperties); } @@ -317,9 +326,13 @@ private Document convertNodeToDocument(DocumentBuilder documentBuilder, Node nod } private DocumentBuilder getNewDocumentBuilder() throws ParserConfigurationException { - synchronized (this.documentBuilderFactoryMonitor) { + this.documentBuilderFactoryMonitor.lock(); + try { return this.documentBuilderFactory.newDocumentBuilder(); } + finally { + this.documentBuilderFactoryMonitor.unlock(); + } } private final class NodeListIterator implements Iterator { diff --git a/spring-integration-xml/src/main/java/org/springframework/integration/xml/transformer/ResultToDocumentTransformer.java b/spring-integration-xml/src/main/java/org/springframework/integration/xml/transformer/ResultToDocumentTransformer.java index 9cdbc40a1c6..10a77624bb0 100644 --- a/spring-integration-xml/src/main/java/org/springframework/integration/xml/transformer/ResultToDocumentTransformer.java +++ b/spring-integration-xml/src/main/java/org/springframework/integration/xml/transformer/ResultToDocumentTransformer.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,6 +17,8 @@ package org.springframework.integration.xml.transformer; import java.io.StringReader; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; @@ -38,9 +40,12 @@ * * @author Jonas Partner * @author Artem Bilan + * @author Christian Tzolov */ public class ResultToDocumentTransformer implements ResultTransformer { + private final Lock lock = new ReentrantLock(); + // Not guaranteed to be thread safe private final DocumentBuilderFactory documentBuilderFactory; @@ -84,13 +89,17 @@ private Document createDocumentFromStringResult(StringResult stringResult) { } } - private synchronized DocumentBuilder getDocumentBuilder() { + private DocumentBuilder getDocumentBuilder() { + this.lock.lock(); try { return this.documentBuilderFactory.newDocumentBuilder(); } catch (ParserConfigurationException e) { throw new MessagingException("failed to create a new DocumentBuilder", e); } + finally { + this.lock.unlock(); + } } } diff --git a/spring-integration-xml/src/main/java/org/springframework/integration/xml/transformer/ResultToStringTransformer.java b/spring-integration-xml/src/main/java/org/springframework/integration/xml/transformer/ResultToStringTransformer.java index ee966a2cc97..cffdd198969 100644 --- a/spring-integration-xml/src/main/java/org/springframework/integration/xml/transformer/ResultToStringTransformer.java +++ b/spring-integration-xml/src/main/java/org/springframework/integration/xml/transformer/ResultToStringTransformer.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,6 +17,8 @@ package org.springframework.integration.xml.transformer; import java.util.Properties; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import javax.xml.transform.Result; import javax.xml.transform.Transformer; @@ -38,9 +40,12 @@ * @author Jonas Partner * @author Mark Fisher * @author Artem Bilan + * @author Christian Tzolov */ public class ResultToStringTransformer implements ResultTransformer { + private final Lock lock = new ReentrantLock(); + private final TransformerFactory transformerFactory; private Properties outputProperties; @@ -90,9 +95,13 @@ else if (result instanceof DOMResult) { private Transformer getNewTransformer() throws TransformerConfigurationException { Transformer transformer; - synchronized (this.transformerFactory) { + this.lock.lock(); + try { transformer = this.transformerFactory.newTransformer(); } + finally { + this.lock.unlock(); + } if (this.outputProperties != null) { transformer.setOutputProperties(this.outputProperties); } diff --git a/spring-integration-xmpp/src/main/java/org/springframework/integration/xmpp/config/XmppConnectionFactoryBean.java b/spring-integration-xmpp/src/main/java/org/springframework/integration/xmpp/config/XmppConnectionFactoryBean.java index a0a9c4ca9ba..833bfc7162a 100644 --- a/spring-integration-xmpp/src/main/java/org/springframework/integration/xmpp/config/XmppConnectionFactoryBean.java +++ b/spring-integration-xmpp/src/main/java/org/springframework/integration/xmpp/config/XmppConnectionFactoryBean.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,9 @@ package org.springframework.integration.xmpp.config; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; + import org.jivesoftware.smack.ConnectionListener; import org.jivesoftware.smack.XMPPConnection; import org.jivesoftware.smack.roster.Roster; @@ -40,6 +43,7 @@ * @author Artem Bilan * @author Philipp Etschel * @author Gary Russell + * @author Christian Tzolov * * @since 2.0 * @@ -47,7 +51,7 @@ */ public class XmppConnectionFactoryBean extends AbstractFactoryBean implements SmartLifecycle { - private final Object lifecycleMonitor = new Object(); + private final Lock lifecycleMonitor = new ReentrantLock(); private XMPPTCPConnectionConfiguration connectionConfiguration; @@ -172,7 +176,8 @@ protected XMPPTCPConnection getConnection() { @Override public void start() { - synchronized (this.lifecycleMonitor) { + this.lifecycleMonitor.lock(); + try { if (this.running) { return; } @@ -195,16 +200,23 @@ public void start() { + connection.getXMPPServiceDomain(), e); } } + finally { + this.lifecycleMonitor.unlock(); + } } @Override public void stop() { - synchronized (this.lifecycleMonitor) { + this.lifecycleMonitor.lock(); + try { if (this.isRunning()) { getConnection().disconnect(); this.running = false; } } + finally { + this.lifecycleMonitor.unlock(); + } } @Override diff --git a/spring-integration-zeromq/src/main/java/org/springframework/integration/zeromq/ZeroMqProxy.java b/spring-integration-zeromq/src/main/java/org/springframework/integration/zeromq/ZeroMqProxy.java index 681f4fd0a86..2ed9aea6fdf 100644 --- a/spring-integration-zeromq/src/main/java/org/springframework/integration/zeromq/ZeroMqProxy.java +++ b/spring-integration-zeromq/src/main/java/org/springframework/integration/zeromq/ZeroMqProxy.java @@ -20,6 +20,8 @@ import java.util.concurrent.Executors; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.function.Consumer; import org.apache.commons.logging.Log; @@ -58,6 +60,7 @@ * The address for this socket is {@code "inproc://" + beanName + ".capture"}. * * @author Artem Bilan + * @author Christian Tzolov * * @since 5.4 * @@ -67,6 +70,8 @@ public class ZeroMqProxy implements InitializingBean, SmartLifecycle, BeanNameAw private static final Log LOG = LogFactory.getLog(ZeroMqProxy.class); + private final Lock lock = new ReentrantLock(); + private final ZContext context; private final Type type; @@ -247,65 +252,78 @@ public void afterPropertiesSet() { } @Override - public synchronized void start() { - if (!this.running.get()) { - this.proxyExecutor - .execute(() -> { - ZMQ.Socket captureSocket = null; - if (this.exposeCaptureSocket) { - captureSocket = this.context.createSocket(SocketType.PUB); - } - try ( - ZMQ.Socket frontendSocket = this.context.createSocket(this.type.getFrontendSocketType()); - ZMQ.Socket backendSocket = this.context.createSocket(this.type.getBackendSocketType()); - ZMQ.Socket controlSocket = this.context.createSocket(SocketType.PAIR) - ) { - - if (this.frontendSocketConfigurer != null) { - this.frontendSocketConfigurer.accept(frontendSocket); + public void start() { + this.lock.lock(); + try { + if (!this.running.get()) { + this.proxyExecutor + .execute(() -> { + ZMQ.Socket captureSocket = null; + if (this.exposeCaptureSocket) { + captureSocket = this.context.createSocket(SocketType.PUB); } + try ( + ZMQ.Socket frontendSocket = this.context + .createSocket(this.type.getFrontendSocketType()); + ZMQ.Socket backendSocket = this.context + .createSocket(this.type.getBackendSocketType()); + ZMQ.Socket controlSocket = this.context.createSocket(SocketType.PAIR)) { + + if (this.frontendSocketConfigurer != null) { + this.frontendSocketConfigurer.accept(frontendSocket); + } - if (this.backendSocketConfigurer != null) { - this.backendSocketConfigurer.accept(backendSocket); - } + if (this.backendSocketConfigurer != null) { + this.backendSocketConfigurer.accept(backendSocket); + } - this.frontendPort.set(bindSocket(frontendSocket, this.frontendPort.get())); // NOSONAR - this.backendPort.set(bindSocket(backendSocket, this.backendPort.get())); // NOSONAR - boolean bound = controlSocket.bind(this.controlAddress); // NOSONAR - if (!bound) { - throw new IllegalArgumentException("Cannot bind ZeroMQ socket to address: " - + this.controlAddress); - } - if (captureSocket != null) { - bound = captureSocket.bind(this.captureAddress); + this.frontendPort.set(bindSocket(frontendSocket, this.frontendPort.get())); // NOSONAR + this.backendPort.set(bindSocket(backendSocket, this.backendPort.get())); // NOSONAR + boolean bound = controlSocket.bind(this.controlAddress); // NOSONAR if (!bound) { throw new IllegalArgumentException("Cannot bind ZeroMQ socket to address: " - + this.captureAddress); + + this.controlAddress); + } + if (captureSocket != null) { + bound = captureSocket.bind(this.captureAddress); + if (!bound) { + throw new IllegalArgumentException("Cannot bind ZeroMQ socket to address: " + + this.captureAddress); + } } + this.running.set(true); + ZMQ.proxy(frontendSocket, backendSocket, captureSocket, controlSocket); } - this.running.set(true); - ZMQ.proxy(frontendSocket, backendSocket, captureSocket, controlSocket); - } - catch (Exception ex) { // NOSONAR - LOG.error("Cannot start ZeroMQ proxy from bean: " + this.beanName, ex); - } - finally { - if (captureSocket != null) { - captureSocket.close(); + catch (Exception ex) { // NOSONAR + LOG.error("Cannot start ZeroMQ proxy from bean: " + this.beanName, ex); } - } - }); + finally { + if (captureSocket != null) { + captureSocket.close(); + } + } + }); + } + } + finally { + this.lock.unlock(); } } @Override - public synchronized void stop() { - if (this.running.getAndSet(false)) { - try (ZMQ.Socket commandSocket = this.context.createSocket(SocketType.PAIR)) { - commandSocket.connect(this.controlAddress); // NOSONAR - commandSocket.send(zmq.ZMQ.PROXY_TERMINATE); + public void stop() { + this.lock.lock(); + try { + if (this.running.getAndSet(false)) { + try (ZMQ.Socket commandSocket = this.context.createSocket(SocketType.PAIR)) { + commandSocket.connect(this.controlAddress); // NOSONAR + commandSocket.send(zmq.ZMQ.PROXY_TERMINATE); + } } } + finally { + this.lock.unlock(); + } } @Override diff --git a/spring-integration-zookeeper/src/main/java/org/springframework/integration/zookeeper/config/CuratorFrameworkFactoryBean.java b/spring-integration-zookeeper/src/main/java/org/springframework/integration/zookeeper/config/CuratorFrameworkFactoryBean.java index 933ecf69584..4fca3c612ea 100644 --- a/spring-integration-zookeeper/src/main/java/org/springframework/integration/zookeeper/config/CuratorFrameworkFactoryBean.java +++ b/spring-integration-zookeeper/src/main/java/org/springframework/integration/zookeeper/config/CuratorFrameworkFactoryBean.java @@ -1,5 +1,5 @@ /* - * Copyright 2015-2019 the original author or authors. + * Copyright 2015-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,9 @@ package org.springframework.integration.zookeeper.config; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; + import org.apache.curator.RetryPolicy; import org.apache.curator.framework.CuratorFramework; import org.apache.curator.framework.CuratorFrameworkFactory; @@ -31,12 +34,13 @@ * * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov * * @since 4.2 */ public class CuratorFrameworkFactoryBean implements FactoryBean, SmartLifecycle { - private final Object lifecycleLock = new Object(); + private final Lock lifecycleLock = new ReentrantLock(); private final CuratorFramework client; @@ -109,7 +113,8 @@ public void setAutoStartup(boolean autoStartup) { @Override public void start() { - synchronized (this.lifecycleLock) { + this.lifecycleLock.lock(); + try { if (!this.running) { if (this.client != null) { this.client.start(); @@ -117,16 +122,23 @@ public void start() { this.running = true; } } + finally { + this.lifecycleLock.unlock(); + } } @Override public void stop() { - synchronized (this.lifecycleLock) { + this.lifecycleLock.lock(); + try { if (this.running) { CloseableUtils.closeQuietly(this.client); this.running = false; } } + finally { + this.lifecycleLock.unlock(); + } } @Override diff --git a/spring-integration-zookeeper/src/main/java/org/springframework/integration/zookeeper/config/LeaderInitiatorFactoryBean.java b/spring-integration-zookeeper/src/main/java/org/springframework/integration/zookeeper/config/LeaderInitiatorFactoryBean.java index 1a098a878b0..c78efb7c3a2 100644 --- a/spring-integration-zookeeper/src/main/java/org/springframework/integration/zookeeper/config/LeaderInitiatorFactoryBean.java +++ b/spring-integration-zookeeper/src/main/java/org/springframework/integration/zookeeper/config/LeaderInitiatorFactoryBean.java @@ -1,5 +1,5 @@ /* - * Copyright 2015-2020 the original author or authors. + * Copyright 2015-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -37,6 +37,7 @@ * * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov * * @since 4.2 */ @@ -177,7 +178,7 @@ else if (this.applicationEventPublisher != null) { } @Override - public synchronized LeaderInitiator getObject() { + public LeaderInitiator getObject() { return this.leaderInitiator; } diff --git a/spring-integration-zookeeper/src/main/java/org/springframework/integration/zookeeper/leader/LeaderInitiator.java b/spring-integration-zookeeper/src/main/java/org/springframework/integration/zookeeper/leader/LeaderInitiator.java index 8203d525b26..b1ae7b11065 100644 --- a/spring-integration-zookeeper/src/main/java/org/springframework/integration/zookeeper/leader/LeaderInitiator.java +++ b/spring-integration-zookeeper/src/main/java/org/springframework/integration/zookeeper/leader/LeaderInitiator.java @@ -18,6 +18,8 @@ import java.util.Collection; import java.util.List; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -43,6 +45,7 @@ * @author Gary Russell * @author Artem Bilan * @author Ivan Zaitsev + * @author Christian Tzolov * * @since 4.2 */ @@ -66,7 +69,7 @@ public class LeaderInitiator implements SmartLifecycle { */ private final Candidate candidate; - private final Object lifecycleMonitor = new Object(); + private final Lock lifecycleMonitor = new ReentrantLock(); /** * Base path in a zookeeper @@ -159,7 +162,8 @@ public void setAutoStartup(boolean autoStartup) { */ @Override public void start() { - synchronized (this.lifecycleMonitor) { + this.lifecycleMonitor.lock(); + try { if (!this.running) { if (this.client.getState() != CuratorFrameworkState.STARTED) { // we want to do curator start here because it needs to @@ -177,6 +181,9 @@ public void start() { LOGGER.debug("Started LeaderInitiator"); } } + finally { + this.lifecycleMonitor.unlock(); + } } /** @@ -185,13 +192,17 @@ public void start() { */ @Override public void stop() { - synchronized (this.lifecycleMonitor) { + this.lifecycleMonitor.lock(); + try { if (this.running) { this.leaderSelector.close(); this.running = false; LOGGER.debug("Stopped LeaderInitiator"); } } + finally { + this.lifecycleMonitor.unlock(); + } } /** diff --git a/spring-integration-zookeeper/src/main/java/org/springframework/integration/zookeeper/lock/ZookeeperLockRegistry.java b/spring-integration-zookeeper/src/main/java/org/springframework/integration/zookeeper/lock/ZookeeperLockRegistry.java index d7afaba8512..fece7d598f0 100644 --- a/spring-integration-zookeeper/src/main/java/org/springframework/integration/zookeeper/lock/ZookeeperLockRegistry.java +++ b/spring-integration-zookeeper/src/main/java/org/springframework/integration/zookeeper/lock/ZookeeperLockRegistry.java @@ -1,5 +1,5 @@ /* - * Copyright 2015-2021 the original author or authors. + * Copyright 2015-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -24,6 +24,7 @@ import java.util.concurrent.TimeoutException; import java.util.concurrent.locks.Condition; import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.apache.curator.framework.CuratorFramework; import org.apache.curator.framework.recipes.locks.InterProcessMutex; @@ -44,6 +45,7 @@ * @author Artem Bilan * @author Vedran Pavic * @author Unseok Kim + * @author Christian Tzolov * * @since 4.2 * @@ -58,6 +60,8 @@ public class ZookeeperLockRegistry implements ExpirableLockRegistry, DisposableB private static final int DEFAULT_CAPACITY = 30_000; + private final Lock locksLock = new ReentrantLock(); + private final Map locks = new LinkedHashMap(16, 0.75F, true) { @@ -145,9 +149,13 @@ public Lock obtain(Object lockKey) { Assert.isInstanceOf(String.class, lockKey); String path = this.keyToPath.pathFor((String) lockKey); ZkLock lock; - synchronized (this.locks) { + this.locksLock.lock(); + try { lock = this.locks.computeIfAbsent(path, p -> new ZkLock(this.client, this.mutexTaskExecutor, p)); } + finally { + this.locksLock.unlock(); + } if (this.trackingTime) { lock.setLastUsed(System.currentTimeMillis()); } @@ -155,10 +163,9 @@ public Lock obtain(Object lockKey) { } /** - * Remove locks last acquired more than 'age' ago that are not currently locked. - * Expiry is not supported if the {@link KeyToPathStrategy} is bounded (returns a finite - * number of paths). With such a {@link KeyToPathStrategy}, the overhead of tracking when - * a lock is obtained is avoided. + * Remove locks last acquired more than 'age' ago that are not currently locked. Expiry is not supported if the + * {@link KeyToPathStrategy} is bounded (returns a finite number of paths). With such a {@link KeyToPathStrategy}, + * the overhead of tracking when a lock is obtained is avoided. * @param age the time since the lock was last obtained. */ @Override @@ -168,13 +175,18 @@ public void expireUnusedOlderThan(long age) { } long now = System.currentTimeMillis(); - synchronized (this.locks) { + this.locksLock.lock(); + try { this.locks.entrySet() .removeIf(entry -> { ZkLock lock = entry.getValue(); return now - lock.getLastUsed() > age && !lock.isAcquiredInThisProcess(); }); } + finally { + this.locksLock.unlock(); + } + } @Override diff --git a/spring-integration-zookeeper/src/main/java/org/springframework/integration/zookeeper/metadata/ZookeeperMetadataStore.java b/spring-integration-zookeeper/src/main/java/org/springframework/integration/zookeeper/metadata/ZookeeperMetadataStore.java index 10cdbbb5e95..5d944acdfd3 100644 --- a/spring-integration-zookeeper/src/main/java/org/springframework/integration/zookeeper/metadata/ZookeeperMetadataStore.java +++ b/spring-integration-zookeeper/src/main/java/org/springframework/integration/zookeeper/metadata/ZookeeperMetadataStore.java @@ -1,5 +1,5 @@ /* - * Copyright 2015-2022 the original author or authors. + * Copyright 2015-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,6 +21,8 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.apache.curator.framework.CuratorFramework; import org.apache.curator.framework.recipes.cache.ChildData; @@ -43,6 +45,7 @@ * @author Marius Bogoevici * @author Gary Russell * @author Artem Bilan + * @author Christian Tzolov * * @since 4.2 */ @@ -72,6 +75,8 @@ public class ZookeeperMetadataStore implements ListenableMetadataStore, SmartLif private boolean running; + private final Lock lock = new ReentrantLock(); + public ZookeeperMetadataStore(CuratorFramework client) { Assert.notNull(client, "Client cannot be null"); this.client = client; // NOSONAR @@ -110,42 +115,54 @@ public void setPhase(int phase) { } @Override - public synchronized String putIfAbsent(String key, String value) { - Assert.notNull(key, KEY_MUST_NOT_BE_NULL); - Assert.notNull(value, "'value' must not be null."); + public String putIfAbsent(String key, String value) { + this.lock.lock(); try { - createNode(key, value); - return null; - } - catch (KeeperException.NodeExistsException ex) { - // so the data actually exists, we can read it - return get(key); + Assert.notNull(key, KEY_MUST_NOT_BE_NULL); + Assert.notNull(value, "'value' must not be null."); + try { + createNode(key, value); + return null; + } + catch (KeeperException.NodeExistsException ex) { + // so the data actually exists, we can read it + return get(key); + } + catch (Exception ex) { + throw new ZookeeperMetadataStoreException("Error while trying to set '" + key + "':", ex); + } } - catch (Exception ex) { - throw new ZookeeperMetadataStoreException("Error while trying to set '" + key + "':", ex); + finally { + this.lock.unlock(); } } @Override - public synchronized boolean replace(String key, String oldValue, String newValue) { - Assert.notNull(key, KEY_MUST_NOT_BE_NULL); - Assert.notNull(oldValue, "'oldValue' must not be null."); - Assert.notNull(newValue, "'newValue' must not be null."); - Stat currentStat = new Stat(); + public boolean replace(String key, String oldValue, String newValue) { + this.lock.lock(); try { - byte[] bytes = this.client.getData().storingStatIn(currentStat).forPath(getPath(key)); - if (oldValue.equals(IntegrationUtils.bytesToString(bytes, this.encoding))) { - updateNode(key, newValue, currentStat.getVersion()); + Assert.notNull(key, KEY_MUST_NOT_BE_NULL); + Assert.notNull(oldValue, "'oldValue' must not be null."); + Assert.notNull(newValue, "'newValue' must not be null."); + Stat currentStat = new Stat(); + try { + byte[] bytes = this.client.getData().storingStatIn(currentStat).forPath(getPath(key)); + if (oldValue.equals(IntegrationUtils.bytesToString(bytes, this.encoding))) { + updateNode(key, newValue, currentStat.getVersion()); + } + return true; + } + catch (KeeperException.NoNodeException | KeeperException.BadVersionException ex) { + // ignore, the node doesn't exist there's nothing to replace + return false; + } + // ignore + catch (Exception ex) { + throw new ZookeeperMetadataStoreException("Cannot replace value", ex); } - return true; - } - catch (KeeperException.NoNodeException | KeeperException.BadVersionException ex) { - // ignore, the node doesn't exist there's nothing to replace - return false; } - // ignore - catch (Exception ex) { - throw new ZookeeperMetadataStoreException("Cannot replace value", ex); + finally { + this.lock.unlock(); } } @@ -161,71 +178,89 @@ public void removeListener(MetadataStoreListener callback) { } @Override - public synchronized void put(String key, String value) { - Assert.notNull(key, KEY_MUST_NOT_BE_NULL); - Assert.notNull(value, "'value' must not be null."); + public void put(String key, String value) { + this.lock.lock(); try { - Stat currentNode = this.client.checkExists().forPath(getPath(key)); - if (currentNode == null) { - try { - createNode(key, value); + Assert.notNull(key, KEY_MUST_NOT_BE_NULL); + Assert.notNull(value, "'value' must not be null."); + try { + Stat currentNode = this.client.checkExists().forPath(getPath(key)); + if (currentNode == null) { + try { + createNode(key, value); + } + catch (KeeperException.NodeExistsException e) { + updateNode(key, value, -1); + } } - catch (KeeperException.NodeExistsException e) { + else { updateNode(key, value, -1); } } - else { - updateNode(key, value, -1); + catch (Exception ex) { + throw new ZookeeperMetadataStoreException("Error while setting value for key '" + key + "':", ex); } } - catch (Exception ex) { - throw new ZookeeperMetadataStoreException("Error while setting value for key '" + key + "':", ex); + finally { + this.lock.unlock(); } } @Override - public synchronized String get(String key) { - Assert.notNull(key, KEY_MUST_NOT_BE_NULL); - Assert.state(isRunning(), "ZookeeperMetadataStore has to be started before using."); - return this.cache.get(getPath(key)) - .map(currentData -> { - // our version is more recent than the cache - if (this.updateMap.containsKey(key) && - this.updateMap.get(key).version() >= currentData.getStat().getVersion()) { - - return this.updateMap.get(key).value(); - } - return IntegrationUtils.bytesToString(currentData.getData(), this.encoding); // NOSONAR - }) - .orElseGet(() -> { - if (this.updateMap.containsKey(key)) { - // we have saved the value, but the cache hasn't updated yet - // if the value had changed via replication, we would have been notified by the listener - return this.updateMap.get(key).value(); - } - else { - // the value just doesn't exist - return null; - } - }); + public String get(String key) { + this.lock.lock(); + try { + Assert.notNull(key, KEY_MUST_NOT_BE_NULL); + Assert.state(isRunning(), "ZookeeperMetadataStore has to be started before using."); + return this.cache.get(getPath(key)) + .map(currentData -> { + // our version is more recent than the cache + if (this.updateMap.containsKey(key) && + this.updateMap.get(key).version() >= currentData.getStat().getVersion()) { + + return this.updateMap.get(key).value(); + } + return IntegrationUtils.bytesToString(currentData.getData(), this.encoding); // NOSONAR + }) + .orElseGet(() -> { + if (this.updateMap.containsKey(key)) { + // we have saved the value, but the cache hasn't updated yet + // if the value had changed via replication, we would have been notified by the listener + return this.updateMap.get(key).value(); + } + else { + // the value just doesn't exist + return null; + } + }); + } + finally { + this.lock.unlock(); + } } @Override - public synchronized String remove(String key) { - Assert.notNull(key, KEY_MUST_NOT_BE_NULL); + public String remove(String key) { + this.lock.lock(); try { - byte[] bytes = this.client.getData().forPath(getPath(key)); - this.client.delete().forPath(getPath(key)); - // we guarantee that the deletion will supersede the existing data - this.updateMap.put(key, new LocalChildData(null, Integer.MAX_VALUE)); - return IntegrationUtils.bytesToString(bytes, this.encoding); - } - catch (KeeperException.NoNodeException ex) { - // ignore - the node doesn't exist - return null; + Assert.notNull(key, KEY_MUST_NOT_BE_NULL); + try { + byte[] bytes = this.client.getData().forPath(getPath(key)); + this.client.delete().forPath(getPath(key)); + // we guarantee that the deletion will supersede the existing data + this.updateMap.put(key, new LocalChildData(null, Integer.MAX_VALUE)); + return IntegrationUtils.bytesToString(bytes, this.encoding); + } + catch (KeeperException.NoNodeException ex) { + // ignore - the node doesn't exist + return null; + } + catch (Exception ex) { + throw new ZookeeperMetadataStoreException("Exception while deleting key '" + key + "'", ex); + } } - catch (Exception ex) { - throw new ZookeeperMetadataStoreException("Exception while deleting key '" + key + "'", ex); + finally { + this.lock.unlock(); } } @@ -250,35 +285,54 @@ public boolean isAutoStartup() { } @Override - public synchronized void start() { - if (!this.running) { - try { - this.client.createContainers(this.root); - this.cache = CuratorCache.builder(this.client, this.root).build(); - this.cache.listenable().addListener(new MetadataStoreCacheListener()); - this.cache.start(); - this.running = true; - } - catch (Exception ex) { - throw new ZookeeperMetadataStoreException("Exception while starting bean", ex); + public void start() { + this.lock.lock(); + try { + if (!this.running) { + try { + this.client.createContainers(this.root); + this.cache = CuratorCache.builder(this.client, this.root).build(); + this.cache.listenable().addListener(new MetadataStoreCacheListener()); + this.cache.start(); + this.running = true; + } + catch (Exception ex) { + throw new ZookeeperMetadataStoreException("Exception while starting bean", ex); + } } + + } + finally { + this.lock.unlock(); } } @Override - public synchronized void stop() { - if (this.running) { - if (this.cache != null) { - CloseableUtils.closeQuietly(this.cache); + public void stop() { + this.lock.lock(); + try { + if (this.running) { + if (this.cache != null) { + CloseableUtils.closeQuietly(this.cache); + } + this.cache = null; + this.running = false; } - this.cache = null; - this.running = false; + } + finally { + this.lock.unlock(); } } @Override - public synchronized boolean isRunning() { - return this.running; + public boolean isRunning() { + this.lock.lock(); + try { + return this.running; + } + finally { + this.lock.unlock(); + } } @Override