[aries-rsa] branch master updated: ARIES-1941 Addressed synchronization inconsistency in (using) MultiMap (#31)

[cs...@apache.org]

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

cschneider pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/aries-rsa.git


The following commit(s) were added to refs/heads/master by this push:
     new f14ba9d  ARIES-1941 Addressed synchronization inconsistency in (using) MultiMap (#31)
f14ba9d is described below

commit f14ba9d7a13eedbb5a9195e87a6e8f9417de3369
Author: Arnoud Glimmerveen <ar...@glimmerveen.org>
AuthorDate: Sat Nov 9 23:33:34 2019 +0100

    ARIES-1941 Addressed synchronization inconsistency in (using) MultiMap (#31)
    
    * ARIES-1941 Addressed synchronization inconsistency in (using) MultiMap
    
    Changed the approach to MultiMap to have all public method thread-safe: using synchronized keyword *and* returning read-only *copies* when a Set is to be returned
    Changed the one case where a defensive copy was made. Note that this old approach was not thread-safe, as the copy-constructors' iterator could be 'tripped' by a concurrent modification.
    
    * With the change to regular HashMap, a defensive copy of keySet is needed.
---
 .../aries/rsa/topologymanager/importer/MultiMap.java  | 19 +++++++++++--------
 .../importer/TopologyManagerImport.java               |  2 +-
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/topology-manager/src/main/java/org/apache/aries/rsa/topologymanager/importer/MultiMap.java b/topology-manager/src/main/java/org/apache/aries/rsa/topologymanager/importer/MultiMap.java
index eac5f40..b0f8dbe 100644
--- a/topology-manager/src/main/java/org/apache/aries/rsa/topologymanager/importer/MultiMap.java
+++ b/topology-manager/src/main/java/org/apache/aries/rsa/topologymanager/importer/MultiMap.java
@@ -19,10 +19,10 @@
 package org.apache.aries.rsa.topologymanager.importer;
 
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
-import java.util.Map;
+import java.util.Map;;
 import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
 
 /**
  * Minimal implementation of a synchronized map
@@ -32,7 +32,7 @@ public class MultiMap<T> {
     private Map<String, Set<T>> map;
     
     public MultiMap() {
-        map = new ConcurrentHashMap<>();
+        map = new HashMap<>();
     }
     
     public synchronized void put(String key, T value) {
@@ -45,7 +45,11 @@ public class MultiMap<T> {
     }
     
     public synchronized Set<T> get(String key) {
-        return map.getOrDefault(key, Collections.<T>emptySet());
+        if (map.containsKey(key)) {
+            return Collections.unmodifiableSet(new HashSet<>(map.get(key)));
+        } else {
+            return Collections.<T>emptySet();
+        }
     }
 
     public synchronized void remove(String key, T value) {
@@ -57,12 +61,11 @@ public class MultiMap<T> {
     }
 
     public synchronized Set<String> keySet() {
-        return map.keySet();
+        return Collections.unmodifiableSet(new HashSet<>(map.keySet()));
     }
 
-    public void remove(T toRemove) {
-        Set<String> keys = new HashSet<>(map.keySet());
-        for (String key : keys) {
+    public synchronized void remove(T toRemove) {
+        for (String key : map.keySet()) {
             remove(key, toRemove);
         }
     }
diff --git a/topology-manager/src/main/java/org/apache/aries/rsa/topologymanager/importer/TopologyManagerImport.java b/topology-manager/src/main/java/org/apache/aries/rsa/topologymanager/importer/TopologyManagerImport.java
index cbe8d5e..43c0bd5 100644
--- a/topology-manager/src/main/java/org/apache/aries/rsa/topologymanager/importer/TopologyManagerImport.java
+++ b/topology-manager/src/main/java/org/apache/aries/rsa/topologymanager/importer/TopologyManagerImport.java
@@ -176,7 +176,7 @@ public class TopologyManagerImport implements EndpointEventListener, RemoteServi
     }
 
     private void unImportForGoneEndpoints(String filter) {
-        Set<ImportRegistration> importRegistrations = new HashSet<>(importedServices.get(filter));
+        Set<ImportRegistration> importRegistrations = importedServices.get(filter);
         Set<EndpointDescription> endpoints = importPossibilities.get(filter);
         for (ImportRegistration ir : importRegistrations) {
             EndpointDescription endpoint = ir.getImportReference().getImportedEndpoint();