Make the injected LP permSubs replacement value maps thread safe

bukkit/src/main/java/me/lucko/luckperms/bukkit/inject/server/LuckPermsSubscriptionMap.java

@@ -25,6 +25,7 @@
 
 package me.lucko.luckperms.bukkit.inject.server;
 
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
 
@@ -37,6 +38,7 @@ import org.bukkit.plugin.PluginManager;
 import org.checkerframework.checker.nullness.qual.NonNull;
 
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Objects;
@@ -72,8 +74,10 @@ public final class LuckPermsSubscriptionMap extends HashMap<String, Map<Permissi
     final LPBukkitPlugin plugin;
 
     public LuckPermsSubscriptionMap(LPBukkitPlugin plugin, Map<String, Map<Permissible, Boolean>> existingData) {
-        super(existingData);
         this.plugin = plugin;
+        for (Entry<String, Map<Permissible, Boolean>> entry : existingData.entrySet()) {
+            super.put(entry.getKey(), new LPSubscriptionValueMap(entry.getKey(), entry.getValue()));
+        }
     }
 
     /*
@@ -94,17 +98,13 @@ public final class LuckPermsSubscriptionMap extends HashMap<String, Map<Permissi
 
         String permission = ((String) key);
 
-        Map<Permissible, Boolean> result = super.get(key);
+        LPSubscriptionValueMap result = (LPSubscriptionValueMap) super.get(key);
-
         if (result == null) {
             // calculate a new map - always!
             result = new LPSubscriptionValueMap(permission);
             super.put(permission, result);
-        } else if (!(result instanceof LPSubscriptionValueMap)) {
-            // ensure return type is a LPSubscriptionMap
-            result = new LPSubscriptionValueMap(permission, result);
-            super.put(permission, result);
         }
+
         return result;
     }
 
@@ -134,15 +134,9 @@ public final class LuckPermsSubscriptionMap extends HashMap<String, Map<Permissi
      */
     public Map<String, Map<Permissible, Boolean>> detach() {
         Map<String, Map<Permissible, Boolean>> map = new HashMap<>();
-
         for (Map.Entry<String, Map<Permissible, Boolean>> ent : entrySet()) {
-            if (ent.getValue() instanceof LPSubscriptionValueMap) {
+            map.put(ent.getKey(), new WeakHashMap<>(((LPSubscriptionValueMap) ent.getValue()).backing));
-                map.put(ent.getKey(), ((LPSubscriptionValueMap) ent.getValue()).backing);
-            } else {
-                map.put(ent.getKey(), ent.getValue());
         }
-        }
-
         return map;
     }
 
@@ -159,7 +153,7 @@ public final class LuckPermsSubscriptionMap extends HashMap<String, Map<Permissi
 
         private LPSubscriptionValueMap(String permission, Map<Permissible, Boolean> backing) {
             this.permission = permission;
-            this.backing = new WeakHashMap<>(backing);
+            this.backing = Collections.synchronizedMap(new WeakHashMap<>(backing));
 
             // remove all players from the map
             this.backing.keySet().removeIf(p -> p instanceof Player);
@@ -167,7 +161,7 @@ public final class LuckPermsSubscriptionMap extends HashMap<String, Map<Permissi
 
         public LPSubscriptionValueMap(String permission) {
             this.permission = permission;
-            this.backing = new WeakHashMap<>();
+            this.backing = Collections.synchronizedMap(new WeakHashMap<>());
         }
 
         @Override
@@ -223,8 +217,13 @@ public final class LuckPermsSubscriptionMap extends HashMap<String, Map<Permissi
                     .filter(player -> player.hasPermission(this.permission) || player.isPermissionSet(this.permission))
                     .collect(Collectors.toSet());
 
+            ImmutableSet<Permissible> backing;
+            synchronized (this.backing) {
+                backing = ImmutableSet.copyOf(this.backing.keySet());
+            }
+
             // then combine the players with the backing map
-            return Sets.union(players, this.backing.keySet());
+            return Sets.union(players, backing);
         }
 
         @Override

nukkit/src/main/java/me/lucko/luckperms/nukkit/inject/server/LuckPermsSubscriptionMap.java

@@ -25,6 +25,7 @@
 
 package me.lucko.luckperms.nukkit.inject.server;
 
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
 
 import me.lucko.luckperms.nukkit.LPNukkitPlugin;
@@ -70,8 +71,10 @@ public final class LuckPermsSubscriptionMap extends HashMap<String, Set<Permissi
     final LPNukkitPlugin plugin;
 
     public LuckPermsSubscriptionMap(LPNukkitPlugin plugin, Map<String, Set<Permissible>> existingData) {
-        super(existingData);
         this.plugin = plugin;
+        for (Entry<String, Set<Permissible>> entry : existingData.entrySet()) {
+            super.put(entry.getKey(), new LPSubscriptionValueSet(entry.getKey(), entry.getValue()));
+        }
     }
 
     /*
@@ -93,16 +96,12 @@ public final class LuckPermsSubscriptionMap extends HashMap<String, Set<Permissi
         String permission = ((String) key);
 
         Set<Permissible> result = super.get(key);
-
         if (result == null) {
             // calculate a new map - always!
             result = new LPSubscriptionValueSet(permission);
             super.put(permission, result);
-        } else if (!(result instanceof LPSubscriptionValueSet)) {
-            // ensure return type is a LPSubscriptionMap
-            result = new LPSubscriptionValueSet(permission, result);
-            super.put(permission, result);
         }
+
         return result;
     }
 
@@ -132,17 +131,12 @@ public final class LuckPermsSubscriptionMap extends HashMap<String, Set<Permissi
      */
     public Map<String, Set<Permissible>> detach() {
         Map<String, Set<Permissible>> map = new HashMap<>();
-
         for (Map.Entry<String, Set<Permissible>> ent : entrySet()) {
-            if (ent.getValue() instanceof LPSubscriptionValueSet) {
             Set<Permissible> backing = ((LPSubscriptionValueSet) ent.getValue()).backing;
-                Set<Permissible> copy; (copy = Collections.newSetFromMap(new WeakHashMap<>(backing.size()))).addAll(backing);
+            Set<Permissible> copy = Collections.newSetFromMap(new WeakHashMap<>(backing.size()));
+            copy.addAll(backing);
             map.put(ent.getKey(), copy);
-            } else {
-                map.put(ent.getKey(), ent.getValue());
         }
-        }
-
         return map;
     }
 
@@ -159,7 +153,7 @@ public final class LuckPermsSubscriptionMap extends HashMap<String, Set<Permissi
 
         private LPSubscriptionValueSet(String permission, Set<Permissible> content) {
             this.permission = permission;
-            this.backing = Collections.newSetFromMap(new WeakHashMap<>());
+            this.backing = Collections.synchronizedSet(Collections.newSetFromMap(new WeakHashMap<>()));
             
             if (content != null) {
                 this.backing.addAll(content);
@@ -173,13 +167,18 @@ public final class LuckPermsSubscriptionMap extends HashMap<String, Set<Permissi
             this(permission, null);
         }
 
-        private Sets.SetView<Permissible> getContentView() {
+        private Set<Permissible> getContentView() {
             // gather players (LPPermissibles)
             Set<Permissible> players = LuckPermsSubscriptionMap.this.plugin.getBootstrap().getServer().getOnlinePlayers().values().stream()
                     .filter(player -> player.hasPermission(this.permission) || player.isPermissionSet(this.permission))
                     .collect(Collectors.toSet());
 
-            return Sets.union(players, this.backing);
+            ImmutableSet<Permissible> backing;
+            synchronized (this.backing) {
+                backing = ImmutableSet.copyOf(this.backing);
+            }
+
+            return Sets.union(players, backing);
         }
 
         @Override