From 4fbf6a3572b818cdc9269a3619165b01cfe2b7fb Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Tue, 7 Jul 2015 11:19:07 +0200 Subject: [PATCH] DefaultSubscriptionRegistry defensively checks for removal between keySet and get calls Issue: SPR-13205 --- .../broker/DefaultSubscriptionRegistry.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/spring-messaging/src/main/java/org/springframework/messaging/simp/broker/DefaultSubscriptionRegistry.java b/spring-messaging/src/main/java/org/springframework/messaging/simp/broker/DefaultSubscriptionRegistry.java index ebc1b783738..38035c602d6 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/simp/broker/DefaultSubscriptionRegistry.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/simp/broker/DefaultSubscriptionRegistry.java @@ -39,6 +39,7 @@ import org.springframework.util.PathMatcher; * * @author Rossen Stoyanchev * @author Sebastien Deleuze + * @author Juergen Hoeller * @since 4.0 */ public class DefaultSubscriptionRegistry extends AbstractSubscriptionRegistry { @@ -258,7 +259,6 @@ public class DefaultSubscriptionRegistry extends AbstractSubscriptionRegistry { private final ConcurrentMap sessions = new ConcurrentHashMap(); - public SessionSubscriptionInfo getSubscriptions(String sessionId) { return this.sessions.get(sessionId); } @@ -301,8 +301,6 @@ public class DefaultSubscriptionRegistry extends AbstractSubscriptionRegistry { // destination -> subscriptionIds private final Map> subscriptions = new ConcurrentHashMap>(4); - private final Object monitor = new Object(); - public SessionSubscriptionInfo(String sessionId) { Assert.notNull(sessionId, "sessionId must not be null"); this.sessionId = sessionId; @@ -323,7 +321,7 @@ public class DefaultSubscriptionRegistry extends AbstractSubscriptionRegistry { public void addSubscription(String destination, String subscriptionId) { Set subs = this.subscriptions.get(destination); if (subs == null) { - synchronized (this.monitor) { + synchronized (this.subscriptions) { subs = this.subscriptions.get(destination); if (subs == null) { subs = new CopyOnWriteArraySet(); @@ -337,13 +335,15 @@ public class DefaultSubscriptionRegistry extends AbstractSubscriptionRegistry { public String removeSubscription(String subscriptionId) { for (String destination : this.subscriptions.keySet()) { Set subscriptionIds = this.subscriptions.get(destination); - if (subscriptionIds.remove(subscriptionId)) { - synchronized (this.monitor) { - if (subscriptionIds.isEmpty()) { - this.subscriptions.remove(destination); + if (subscriptionIds != null) { + if (subscriptionIds.remove(subscriptionId)) { + synchronized (this.subscriptions) { + if (subscriptionIds.isEmpty()) { + this.subscriptions.remove(destination); + } } + return destination; } - return destination; } } return null;