From 96ab1b4533b62464b8325373e677209290bced3e Mon Sep 17 00:00:00 2001 From: ElgarL Date: Mon, 10 Sep 2012 00:15:42 +0100 Subject: Change to Hashtables to reduce the chance of a ConcurrentModificationException pulling group/user data in an Async thread. --- .../src/org/anjocaido/groupmanager/dataholder/GroupsDataHolder.java | 6 +++--- .../src/org/anjocaido/groupmanager/dataholder/UsersDataHolder.java | 6 +++--- .../org/anjocaido/groupmanager/dataholder/worlds/WorldsHolder.java | 5 +++-- 3 files changed, 9 insertions(+), 8 deletions(-) (limited to 'EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder') diff --git a/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/GroupsDataHolder.java b/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/GroupsDataHolder.java index eaaaace74..ed6a985ed 100644 --- a/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/GroupsDataHolder.java +++ b/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/GroupsDataHolder.java @@ -1,7 +1,7 @@ package org.anjocaido.groupmanager.dataholder; import java.io.File; -import java.util.HashMap; +import java.util.Hashtable; import java.util.Map; import org.anjocaido.groupmanager.data.Group; @@ -23,7 +23,7 @@ public class GroupsDataHolder { /** * The actual groups holder */ - private Map groups = new HashMap(); + private Hashtable groups = new Hashtable(); /** * Constructor @@ -69,7 +69,7 @@ public class GroupsDataHolder { */ public void setGroups(Map groups) { - this.groups = groups; + this.groups = (Hashtable) groups; } /** diff --git a/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/UsersDataHolder.java b/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/UsersDataHolder.java index 665fe227d..3ddc2177d 100644 --- a/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/UsersDataHolder.java +++ b/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/UsersDataHolder.java @@ -1,7 +1,7 @@ package org.anjocaido.groupmanager.dataholder; import java.io.File; -import java.util.HashMap; +import java.util.Hashtable; import java.util.Map; import org.anjocaido.groupmanager.data.User; @@ -22,7 +22,7 @@ public class UsersDataHolder { /** * The actual groups holder */ - private Map users = new HashMap(); + private Hashtable users = new Hashtable(); /** * Constructor @@ -53,7 +53,7 @@ public class UsersDataHolder { */ public void setUsers(Map users) { - this.users = users; + this.users = (Hashtable) users; } /** diff --git a/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/worlds/WorldsHolder.java b/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/worlds/WorldsHolder.java index f84262eeb..2a6463fe3 100644 --- a/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/worlds/WorldsHolder.java +++ b/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/worlds/WorldsHolder.java @@ -11,6 +11,7 @@ import java.io.InputStream; import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; +import java.util.Hashtable; import java.util.List; import java.util.Map; import java.util.Properties; @@ -33,7 +34,7 @@ public class WorldsHolder { /** * Map with instances of loaded worlds. */ - private Map worldsData = new HashMap(); + private Hashtable worldsData = new Hashtable(); /** * Map of mirrors: @@ -61,7 +62,7 @@ public class WorldsHolder { public void resetWorldsHolder() { - worldsData = new HashMap(); + worldsData = new Hashtable(); mirrorsGroup = new HashMap(); mirrorsUser = new HashMap(); -- cgit v1.2.3 From 71179e3dfd4ed8668fd4096d15326613c4a05750 Mon Sep 17 00:00:00 2001 From: ElgarL Date: Mon, 10 Sep 2012 17:46:01 +0100 Subject: Synchronize the world data holder. --- .../org/anjocaido/groupmanager/dataholder/worlds/WorldsHolder.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder') diff --git a/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/worlds/WorldsHolder.java b/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/worlds/WorldsHolder.java index 2a6463fe3..123070759 100644 --- a/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/worlds/WorldsHolder.java +++ b/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/worlds/WorldsHolder.java @@ -403,8 +403,10 @@ public class WorldsHolder { if (worldsData.containsKey(worldNameLowered)) { OverloadedWorldHolder data = worldsData.get(worldNameLowered); - data.updateDataSource(); - return data; + synchronized (data) { + data.updateDataSource(); + return data; + } } return null; -- cgit v1.2.3 From 736a6d273abb000c81b17d0142ce358d2225f873 Mon Sep 17 00:00:00 2001 From: snowleo Date: Mon, 10 Sep 2012 19:06:17 +0200 Subject: Revert bad synchronization This reverts commit 96ab1b4533b62464b8325373e677209290bced3e. --- .../anjocaido/groupmanager/dataholder/GroupsDataHolder.java | 6 +++--- .../anjocaido/groupmanager/dataholder/UsersDataHolder.java | 6 +++--- .../groupmanager/dataholder/worlds/WorldsHolder.java | 11 ++++------- 3 files changed, 10 insertions(+), 13 deletions(-) (limited to 'EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder') diff --git a/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/GroupsDataHolder.java b/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/GroupsDataHolder.java index ed6a985ed..eaaaace74 100644 --- a/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/GroupsDataHolder.java +++ b/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/GroupsDataHolder.java @@ -1,7 +1,7 @@ package org.anjocaido.groupmanager.dataholder; import java.io.File; -import java.util.Hashtable; +import java.util.HashMap; import java.util.Map; import org.anjocaido.groupmanager.data.Group; @@ -23,7 +23,7 @@ public class GroupsDataHolder { /** * The actual groups holder */ - private Hashtable groups = new Hashtable(); + private Map groups = new HashMap(); /** * Constructor @@ -69,7 +69,7 @@ public class GroupsDataHolder { */ public void setGroups(Map groups) { - this.groups = (Hashtable) groups; + this.groups = groups; } /** diff --git a/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/UsersDataHolder.java b/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/UsersDataHolder.java index 3ddc2177d..665fe227d 100644 --- a/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/UsersDataHolder.java +++ b/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/UsersDataHolder.java @@ -1,7 +1,7 @@ package org.anjocaido.groupmanager.dataholder; import java.io.File; -import java.util.Hashtable; +import java.util.HashMap; import java.util.Map; import org.anjocaido.groupmanager.data.User; @@ -22,7 +22,7 @@ public class UsersDataHolder { /** * The actual groups holder */ - private Hashtable users = new Hashtable(); + private Map users = new HashMap(); /** * Constructor @@ -53,7 +53,7 @@ public class UsersDataHolder { */ public void setUsers(Map users) { - this.users = (Hashtable) users; + this.users = users; } /** diff --git a/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/worlds/WorldsHolder.java b/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/worlds/WorldsHolder.java index 123070759..f84262eeb 100644 --- a/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/worlds/WorldsHolder.java +++ b/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/worlds/WorldsHolder.java @@ -11,7 +11,6 @@ import java.io.InputStream; import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; -import java.util.Hashtable; import java.util.List; import java.util.Map; import java.util.Properties; @@ -34,7 +33,7 @@ public class WorldsHolder { /** * Map with instances of loaded worlds. */ - private Hashtable worldsData = new Hashtable(); + private Map worldsData = new HashMap(); /** * Map of mirrors: @@ -62,7 +61,7 @@ public class WorldsHolder { public void resetWorldsHolder() { - worldsData = new Hashtable(); + worldsData = new HashMap(); mirrorsGroup = new HashMap(); mirrorsUser = new HashMap(); @@ -403,10 +402,8 @@ public class WorldsHolder { if (worldsData.containsKey(worldNameLowered)) { OverloadedWorldHolder data = worldsData.get(worldNameLowered); - synchronized (data) { - data.updateDataSource(); - return data; - } + data.updateDataSource(); + return data; } return null; -- cgit v1.2.3 From 1cf0ebbd7f120c0b8d421cc459f2d5a1386d82dc Mon Sep 17 00:00:00 2001 From: snowleo Date: Mon, 10 Sep 2012 20:49:07 +0200 Subject: Thread safety for GM --- .../groupmanager/dataholder/GroupsDataHolder.java | 11 +++++--- .../dataholder/OverloadedWorldHolder.java | 11 +++++++- .../groupmanager/dataholder/UsersDataHolder.java | 12 +++++---- .../groupmanager/dataholder/WorldDataHolder.java | 29 ++++++++++++++++------ 4 files changed, 46 insertions(+), 17 deletions(-) (limited to 'EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder') diff --git a/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/GroupsDataHolder.java b/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/GroupsDataHolder.java index eaaaace74..fdd099c87 100644 --- a/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/GroupsDataHolder.java +++ b/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/GroupsDataHolder.java @@ -1,6 +1,7 @@ package org.anjocaido.groupmanager.dataholder; import java.io.File; +import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -23,7 +24,7 @@ public class GroupsDataHolder { /** * The actual groups holder */ - private Map groups = new HashMap(); + private final Map groups = Collections.synchronizedMap(new HashMap()); /** * Constructor @@ -36,8 +37,10 @@ public class GroupsDataHolder { this.dataSource = dataSource; //push this data source to the users, so they pull the correct groups data. + synchronized(groups) { for (Group group : groups.values()) group.setDataSource(this.dataSource); + } } /** @@ -57,6 +60,7 @@ public class GroupsDataHolder { } /** + * Note: Iteration over this object has to be synchronized! * @return the groups */ public Map getGroups() { @@ -67,9 +71,8 @@ public class GroupsDataHolder { /** * @param groups the groups to set */ - public void setGroups(Map groups) { - - this.groups = groups; + public void resetGroups() { + this.groups.clear(); } /** diff --git a/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/OverloadedWorldHolder.java b/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/OverloadedWorldHolder.java index 84561b6e5..ef9f605ed 100644 --- a/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/OverloadedWorldHolder.java +++ b/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/OverloadedWorldHolder.java @@ -6,6 +6,7 @@ package org.anjocaido.groupmanager.dataholder; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.Map; import org.anjocaido.groupmanager.data.User; @@ -19,7 +20,7 @@ public class OverloadedWorldHolder extends WorldDataHolder { /** * */ - protected Map overloadedUsers = new HashMap(); + protected final Map overloadedUsers = Collections.synchronizedMap(new HashMap()); /** * @@ -112,29 +113,35 @@ public class OverloadedWorldHolder extends WorldDataHolder { if (groupName.equals(getDefaultGroup())) { return false; } + synchronized(getGroups()) { for (String key : getGroups().keySet()) { if (groupName.equalsIgnoreCase(key)) { getGroups().remove(key); + synchronized(getUsers()) { for (String userKey : getUsers().keySet()) { User user = getUsers().get(userKey); if (user.getGroupName().equalsIgnoreCase(key)) { user.setGroup(getDefaultGroup()); } + } } //OVERLOADED CODE + synchronized(overloadedUsers) { for (String userKey : overloadedUsers.keySet()) { User user = overloadedUsers.get(userKey); if (user.getGroupName().equalsIgnoreCase(key)) { user.setGroup(getDefaultGroup()); } + } } //END OVERLOAD setGroupsChanged(true); return true; } } + } return false; } @@ -146,6 +153,7 @@ public class OverloadedWorldHolder extends WorldDataHolder { public Collection getUserList() { Collection overloadedList = new ArrayList(); + synchronized(getUsers()) { Collection normalList = getUsers().values(); for (User u : normalList) { if (overloadedUsers.containsKey(u.getName().toLowerCase())) { @@ -154,6 +162,7 @@ public class OverloadedWorldHolder extends WorldDataHolder { overloadedList.add(u); } } + } return overloadedList; } diff --git a/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/UsersDataHolder.java b/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/UsersDataHolder.java index 665fe227d..8a3c4c102 100644 --- a/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/UsersDataHolder.java +++ b/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/UsersDataHolder.java @@ -1,6 +1,7 @@ package org.anjocaido.groupmanager.dataholder; import java.io.File; +import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -22,7 +23,7 @@ public class UsersDataHolder { /** * The actual groups holder */ - private Map users = new HashMap(); + private final Map users = Collections.synchronizedMap(new HashMap()); /** * Constructor @@ -35,12 +36,14 @@ public class UsersDataHolder { this.dataSource = dataSource; //push this data source to the users, so they pull the correct groups data. + synchronized(users) { for (User user : users.values()) user.setDataSource(this.dataSource); - + } } /** + * Note: Iteration over this object has to be synchronized! * @return the users */ public Map getUsers() { @@ -51,9 +54,8 @@ public class UsersDataHolder { /** * @param users the users to set */ - public void setUsers(Map users) { - - this.users = users; + public void resetUsers() { + this.users.clear(); } /** diff --git a/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/WorldDataHolder.java b/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/WorldDataHolder.java index aad59e5aa..4d85eff68 100644 --- a/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/WorldDataHolder.java +++ b/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/WorldDataHolder.java @@ -319,8 +319,9 @@ public class WorldDataHolder { * @return a collection of the groups */ public Collection getGroupList() { - - return getGroups().values(); + synchronized(getGroups()) { + return new ArrayList(getGroups().values()); + } } /** @@ -328,8 +329,9 @@ public class WorldDataHolder { * @return a collection of the users */ public Collection getUserList() { - - return getUsers().values(); + synchronized(getUsers()) { + return new ArrayList(getUsers().values()); + } } /** @@ -944,6 +946,7 @@ public class WorldDataHolder { Map groupsMap = new HashMap(); root.put("groups", groupsMap); + synchronized(ph.getGroups()) { for (String groupKey : ph.getGroups().keySet()) { Group group = ph.getGroups().get(groupKey); @@ -966,6 +969,7 @@ public class WorldDataHolder { aGroupMap.put("permissions", group.getPermissionList()); } + } if (!root.isEmpty()) { DumperOptions opt = new DumperOptions(); @@ -1031,6 +1035,7 @@ public class WorldDataHolder { Map usersMap = new HashMap(); root.put("users", usersMap); + synchronized(ph.getUsers()) { for (String userKey : ph.getUsers().keySet()) { User user = ph.getUsers().get(userKey); if ((user.getGroup() == null || user.getGroup().equals(ph.getDefaultGroup())) && user.getPermissionList().isEmpty() && user.getVariables().isEmpty() && user.isSubGroupsEmpty()) { @@ -1060,6 +1065,7 @@ public class WorldDataHolder { aUserMap.put("subgroups", user.subGroupListStringCopy()); // END SUBGROUPS NODE - BETA } + } if (!root.isEmpty()) { DumperOptions opt = new DumperOptions(); @@ -1159,11 +1165,13 @@ public class WorldDataHolder { if (users.HaveUsersChanged()) { return true; } + synchronized(users.getUsers()) { for (User u : users.getUsers().values()) { if (u.isChanged()) { return true; } } + } return false; } @@ -1184,11 +1192,13 @@ public class WorldDataHolder { if (groups.HaveGroupsChanged()) { return true; } + synchronized(groups.getGroups()) { for (Group g : groups.getGroups().values()) { if (g.isChanged()) { return true; } } + } return false; } @@ -1198,9 +1208,11 @@ public class WorldDataHolder { public void removeUsersChangedFlag() { setUsersChanged(false); + synchronized(getUsers()) { for (User u : getUsers().values()) { u.flagAsSaved(); } + } } /** @@ -1209,9 +1221,11 @@ public class WorldDataHolder { public void removeGroupsChangedFlag() { setGroupsChanged(false); + synchronized(getGroups()) { for (Group g : getGroups().values()) { g.flagAsSaved(); } + } } /** @@ -1260,18 +1274,18 @@ public class WorldDataHolder { public void resetGroups() { // setDefaultGroup(null); - groups.setGroups(new HashMap()); + groups.resetGroups(); } /** * Resets Users */ public void resetUsers() { - - users.setUsers(new HashMap()); + users.resetUsers(); } /** + * Note: Iteration over this object has to be synchronized! * @return the groups */ public Map getGroups() { @@ -1280,6 +1294,7 @@ public class WorldDataHolder { } /** + * Note: Iteration over this object has to be synchronized! * @return the users */ public Map getUsers() { -- cgit v1.2.3