From c810aaae12c3e364b4e01845160866f4a3b8b332 Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Tue, 8 Dec 2015 17:38:49 -0500 Subject: [PATCH] INT-3897: Fix deadlock in the StompMessageHandler JIRA: https://jira.spring.io/browse/INT-3897 The `StompMessageHandler` synchronized on the `sessionHandler` during `connectIfNecessary()` performing mutually exclusive connect operation in the `StompSessionManager`. Since the last one synchronizes on all `sessionHandler`s during its own operations, we end up with the deadlock waiting for the `connectSemaphore`. Change synchronization monitor object to the `connectSemaphore` accomplishing the same desired mutually exclusive connect logic in the `StompMessageHandler`. **Cherry-pick to 4.2.x** --- .../stomp/AbstractStompSessionManager.java | 11 +++++------ .../stomp/outbound/StompMessageHandler.java | 4 ++-- 2 files changed, 7 insertions(+), 8 deletions(-) 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 d72fb18367..d62bfe92fd 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 @@ -60,7 +60,10 @@ import org.springframework.util.concurrent.ListenableFutureCallback; * to the provided {@link StompSessionHandler}s. * This {@link AbstractStompSessionManager.CompositeStompSessionHandler} is used for the * {@link StompSession} connection. + * * @author Artem Bilan + * @author Gary Russell + * * @since 4.2 */ public abstract class AbstractStompSessionManager implements StompSessionManager, ApplicationEventPublisherAware, @@ -363,15 +366,11 @@ public abstract class AbstractStompSessionManager implements StompSessionManager if (this.session != null) { delegate.afterConnected(this.session, getConnectHeaders()); } - synchronized (this.delegates) { - this.delegates.add(delegate); - } + this.delegates.add(delegate); } void removeHandler(StompSessionHandler delegate) { - synchronized (this.delegates) { - this.delegates.remove(delegate); - } + this.delegates.remove(delegate); } @Override 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 38e737fc2f..2aa16ec130 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 @@ -181,7 +181,7 @@ public class StompMessageHandler extends AbstractMessageHandler implements Appli } private StompSession connectIfNecessary() throws Exception { - synchronized (this.sessionHandler) { + synchronized (this.connectSemaphore) { if (this.stompSession == null || !this.stompSessionManager.isConnected()) { this.stompSessionManager.disconnect(this.sessionHandler); this.stompSessionManager.connect(this.sessionHandler); @@ -259,7 +259,7 @@ public class StompMessageHandler extends AbstractMessageHandler implements Appli } @Override - public synchronized void handleTransportError(StompSession session, Throwable exception) { + public void handleTransportError(StompSession session, Throwable exception) { StompMessageHandler.this.transportError = exception; StompMessageHandler.this.stompSession = null; }