[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>
This commit is contained in:
J-N-K 2021-01-15 23:48:48 +01:00 committed by GitHub
parent 6cf488f5aa
commit cc70f5609a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 35 additions and 33 deletions

View File

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

View File

@ -19,6 +19,7 @@ import java.util.LinkedList;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Map.Entry; import java.util.Map.Entry;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
@ -113,7 +114,7 @@ public class DeviceStatusManagerImpl implements DeviceStatusManager {
private SceneReadingJobExecutor sceneJobExecutor; private SceneReadingJobExecutor sceneJobExecutor;
private EventListener eventListener; private EventListener eventListener;
private final List<TrashDevice> trashDevices = new LinkedList<>(); private final List<TrashDevice> trashDevices = new CopyOnWriteArrayList<>();
private long lastBinCheck = 0; private long lastBinCheck = 0;
private ManagerStates state = ManagerStates.STOPPED; private ManagerStates state = ManagerStates.STOPPED;
@ -310,7 +311,6 @@ public class DeviceStatusManagerImpl implements DeviceStatusManager {
} }
} }
} }
} else { } else {
logger.debug("Found new device!"); logger.debug("Found new device!");
if (trashDevices.isEmpty()) { if (trashDevices.isEmpty()) {
@ -320,21 +320,18 @@ public class DeviceStatusManagerImpl implements DeviceStatusManager {
currentDevice.getDSID()); currentDevice.getDSID());
} else { } else {
logger.debug("Search device in trashDevices."); logger.debug("Search device in trashDevices.");
TrashDevice foundTrashDevice = null; boolean found = trashDevices.removeIf(trashDevice -> {
for (TrashDevice trashDevice : trashDevices) { if (trashDevice.getDevice().equals(currentDevice)) {
if (trashDevice != null) { logger.debug(
if (trashDevice.getDevice().equals(currentDevice)) { "Found device in trashDevices, add TrashDevice with dSID {} to the StructureManager!",
foundTrashDevice = trashDevice; currentDeviceDSID);
logger.debug( strucMan.addDeviceToStructure(trashDevice.getDevice());
"Found device in trashDevices, add TrashDevice with dSID {} to the StructureManager!", return true;
currentDeviceDSID); } else {
} return false;
} }
} });
if (foundTrashDevice != null) { if (!found) {
trashDevices.remove(foundTrashDevice);
strucMan.addDeviceToStructure(foundTrashDevice.getDevice());
} else {
strucMan.addDeviceToStructure(currentDevice); strucMan.addDeviceToStructure(currentDevice);
logger.debug( logger.debug(
"Can't find device in trashDevices, add Device with dSID: {} to the StructureManager!", "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()); DeviceStatusListener.DEVICE_DISCOVERY, device.getDSID().getValue());
} else { } else {
logger.debug( 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())) { 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))) { if (trashDevice.isTimeToDelete(Calendar.getInstance().get(Calendar.DAY_OF_YEAR))) {
logger.debug("Found trashDevice that have to delete!"); logger.debug("Deleted trashDevice: {}", trashDevice.getDevice().getDSID().getValue());
trashDevices.remove(trashDevice); return true;
logger.debug("Delete trashDevice: {}", trashDevice.getDevice().getDSID().getValue()); } else {
return false;
} }
} });
lastBinCheck = System.currentTimeMillis(); lastBinCheck = System.currentTimeMillis();
} }
} }

View File

@ -286,13 +286,16 @@ public class SceneManagerImpl implements SceneManager {
} }
} else { } else {
InternalScene oldScene = this.internalSceneMap.get(intScene.getID()); InternalScene oldScene = this.internalSceneMap.get(intScene.getID());
String oldSceneName = this.internalSceneMap.get(intScene.getID()).getSceneName(); if (oldScene != null) {
String newSceneName = intScene.getSceneName(); String oldSceneName = oldScene.getSceneName();
if ((oldSceneName.contains("Zone:") && oldSceneName.contains("Group:") && oldSceneName.contains("Scene:")) String newSceneName = intScene.getSceneName();
&& !(newSceneName.contains("Zone:") && newSceneName.contains("Group:") if ((oldSceneName.contains("Zone:") && oldSceneName.contains("Group:")
&& newSceneName.contains("Scene:"))) { && oldSceneName.contains("Scene:"))
oldScene.setSceneName(newSceneName); && !(newSceneName.contains("Zone:") && newSceneName.contains("Group:")
this.discovery.sceneDiscoverd(oldScene); && newSceneName.contains("Scene:"))) {
oldScene.setSceneName(newSceneName);
this.discovery.sceneDiscoverd(oldScene);
}
} }
} }
} }