[digitalstrom] fix concurrency issue (#9834)

* fix concurrency issue
* address review comments
* address review comment

Signed-off-by: Jan N. Klug <jan.n.klug@rub.de>

diff --git a/bundles/org.openhab.binding.digitalstrom/src/main/java/org/openhab/binding/digitalstrom/internal/DigitalSTROMHandlerFactory.java b/bundles/org.openhab.binding.digitalstrom/src/main/java/org/openhab/binding/digitalstrom/internal/DigitalSTROMHandlerFactory.java
index 37bf0214eb0290b54aa1c99d4c67ae0ab6b6ca92..6fa388fe73856043f4892c62a9ea4fdb776b6950 100644 (file)
--- a/bundles/org.openhab.binding.digitalstrom/src/main/java/org/openhab/binding/digitalstrom/internal/DigitalSTROMHandlerFactory.java
+++ b/bundles/org.openhab.binding.digitalstrom/src/main/java/org/openhab/binding/digitalstrom/internal/DigitalSTROMHandlerFactory.java
@@ -16,6 +16,7 @@ import static org.openhab.binding.digitalstrom.internal.DigitalSTROMBindingConst
 
 import java.util.HashMap;
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 
 import org.apache.commons.lang.StringUtils;
 import org.openhab.binding.digitalstrom.internal.discovery.DiscoveryServiceManager;
@@ -50,7 +51,7 @@ import org.slf4j.LoggerFactory;
 public class DigitalSTROMHandlerFactory extends BaseThingHandlerFactory {
 
     private final Logger logger = LoggerFactory.getLogger(DigitalSTROMHandlerFactory.class);
-    private final Map<String, DiscoveryServiceManager> discoveryServiceManagers = new HashMap<>();
+    private final Map<String, DiscoveryServiceManager> discoveryServiceManagers = new ConcurrentHashMap<>();
 
     private Map<ThingUID, BridgeHandler> bridgeHandlers;
 
@@ -256,9 +257,9 @@ public class DigitalSTROMHandlerFactory extends BaseThingHandlerFactory {
     protected synchronized void removeHandler(ThingHandler thingHandler) {
         if (thingHandler instanceof BridgeHandler) {
             String uid = thingHandler.getThing().getUID().getAsString();
-            if (discoveryServiceManagers.get(uid) != null) {
-                discoveryServiceManagers.get(uid).unregisterDiscoveryServices(bundleContext);
-                discoveryServiceManagers.remove(uid);
+            DiscoveryServiceManager discoveryServiceManager = discoveryServiceManagers.remove(uid);
+            if (discoveryServiceManager != null) {
+                discoveryServiceManager.unregisterDiscoveryServices(bundleContext);
             }
         }
     }

diff --git a/bundles/org.openhab.binding.digitalstrom/src/main/java/org/openhab/binding/digitalstrom/internal/lib/manager/impl/DeviceStatusManagerImpl.java b/bundles/org.openhab.binding.digitalstrom/src/main/java/org/openhab/binding/digitalstrom/internal/lib/manager/impl/DeviceStatusManagerImpl.java
index e40ca8fc879361537f837deb96cefc8d74115a74..cf6989436f5bfe8d08a5dde5ee50c939bbe9cfb5 100644 (file)
--- a/bundles/org.openhab.binding.digitalstrom/src/main/java/org/openhab/binding/digitalstrom/internal/lib/manager/impl/DeviceStatusManagerImpl.java
+++ b/bundles/org.openhab.binding.digitalstrom/src/main/java/org/openhab/binding/digitalstrom/internal/lib/manager/impl/DeviceStatusManagerImpl.java
@@ -19,6 +19,7 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
@@ -113,7 +114,7 @@ public class DeviceStatusManagerImpl implements DeviceStatusManager {
     private SceneReadingJobExecutor sceneJobExecutor;
     private EventListener eventListener;
 
-    private final List<TrashDevice> trashDevices = new LinkedList<>();
+    private final List<TrashDevice> trashDevices = new CopyOnWriteArrayList<>();
 
     private long lastBinCheck = 0;
     private ManagerStates state = ManagerStates.STOPPED;
@@ -310,7 +311,6 @@ public class DeviceStatusManagerImpl implements DeviceStatusManager {
                             }
                         }
                     }
-
                 } else {
                     logger.debug("Found new device!");
                     if (trashDevices.isEmpty()) {
@@ -320,21 +320,18 @@ public class DeviceStatusManagerImpl implements DeviceStatusManager {
                                 currentDevice.getDSID());
                     } else {
                         logger.debug("Search device in trashDevices.");
-                        TrashDevice foundTrashDevice = null;
-                        for (TrashDevice trashDevice : trashDevices) {
-                            if (trashDevice != null) {
-                                if (trashDevice.getDevice().equals(currentDevice)) {
-                                    foundTrashDevice = trashDevice;
-                                    logger.debug(
-                                            "Found device in trashDevices, add TrashDevice with dSID {} to the StructureManager!",
-                                            currentDeviceDSID);
-                                }
+                        boolean found = trashDevices.removeIf(trashDevice -> {
+                            if (trashDevice.getDevice().equals(currentDevice)) {
+                                logger.debug(
+                                        "Found device in trashDevices, add TrashDevice with dSID {} to the StructureManager!",
+                                        currentDeviceDSID);
+                                strucMan.addDeviceToStructure(trashDevice.getDevice());
+                                return true;
+                            } else {
+                                return false;
                             }
-                        }
-                        if (foundTrashDevice != null) {
-                            trashDevices.remove(foundTrashDevice);
-                            strucMan.addDeviceToStructure(foundTrashDevice.getDevice());
-                        } else {
+                        });
+                        if (!found) {
                             strucMan.addDeviceToStructure(currentDevice);
                             logger.debug(
                                     "Can't find device in trashDevices, add Device with dSID: {} to the StructureManager!",
@@ -387,18 +384,19 @@ public class DeviceStatusManagerImpl implements DeviceStatusManager {
                             DeviceStatusListener.DEVICE_DISCOVERY, device.getDSID().getValue());
                 } else {
                     logger.debug(
-                            "The device-Discovery is not registrated, can't inform device discovery about removed device.");
+                            "The device-Discovery is not registered, can't inform device discovery about removed device.");
                 }
             }
 
             if (!trashDevices.isEmpty() && (lastBinCheck + config.getBinCheckTime() < System.currentTimeMillis())) {
-                for (TrashDevice trashDevice : trashDevices) {
+                trashDevices.removeIf(trashDevice -> {
                     if (trashDevice.isTimeToDelete(Calendar.getInstance().get(Calendar.DAY_OF_YEAR))) {
-                        logger.debug("Found trashDevice that have to delete!");
-                        trashDevices.remove(trashDevice);
-                        logger.debug("Delete trashDevice: {}", trashDevice.getDevice().getDSID().getValue());
+                        logger.debug("Deleted trashDevice: {}", trashDevice.getDevice().getDSID().getValue());
+                        return true;
+                    } else {
+                        return false;
                     }
-                }
+                });
                 lastBinCheck = System.currentTimeMillis();
             }
         }

diff --git a/bundles/org.openhab.binding.digitalstrom/src/main/java/org/openhab/binding/digitalstrom/internal/lib/manager/impl/SceneManagerImpl.java b/bundles/org.openhab.binding.digitalstrom/src/main/java/org/openhab/binding/digitalstrom/internal/lib/manager/impl/SceneManagerImpl.java
index 9fba69a9bb4fe812245ac0450149acfe8bfe507b..1c7bd66589794196d288a21108d844c23d80676f 100644 (file)
--- a/bundles/org.openhab.binding.digitalstrom/src/main/java/org/openhab/binding/digitalstrom/internal/lib/manager/impl/SceneManagerImpl.java
+++ b/bundles/org.openhab.binding.digitalstrom/src/main/java/org/openhab/binding/digitalstrom/internal/lib/manager/impl/SceneManagerImpl.java
@@ -286,13 +286,16 @@ public class SceneManagerImpl implements SceneManager {
             }
         } else {
             InternalScene oldScene = this.internalSceneMap.get(intScene.getID());
-            String oldSceneName = this.internalSceneMap.get(intScene.getID()).getSceneName();
-            String newSceneName = intScene.getSceneName();
-            if ((oldSceneName.contains("Zone:") && oldSceneName.contains("Group:") && oldSceneName.contains("Scene:"))
-                    && !(newSceneName.contains("Zone:") && newSceneName.contains("Group:")
-                            && newSceneName.contains("Scene:"))) {
-                oldScene.setSceneName(newSceneName);
-                this.discovery.sceneDiscoverd(oldScene);
+            if (oldScene != null) {
+                String oldSceneName = oldScene.getSceneName();
+                String newSceneName = intScene.getSceneName();
+                if ((oldSceneName.contains("Zone:") && oldSceneName.contains("Group:")
+                        && oldSceneName.contains("Scene:"))
+                        && !(newSceneName.contains("Zone:") && newSceneName.contains("Group:")
+                                && newSceneName.contains("Scene:"))) {
+                    oldScene.setSceneName(newSceneName);
+                    this.discovery.sceneDiscoverd(oldScene);
+                }
             }
         }
     }