From deb519ebd6798450509afaf4067e140edd7eb0d0 Mon Sep 17 00:00:00 2001 From: Anton Tananaev Date: Sat, 22 Feb 2020 11:34:57 -0800 Subject: Fix concurrency issues --- .../org/traccar/api/ExtendedObjectResource.java | 2 +- .../org/traccar/database/BaseObjectManager.java | 60 +++++++++-- .../java/org/traccar/database/CommandsManager.java | 29 ++++- .../java/org/traccar/database/DeviceManager.java | 118 +++++++++++++++------ .../java/org/traccar/database/DriversManager.java | 51 ++++++--- .../traccar/database/ExtendedObjectManager.java | 78 +++++++++----- .../java/org/traccar/database/GroupsManager.java | 5 +- .../org/traccar/database/SimpleObjectManager.java | 30 +++--- .../java/org/traccar/database/UsersManager.java | 6 +- 9 files changed, 276 insertions(+), 103 deletions(-) (limited to 'src/main/java') diff --git a/src/main/java/org/traccar/api/ExtendedObjectResource.java b/src/main/java/org/traccar/api/ExtendedObjectResource.java index 007a7b1bd..9e554217e 100644 --- a/src/main/java/org/traccar/api/ExtendedObjectResource.java +++ b/src/main/java/org/traccar/api/ExtendedObjectResource.java @@ -55,8 +55,8 @@ public class ExtendedObjectResource extends BaseObjectResou Context.getPermissionsManager().checkDevice(getUserId(), deviceId); result.retainAll(manager.getDeviceItems(deviceId)); } - return manager.getItems(result); + return manager.getItems(result); } } diff --git a/src/main/java/org/traccar/database/BaseObjectManager.java b/src/main/java/org/traccar/database/BaseObjectManager.java index 8bf9ef860..e274e5aba 100644 --- a/src/main/java/org/traccar/database/BaseObjectManager.java +++ b/src/main/java/org/traccar/database/BaseObjectManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2017 Anton Tananaev (anton@traccar.org) + * Copyright 2017 - 2020 Anton Tananaev (anton@traccar.org) * Copyright 2017 Andrey Kunitsyn (andrey@traccar.org) * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -23,6 +23,8 @@ import java.util.LinkedList; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -32,6 +34,8 @@ public class BaseObjectManager { private static final Logger LOGGER = LoggerFactory.getLogger(BaseObjectManager.class); + private final ReadWriteLock lock = new ReentrantReadWriteLock(); + private final DataManager dataManager; private Map items; @@ -43,6 +47,22 @@ public class BaseObjectManager { refreshItems(); } + protected final void readLock() { + lock.readLock().lock(); + } + + protected final void readUnlock() { + lock.readLock().unlock(); + } + + protected final void writeLock() { + lock.writeLock().lock(); + } + + protected final void writeUnlock() { + lock.writeLock().unlock(); + } + protected final DataManager getDataManager() { return dataManager; } @@ -52,12 +72,18 @@ public class BaseObjectManager { } public T getById(long itemId) { - return items.get(itemId); + try { + readLock(); + return items.get(itemId); + } finally { + readUnlock(); + } } public void refreshItems() { if (dataManager != null) { try { + writeLock(); Collection databaseItems = dataManager.getObjects(baseClass); if (items == null) { items = new ConcurrentHashMap<>(databaseItems.size()); @@ -78,12 +104,19 @@ public class BaseObjectManager { } } catch (SQLException error) { LOGGER.warn("Error refreshing items", error); + } finally { + writeUnlock(); } } } protected void addNewItem(T item) { - items.put(item.getId(), item); + try { + writeLock(); + items.put(item.getId(), item); + } finally { + writeUnlock(); + } } public void addItem(T item) throws SQLException { @@ -92,7 +125,12 @@ public class BaseObjectManager { } protected void updateCachedItem(T item) { - items.put(item.getId(), item); + try { + writeLock(); + items.put(item.getId(), item); + } finally { + writeUnlock(); + } } public void updateItem(T item) throws SQLException { @@ -101,7 +139,12 @@ public class BaseObjectManager { } protected void removeCachedItem(long itemId) { - items.remove(itemId); + try { + writeLock(); + items.remove(itemId); + } finally { + writeUnlock(); + } } public void removeItem(long itemId) throws SQLException { @@ -121,7 +164,12 @@ public class BaseObjectManager { } public Set getAllItems() { - return items.keySet(); + try { + readLock(); + return items.keySet(); + } finally { + readUnlock(); + } } } diff --git a/src/main/java/org/traccar/database/CommandsManager.java b/src/main/java/org/traccar/database/CommandsManager.java index dc9512d9e..de6eeeba8 100644 --- a/src/main/java/org/traccar/database/CommandsManager.java +++ b/src/main/java/org/traccar/database/CommandsManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2017 Anton Tananaev (anton@traccar.org) + * Copyright 2017 - 2020 Anton Tananaev (anton@traccar.org) * Copyright 2017 Andrey Kunitsyn (andrey@traccar.org) * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -142,14 +142,33 @@ public class CommandsManager extends ExtendedObjectManager { } private Queue getDeviceQueue(long deviceId) { - if (!deviceQueues.containsKey(deviceId)) { - deviceQueues.put(deviceId, new ConcurrentLinkedQueue()); + Queue deviceQueue; + try { + readLock(); + deviceQueue = deviceQueues.get(deviceId); + } finally { + readUnlock(); + } + if (deviceQueue != null) { + return deviceQueue; + } else { + try { + writeLock(); + return deviceQueues.computeIfAbsent(deviceId, key -> new ConcurrentLinkedQueue<>()); + } finally { + writeUnlock(); + } } - return deviceQueues.get(deviceId); } public void sendQueuedCommands(ActiveDevice activeDevice) { - Queue deviceQueue = deviceQueues.get(activeDevice.getDeviceId()); + Queue deviceQueue; + try { + readLock(); + deviceQueue = deviceQueues.get(activeDevice.getDeviceId()); + } finally { + readUnlock(); + } if (deviceQueue != null) { Command command = deviceQueue.poll(); while (command != null) { diff --git a/src/main/java/org/traccar/database/DeviceManager.java b/src/main/java/org/traccar/database/DeviceManager.java index fa95adeb2..fe17f7ced 100644 --- a/src/main/java/org/traccar/database/DeviceManager.java +++ b/src/main/java/org/traccar/database/DeviceManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2016 - 2019 Anton Tananaev (anton@traccar.org) + * Copyright 2016 - 2020 Anton Tananaev (anton@traccar.org) * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -58,11 +58,16 @@ public class DeviceManager extends BaseObjectManager implements Identity public DeviceManager(DataManager dataManager) { super(dataManager, Device.class); this.config = Context.getConfig(); - if (devicesByPhone == null) { - devicesByPhone = new ConcurrentHashMap<>(); - } - if (devicesByUniqueId == null) { - devicesByUniqueId = new ConcurrentHashMap<>(); + try { + writeLock(); + if (devicesByPhone == null) { + devicesByPhone = new ConcurrentHashMap<>(); + } + if (devicesByUniqueId == null) { + devicesByUniqueId = new ConcurrentHashMap<>(); + } + } finally { + writeUnlock(); } dataRefreshDelay = config.getLong("database.refreshDelay", DEFAULT_REFRESH_DELAY) * 1000; lookupGroupsAttribute = config.getBoolean("deviceManager.lookupGroupsAttribute"); @@ -108,11 +113,20 @@ public class DeviceManager extends BaseObjectManager implements Identity @Override public Device getByUniqueId(String uniqueId) throws SQLException { - boolean forceUpdate = !devicesByUniqueId.containsKey(uniqueId) && !config.getBoolean("database.ignoreUnknown"); - + boolean forceUpdate; + try { + readLock(); + forceUpdate = !devicesByUniqueId.containsKey(uniqueId) && !config.getBoolean("database.ignoreUnknown"); + } finally { + readUnlock(); + } updateDeviceCache(forceUpdate); - - return devicesByUniqueId.get(uniqueId); + try { + readLock(); + return devicesByUniqueId.get(uniqueId); + } finally { + readUnlock(); + } } @Override @@ -134,7 +148,12 @@ public class DeviceManager extends BaseObjectManager implements Identity } public Device getDeviceByPhone(String phone) { - return devicesByPhone.get(phone); + try { + readLock(); + return devicesByPhone.get(phone); + } finally { + readUnlock(); + } } @Override @@ -176,8 +195,7 @@ public class DeviceManager extends BaseObjectManager implements Identity } public Set getAllManagedItems(long userId) { - Set result = new HashSet<>(); - result.addAll(getAllUserItems(userId)); + Set result = new HashSet<>(getAllUserItems(userId)); for (long managedUserId : Context.getUsersManager().getUserItems(userId)) { result.addAll(getAllUserItems(managedUserId)); } @@ -186,34 +204,68 @@ public class DeviceManager extends BaseObjectManager implements Identity @Override public Set getManagedItems(long userId) { - Set result = new HashSet<>(); - result.addAll(getUserItems(userId)); + Set result = new HashSet<>(getUserItems(userId)); for (long managedUserId : Context.getUsersManager().getUserItems(userId)) { result.addAll(getUserItems(managedUserId)); } return result; } - private void putUniqueDeviceId(Device device) { - if (devicesByUniqueId == null) { - devicesByUniqueId = new ConcurrentHashMap<>(getAllItems().size()); + private void addByUniqueId(Device device) { + try { + writeLock(); + if (devicesByUniqueId == null) { + devicesByUniqueId = new ConcurrentHashMap<>(); + } + devicesByUniqueId.put(device.getUniqueId(), device); + } finally { + writeUnlock(); + } + } + + private void removeByUniqueId(String deviceUniqueId) { + try { + writeLock(); + if (devicesByUniqueId != null) { + devicesByUniqueId.remove(deviceUniqueId); + } + } finally { + writeUnlock(); + } + } + + private void addByPhone(Device device) { + try { + writeLock(); + if (devicesByPhone == null) { + devicesByPhone = new ConcurrentHashMap<>(); + } + devicesByPhone.put(device.getPhone(), device); + } finally { + writeUnlock(); } - devicesByUniqueId.put(device.getUniqueId(), device); } - private void putPhone(Device device) { - if (devicesByPhone == null) { - devicesByPhone = new ConcurrentHashMap<>(getAllItems().size()); + private void removeByPhone(String phone) { + if (phone == null || phone.isEmpty()) { + return; + } + try { + writeLock(); + if (devicesByPhone != null) { + devicesByPhone.remove(phone); + } + } finally { + writeUnlock(); } - devicesByPhone.put(device.getPhone(), device); } @Override protected void addNewItem(Device device) { super.addNewItem(device); - putUniqueDeviceId(device); + addByUniqueId(device); if (device.getPhone() != null && !device.getPhone().isEmpty()) { - putPhone(device); + addByPhone(device); } if (Context.getGeofenceManager() != null) { Position lastPosition = getLastPosition(device.getId()); @@ -234,18 +286,16 @@ public class DeviceManager extends BaseObjectManager implements Identity cachedDevice.setDisabled(device.getDisabled()); cachedDevice.setAttributes(device.getAttributes()); if (!device.getUniqueId().equals(cachedDevice.getUniqueId())) { - devicesByUniqueId.remove(cachedDevice.getUniqueId()); + removeByUniqueId(cachedDevice.getUniqueId()); cachedDevice.setUniqueId(device.getUniqueId()); - putUniqueDeviceId(cachedDevice); + addByUniqueId(cachedDevice); } if (device.getPhone() != null && !device.getPhone().isEmpty() && !device.getPhone().equals(cachedDevice.getPhone())) { String phone = cachedDevice.getPhone(); - if (phone != null && !phone.isEmpty()) { - devicesByPhone.remove(phone); - } + removeByPhone(phone); cachedDevice.setPhone(device.getPhone()); - putPhone(cachedDevice); + addByPhone(cachedDevice); } } @@ -256,10 +306,8 @@ public class DeviceManager extends BaseObjectManager implements Identity String deviceUniqueId = cachedDevice.getUniqueId(); String phone = cachedDevice.getPhone(); super.removeCachedItem(deviceId); - devicesByUniqueId.remove(deviceUniqueId); - if (phone != null && !phone.isEmpty()) { - devicesByPhone.remove(phone); - } + removeByUniqueId(deviceUniqueId); + removeByPhone(phone); } positions.remove(deviceId); } diff --git a/src/main/java/org/traccar/database/DriversManager.java b/src/main/java/org/traccar/database/DriversManager.java index 930951460..d111cd643 100644 --- a/src/main/java/org/traccar/database/DriversManager.java +++ b/src/main/java/org/traccar/database/DriversManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2017 Anton Tananaev (anton@traccar.org) + * Copyright 2017 - 2020 Anton Tananaev (anton@traccar.org) * Copyright 2017 Andrey Kunitsyn (andrey@traccar.org) * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -27,22 +27,44 @@ public class DriversManager extends ExtendedObjectManager { public DriversManager(DataManager dataManager) { super(dataManager, Driver.class); - if (driversByUniqueId == null) { - driversByUniqueId = new ConcurrentHashMap<>(); + try { + writeLock(); + if (driversByUniqueId == null) { + driversByUniqueId = new ConcurrentHashMap<>(); + } + } finally { + writeUnlock(); } } - private void putUniqueDriverId(Driver driver) { - if (driversByUniqueId == null) { - driversByUniqueId = new ConcurrentHashMap<>(getAllItems().size()); + private void addByUniqueId(Driver driver) { + try { + writeLock(); + if (driversByUniqueId == null) { + driversByUniqueId = new ConcurrentHashMap<>(); + } + driversByUniqueId.put(driver.getUniqueId(), driver); + } finally { + writeUnlock(); + } + } + + private void removeByUniqueId(String driverUniqueId) { + try { + writeLock(); + if (driversByUniqueId == null) { + driversByUniqueId = new ConcurrentHashMap<>(); + } + driversByUniqueId.remove(driverUniqueId); + } finally { + writeUnlock(); } - driversByUniqueId.put(driver.getUniqueId(), driver); } @Override protected void addNewItem(Driver driver) { super.addNewItem(driver); - putUniqueDriverId(driver); + addByUniqueId(driver); } @Override @@ -50,9 +72,9 @@ public class DriversManager extends ExtendedObjectManager { Driver cachedDriver = getById(driver.getId()); cachedDriver.setName(driver.getName()); if (!driver.getUniqueId().equals(cachedDriver.getUniqueId())) { - driversByUniqueId.remove(cachedDriver.getUniqueId()); + removeByUniqueId(cachedDriver.getUniqueId()); cachedDriver.setUniqueId(driver.getUniqueId()); - putUniqueDriverId(cachedDriver); + addByUniqueId(cachedDriver); } cachedDriver.setAttributes(driver.getAttributes()); } @@ -63,11 +85,16 @@ public class DriversManager extends ExtendedObjectManager { if (cachedDriver != null) { String driverUniqueId = cachedDriver.getUniqueId(); super.removeCachedItem(driverId); - driversByUniqueId.remove(driverUniqueId); + removeByUniqueId(driverUniqueId); } } public Driver getDriverByUniqueId(String uniqueId) { - return driversByUniqueId.get(uniqueId); + try { + readLock(); + return driversByUniqueId.get(uniqueId); + } finally { + readUnlock(); + } } } diff --git a/src/main/java/org/traccar/database/ExtendedObjectManager.java b/src/main/java/org/traccar/database/ExtendedObjectManager.java index ceb85b537..93e5820fb 100644 --- a/src/main/java/org/traccar/database/ExtendedObjectManager.java +++ b/src/main/java/org/traccar/database/ExtendedObjectManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2017 Anton Tananaev (anton@traccar.org) + * Copyright 2017 - 2020 Anton Tananaev (anton@traccar.org) * Copyright 2017 Andrey Kunitsyn (andrey@traccar.org) * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -45,24 +45,45 @@ public abstract class ExtendedObjectManager extends SimpleO } public final Set getGroupItems(long groupId) { - if (!groupItems.containsKey(groupId)) { - groupItems.put(groupId, new HashSet()); + try { + readLock(); + Set result = groupItems.get(groupId); + if (result != null) { + return new HashSet<>(result); + } else { + return new HashSet<>(); + } + } finally { + readUnlock(); } - return groupItems.get(groupId); } public final Set getDeviceItems(long deviceId) { - if (!deviceItems.containsKey(deviceId)) { - deviceItems.put(deviceId, new HashSet()); + try { + readLock(); + Set result = deviceItems.get(deviceId); + if (result != null) { + return new HashSet<>(result); + } else { + return new HashSet<>(); + } + } finally { + readUnlock(); } - return deviceItems.get(deviceId); } public Set getAllDeviceItems(long deviceId) { - if (!deviceItemsWithGroups.containsKey(deviceId)) { - deviceItemsWithGroups.put(deviceId, new HashSet()); + try { + readLock(); + Set result = deviceItemsWithGroups.get(deviceId); + if (result != null) { + return new HashSet<>(result); + } else { + return new HashSet<>(); + } + } finally { + readUnlock(); } - return deviceItemsWithGroups.get(deviceId); } @Override @@ -74,41 +95,48 @@ public abstract class ExtendedObjectManager extends SimpleO public void refreshExtendedPermissions() { if (getDataManager() != null) { try { - Collection databaseGroupPermissions = getDataManager().getPermissions(Group.class, getBaseClass()); - groupItems.clear(); - for (Permission groupPermission : databaseGroupPermissions) { - getGroupItems(groupPermission.getOwnerId()).add(groupPermission.getPropertyId()); - } - Collection databaseDevicePermissions = getDataManager().getPermissions(Device.class, getBaseClass()); + writeLock(); + + groupItems.clear(); deviceItems.clear(); deviceItemsWithGroups.clear(); + for (Permission groupPermission : databaseGroupPermissions) { + groupItems + .computeIfAbsent(groupPermission.getOwnerId(), key -> new HashSet<>()) + .add(groupPermission.getPropertyId()); + } + for (Permission devicePermission : databaseDevicePermissions) { - getDeviceItems(devicePermission.getOwnerId()).add(devicePermission.getPropertyId()); - getAllDeviceItems(devicePermission.getOwnerId()).add(devicePermission.getPropertyId()); + deviceItems + .computeIfAbsent(devicePermission.getOwnerId(), key -> new HashSet<>()) + .add(devicePermission.getPropertyId()); + deviceItemsWithGroups + .computeIfAbsent(devicePermission.getOwnerId(), key -> new HashSet<>()) + .add(devicePermission.getPropertyId()); } for (Device device : Context.getDeviceManager().getAllDevices()) { long groupId = device.getGroupId(); - while (groupId != 0) { - getAllDeviceItems(device.getId()).addAll(getGroupItems(groupId)); + while (groupId > 0) { + deviceItemsWithGroups + .computeIfAbsent(device.getId(), key -> new HashSet<>()) + .addAll(groupItems.getOrDefault(groupId, new HashSet<>())); Group group = Context.getGroupsManager().getById(groupId); - if (group != null) { - groupId = group.getGroupId(); - } else { - groupId = 0; - } + groupId = group != null ? group.getGroupId() : 0; } } } catch (SQLException | ClassNotFoundException error) { LOGGER.warn("Refresh permissions error", error); + } finally { + writeUnlock(); } } } diff --git a/src/main/java/org/traccar/database/GroupsManager.java b/src/main/java/org/traccar/database/GroupsManager.java index d8404c614..81f1968aa 100644 --- a/src/main/java/org/traccar/database/GroupsManager.java +++ b/src/main/java/org/traccar/database/GroupsManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2017 Anton Tananaev (anton@traccar.org) + * Copyright 2017 - 2020 Anton Tananaev (anton@traccar.org) * Copyright 2017 Andrey Kunitsyn (andrey@traccar.org) * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -95,8 +95,7 @@ public class GroupsManager extends BaseObjectManager implements Managable @Override public Set getManagedItems(long userId) { - Set result = new HashSet<>(); - result.addAll(getUserItems(userId)); + Set result = getUserItems(userId); for (long managedUserId : Context.getUsersManager().getUserItems(userId)) { result.addAll(getUserItems(managedUserId)); } diff --git a/src/main/java/org/traccar/database/SimpleObjectManager.java b/src/main/java/org/traccar/database/SimpleObjectManager.java index 15dda4520..eb8284d4e 100644 --- a/src/main/java/org/traccar/database/SimpleObjectManager.java +++ b/src/main/java/org/traccar/database/SimpleObjectManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2017 Anton Tananaev (anton@traccar.org) + * Copyright 2017 - 2020 Anton Tananaev (anton@traccar.org) * Copyright 2017 Andrey Kunitsyn (andrey@traccar.org) * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -42,16 +42,22 @@ public abstract class SimpleObjectManager extends BaseObjec @Override public final Set getUserItems(long userId) { - if (!userItems.containsKey(userId)) { - userItems.put(userId, new HashSet()); + try { + readLock(); + Set result = userItems.get(userId); + if (result != null) { + return new HashSet<>(result); + } else { + return new HashSet<>(); + } + } finally { + readUnlock(); } - return userItems.get(userId); } @Override public Set getManagedItems(long userId) { - Set result = new HashSet<>(); - result.addAll(getUserItems(userId)); + Set result = getUserItems(userId); for (long managedUserId : Context.getUsersManager().getUserItems(userId)) { result.addAll(getUserItems(managedUserId)); } @@ -71,16 +77,16 @@ public abstract class SimpleObjectManager extends BaseObjec public final void refreshUserItems() { if (getDataManager() != null) { try { - if (userItems != null) { - userItems.clear(); - } else { - userItems = new ConcurrentHashMap<>(); - } + writeLock(); + userItems = new ConcurrentHashMap<>(); for (Permission permission : getDataManager().getPermissions(User.class, getBaseClass())) { - getUserItems(permission.getOwnerId()).add(permission.getPropertyId()); + Set items = userItems.computeIfAbsent(permission.getOwnerId(), key -> new HashSet<>()); + items.add(permission.getPropertyId()); } } catch (SQLException | ClassNotFoundException error) { LOGGER.warn("Error getting permissions", error); + } finally { + writeUnlock(); } } } diff --git a/src/main/java/org/traccar/database/UsersManager.java b/src/main/java/org/traccar/database/UsersManager.java index 576a9e6c7..b741a85b6 100644 --- a/src/main/java/org/traccar/database/UsersManager.java +++ b/src/main/java/org/traccar/database/UsersManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2017 Anton Tananaev (anton@traccar.org) + * Copyright 2017 - 2020 Anton Tananaev (anton@traccar.org) * Copyright 2017 Andrey Kunitsyn (andrey@traccar.org) * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -16,7 +16,6 @@ */ package org.traccar.database; -import java.util.HashSet; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; @@ -73,8 +72,7 @@ public class UsersManager extends SimpleObjectManager { @Override public Set getManagedItems(long userId) { - Set result = new HashSet<>(); - result.addAll(getUserItems(userId)); + Set result = getUserItems(userId); result.add(userId); return result; } -- cgit v1.2.3