From cab15d39c3836a08187af82f791de9b939968ae5 Mon Sep 17 00:00:00 2001 From: Luck Date: Tue, 21 Apr 2020 20:06:52 +0100 Subject: [PATCH] Make the injected LP permSubs replacement value maps thread safe --- .../server/LuckPermsSubscriptionMap.java | 33 +++++++++-------- .../server/LuckPermsSubscriptionMap.java | 35 +++++++++---------- 2 files changed, 33 insertions(+), 35 deletions(-) diff --git a/bukkit/src/main/java/me/lucko/luckperms/bukkit/inject/server/LuckPermsSubscriptionMap.java b/bukkit/src/main/java/me/lucko/luckperms/bukkit/inject/server/LuckPermsSubscriptionMap.java index ed05a02f3..85a3eedfa 100644 --- a/bukkit/src/main/java/me/lucko/luckperms/bukkit/inject/server/LuckPermsSubscriptionMap.java +++ b/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> existingData) { - super(existingData); this.plugin = plugin; + for (Entry> entry : existingData.entrySet()) { + super.put(entry.getKey(), new LPSubscriptionValueMap(entry.getKey(), entry.getValue())); + } } /* @@ -94,17 +98,13 @@ public final class LuckPermsSubscriptionMap extends HashMap 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> detach() { Map> map = new HashMap<>(); - for (Map.Entry> ent : entrySet()) { - if (ent.getValue() instanceof LPSubscriptionValueMap) { - map.put(ent.getKey(), ((LPSubscriptionValueMap) ent.getValue()).backing); - } else { - map.put(ent.getKey(), ent.getValue()); - } + map.put(ent.getKey(), new WeakHashMap<>(((LPSubscriptionValueMap) ent.getValue()).backing)); } - return map; } @@ -159,7 +153,7 @@ public final class LuckPermsSubscriptionMap extends HashMap 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(); + this.backing = Collections.synchronizedMap(new WeakHashMap<>()); } @Override @@ -223,8 +217,13 @@ public final class LuckPermsSubscriptionMap extends HashMap player.hasPermission(this.permission) || player.isPermissionSet(this.permission)) .collect(Collectors.toSet()); + ImmutableSet 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 diff --git a/nukkit/src/main/java/me/lucko/luckperms/nukkit/inject/server/LuckPermsSubscriptionMap.java b/nukkit/src/main/java/me/lucko/luckperms/nukkit/inject/server/LuckPermsSubscriptionMap.java index d95e84c3c..8727f192f 100644 --- a/nukkit/src/main/java/me/lucko/luckperms/nukkit/inject/server/LuckPermsSubscriptionMap.java +++ b/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> existingData) { - super(existingData); this.plugin = plugin; + for (Entry> entry : existingData.entrySet()) { + super.put(entry.getKey(), new LPSubscriptionValueSet(entry.getKey(), entry.getValue())); + } } /* @@ -93,16 +96,12 @@ public final class LuckPermsSubscriptionMap extends HashMap 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> detach() { Map> map = new HashMap<>(); - for (Map.Entry> ent : entrySet()) { - if (ent.getValue() instanceof LPSubscriptionValueSet) { - Set backing = ((LPSubscriptionValueSet) ent.getValue()).backing; - Set copy; (copy = Collections.newSetFromMap(new WeakHashMap<>(backing.size()))).addAll(backing); - map.put(ent.getKey(), copy); - } else { - map.put(ent.getKey(), ent.getValue()); - } + Set backing = ((LPSubscriptionValueSet) ent.getValue()).backing; + Set copy = Collections.newSetFromMap(new WeakHashMap<>(backing.size())); + copy.addAll(backing); + map.put(ent.getKey(), copy); } - return map; } @@ -159,7 +153,7 @@ public final class LuckPermsSubscriptionMap extends HashMap 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 getContentView() { + private Set getContentView() { // gather players (LPPermissibles) Set 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 backing; + synchronized (this.backing) { + backing = ImmutableSet.copyOf(this.backing); + } + + return Sets.union(players, backing); } @Override