[activemq] 03/03: AMQ-8012 - Improve thread safety of SubQueueSelectorCacheBroker

[cs...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

cshannon pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/activemq.git

commit 62f5576fe5dc8bc06eefc1e26b8e66a10781fe32
Author: Christopher L. Shannon (cshannon) <ch...@gmail.com>
AuthorDate: Fri Jul 24 14:15:57 2020 -0400

    AMQ-8012 - Improve thread safety of SubQueueSelectorCacheBroker
    
    When returning a copy of the set of selectors we need to synchronize
---
 .../plugin/SubQueueSelectorCacheBroker.java         | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/activemq-broker/src/main/java/org/apache/activemq/plugin/SubQueueSelectorCacheBroker.java b/activemq-broker/src/main/java/org/apache/activemq/plugin/SubQueueSelectorCacheBroker.java
index dbd9ad7..0e48830 100644
--- a/activemq-broker/src/main/java/org/apache/activemq/plugin/SubQueueSelectorCacheBroker.java
+++ b/activemq-broker/src/main/java/org/apache/activemq/plugin/SubQueueSelectorCacheBroker.java
@@ -69,7 +69,7 @@ public class SubQueueSelectorCacheBroker extends BrokerFilter implements Runnabl
      * The subscription's selector cache. We cache compiled expressions keyed
      * by the target destination.
      */
-    private ConcurrentMap<String, Set<String>> subSelectorCache = new ConcurrentHashMap<String, Set<String>>();
+    private ConcurrentMap<String, Set<String>> subSelectorCache = new ConcurrentHashMap<>();
 
     private final File persistFile;
     private boolean singleSelectorPerDestination = false;
@@ -275,8 +275,11 @@ public class SubQueueSelectorCacheBroker extends BrokerFilter implements Runnabl
 
     @SuppressWarnings("unchecked")
     public Set<String> getSelectorsForDestination(String destinationName) {
-        if (subSelectorCache.containsKey(destinationName)) {
-            return new HashSet<String>(subSelectorCache.get(destinationName));
+        final Set<String> cachedSelectors = subSelectorCache.get(destinationName);
+        synchronized(cachedSelectors) {
+            if (cachedSelectors != null) {
+                return new HashSet<>(cachedSelectors);
+            }
         }
 
         return Collections.EMPTY_SET;
@@ -291,17 +294,13 @@ public class SubQueueSelectorCacheBroker extends BrokerFilter implements Runnabl
     }
 
     public boolean deleteSelectorForDestination(String destinationName, String selector) {
-        if (subSelectorCache.containsKey(destinationName)) {
-            Set<String> cachedSelectors = subSelectorCache.get(destinationName);
-            return cachedSelectors.remove(selector);
-        }
-
-        return false;
+        final Set<String> cachedSelectors = subSelectorCache.get(destinationName);
+        return cachedSelectors != null ? cachedSelectors.remove(selector) : false;
     }
 
     public boolean deleteAllSelectorsForDestination(String destinationName) {
-        if (subSelectorCache.containsKey(destinationName)) {
-            Set<String> cachedSelectors = subSelectorCache.get(destinationName);
+        final Set<String> cachedSelectors = subSelectorCache.get(destinationName);
+        if (cachedSelectors != null) {
             cachedSelectors.clear();
         }
         return true;