From b35f3b4375a30a7cbbad5c66aa431e6ac4376d88 Mon Sep 17 00:00:00 2001 From: Luck Date: Fri, 23 Dec 2016 11:28:42 +0000 Subject: [PATCH] Properly cleanup in the case of a reload - towards #100 --- .../luckperms/bukkit/BukkitListener.java | 2 +- .../luckperms/bukkit/LPBukkitPlugin.java | 80 ++++++++++++++++++- .../luckperms/bukkit/inject/Injector.java | 57 ++++++++++--- .../bukkit/model/DefaultsProvider.java | 5 ++ .../luckperms/bukkit/model/LPPermissible.java | 19 ++++- .../common/utils/AbstractListener.java | 2 +- .../luckperms/common/utils/DebugHandler.java | 7 ++ .../common/utils/PermissionCache.java | 11 ++- 8 files changed, 163 insertions(+), 20 deletions(-) diff --git a/bukkit/src/main/java/me/lucko/luckperms/bukkit/BukkitListener.java b/bukkit/src/main/java/me/lucko/luckperms/bukkit/BukkitListener.java index dc1a7a926..c320da72e 100644 --- a/bukkit/src/main/java/me/lucko/luckperms/bukkit/BukkitListener.java +++ b/bukkit/src/main/java/me/lucko/luckperms/bukkit/BukkitListener.java @@ -117,7 +117,7 @@ class BukkitListener extends AbstractListener implements Listener { plugin.getWorldCalculator().getWorldCache().remove(internal); // Remove the custom permissible - Injector.unInject(player); + Injector.unInject(player, true); // Handle auto op if (plugin.getConfiguration().isAutoOp()) { diff --git a/bukkit/src/main/java/me/lucko/luckperms/bukkit/LPBukkitPlugin.java b/bukkit/src/main/java/me/lucko/luckperms/bukkit/LPBukkitPlugin.java index c17c8363c..050875618 100644 --- a/bukkit/src/main/java/me/lucko/luckperms/bukkit/LPBukkitPlugin.java +++ b/bukkit/src/main/java/me/lucko/luckperms/bukkit/LPBukkitPlugin.java @@ -34,8 +34,10 @@ import me.lucko.luckperms.api.PlatformType; import me.lucko.luckperms.api.context.ContextSet; import me.lucko.luckperms.api.context.MutableContextSet; import me.lucko.luckperms.bukkit.calculators.AutoOPListener; +import me.lucko.luckperms.bukkit.inject.Injector; import me.lucko.luckperms.bukkit.model.ChildPermissionProvider; import me.lucko.luckperms.bukkit.model.DefaultsProvider; +import me.lucko.luckperms.bukkit.model.LPPermissible; import me.lucko.luckperms.bukkit.vault.VaultHook; import me.lucko.luckperms.common.LuckPermsPlugin; import me.lucko.luckperms.common.api.ApiProvider; @@ -72,6 +74,7 @@ import me.lucko.luckperms.common.utils.PermissionCache; import org.bukkit.World; import org.bukkit.command.PluginCommand; import org.bukkit.entity.Player; +import org.bukkit.event.HandlerList; import org.bukkit.permissions.PermissionDefault; import org.bukkit.plugin.PluginManager; import org.bukkit.plugin.ServicePriority; @@ -92,7 +95,7 @@ import java.util.stream.Collectors; @Getter public class LPBukkitPlugin extends JavaPlugin implements LuckPermsPlugin { - private final Set ignoringLogs = ConcurrentHashMap.newKeySet(); + private Set ignoringLogs; private Executor syncExecutor; private Executor asyncExecutor; private VaultHook vaultHook = null; @@ -103,6 +106,7 @@ public class LPBukkitPlugin extends JavaPlugin implements LuckPermsPlugin { private Storage storage; private RedisMessaging redisMessaging = null; private UuidCache uuidCache; + private BukkitListener listener; private ApiProvider apiProvider; private Logger log; private Importer importer; @@ -127,6 +131,7 @@ public class LPBukkitPlugin extends JavaPlugin implements LuckPermsPlugin { Executor bukkitAsyncExecutor = r -> getServer().getScheduler().runTaskAsynchronously(this, r); log = LogFactory.wrap(getLogger()); + ignoringLogs = ConcurrentHashMap.newKeySet(); debugHandler = new DebugHandler(bukkitAsyncExecutor, getVersion()); senderFactory = new BukkitSenderFactory(this); permissionCache = new PermissionCache(bukkitAsyncExecutor); @@ -168,7 +173,8 @@ public class LPBukkitPlugin extends JavaPlugin implements LuckPermsPlugin { // register events PluginManager pm = getServer().getPluginManager(); - pm.registerEvents(new BukkitListener(this), this); + listener = new BukkitListener(this); + pm.registerEvents(listener, this); // initialise datastore storage = StorageFactory.getInstance(this, StorageType.H2); @@ -250,7 +256,7 @@ public class LPBukkitPlugin extends JavaPlugin implements LuckPermsPlugin { int mins = getConfiguration().getSyncTime(); if (mins > 0) { long ticks = mins * 60 * 20; - getServer().getScheduler().runTaskTimerAsynchronously(this, () -> updateTaskBuffer.request(), 20L, ticks); + getServer().getScheduler().runTaskTimerAsynchronously(this, () -> updateTaskBuffer.request(), 40L, ticks); } // run an update instantly. @@ -271,6 +277,24 @@ public class LPBukkitPlugin extends JavaPlugin implements LuckPermsPlugin { asyncExecutor = bukkitAsyncExecutor; }); + // Load any online users (in the case of a reload) + for (Player player : getServer().getOnlinePlayers()) { + doAsync(() -> { + listener.onAsyncLogin(player.getUniqueId(), player.getName()); + User user = getUserManager().get(getUuidCache().getUUID(player.getUniqueId())); + if (user != null) { + doSync(() -> { + try { + LPPermissible lpPermissible = new LPPermissible(player, user, this); + Injector.inject(player, lpPermissible); + } catch (Throwable t) { + t.printStackTrace(); + } + }); + } + }); + } + started = true; getLog().info("Successfully loaded."); } @@ -278,6 +302,24 @@ public class LPBukkitPlugin extends JavaPlugin implements LuckPermsPlugin { @Override public void onDisable() { started = false; + + defaultsProvider.close(); + permissionCache.setShutdown(true); + debugHandler.setShutdown(true); + + for (Player player : getServer().getOnlinePlayers()) { + Injector.unInject(player, false); + if (getConfiguration().isAutoOp()) { + player.setOp(false); + } + + final User user = getUserManager().get(getUuidCache().getUUID(player.getUniqueId())); + if (user != null) { + user.unregisterData(); + getUserManager().unload(user); + } + } + getLog().info("Closing datastore..."); storage.shutdown(); @@ -293,6 +335,38 @@ public class LPBukkitPlugin extends JavaPlugin implements LuckPermsPlugin { if (vaultHook != null) { vaultHook.unhook(this); } + + // Bukkit will do this again when #onDisable completes, but we do it early to prevent NPEs elsewhere. + getServer().getScheduler().cancelTasks(this); + HandlerList.unregisterAll(this); + + // Null everything + ignoringLogs = null; + syncExecutor = null; + asyncExecutor = null; + vaultHook = null; + configuration = null; + userManager = null; + groupManager = null; + trackManager = null; + storage = null; + redisMessaging = null; + uuidCache = null; + listener = null; + apiProvider = null; + log = null; + importer = null; + defaultsProvider = null; + childPermissionProvider = null; + localeManager = null; + cachedStateManager = null; + contextManager = null; + worldCalculator = null; + calculatorFactory = null; + updateTaskBuffer = null; + debugHandler = null; + senderFactory = null; + permissionCache = null; } public void tryVaultHook(boolean force) { diff --git a/bukkit/src/main/java/me/lucko/luckperms/bukkit/inject/Injector.java b/bukkit/src/main/java/me/lucko/luckperms/bukkit/inject/Injector.java index 65e4116f6..3a3562ec8 100644 --- a/bukkit/src/main/java/me/lucko/luckperms/bukkit/inject/Injector.java +++ b/bukkit/src/main/java/me/lucko/luckperms/bukkit/inject/Injector.java @@ -28,9 +28,11 @@ import me.lucko.luckperms.bukkit.model.LPPermissible; import org.bukkit.Bukkit; import org.bukkit.entity.Player; -import org.bukkit.permissions.Permissible; +import org.bukkit.permissions.PermissibleBase; +import org.bukkit.permissions.PermissionAttachment; import java.lang.reflect.Field; +import java.util.List; import java.util.Map; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; @@ -41,21 +43,42 @@ import java.util.concurrent.ConcurrentHashMap; @UtilityClass public class Injector { private static final Map INJECTED_PERMISSIBLES = new ConcurrentHashMap<>(); + private static Field HUMAN_ENTITY_FIELD; + private static Field PERMISSIBLEBASE_ATTACHMENTS; static { try { HUMAN_ENTITY_FIELD = Class.forName(getVersionedClassName("entity.CraftHumanEntity")).getDeclaredField("perm"); HUMAN_ENTITY_FIELD.setAccessible(true); + + PERMISSIBLEBASE_ATTACHMENTS = PermissibleBase.class.getDeclaredField("attachments"); + PERMISSIBLEBASE_ATTACHMENTS.setAccessible(true); + } catch (Exception e) { e.printStackTrace(); } } - public static boolean inject(Player player, LPPermissible permissible) { + public static boolean inject(Player player, LPPermissible lpPermissible) { try { - HUMAN_ENTITY_FIELD.set(player, permissible); - INJECTED_PERMISSIBLES.put(player.getUniqueId(), permissible); + PermissibleBase existing = (PermissibleBase) HUMAN_ENTITY_FIELD.get(player); + if (existing instanceof LPPermissible) { + // uh oh + throw new IllegalStateException(); + } + + // Move attachments over from the old permissible. + List attachments = (List) PERMISSIBLEBASE_ATTACHMENTS.get(existing); + lpPermissible.addAttachments(attachments); + attachments.clear(); + existing.clearPermissions(); + + lpPermissible.recalculatePermissions(); + lpPermissible.setOldPermissible(existing); + + HUMAN_ENTITY_FIELD.set(player, lpPermissible); + INJECTED_PERMISSIBLES.put(player.getUniqueId(), lpPermissible); return true; } catch (Exception e) { e.printStackTrace(); @@ -63,14 +86,28 @@ public class Injector { } } - public static boolean unInject(Player player) { + public static boolean unInject(Player player, boolean dummy) { try { - Permissible permissible = (Permissible) HUMAN_ENTITY_FIELD.get(player); + PermissibleBase permissible = (PermissibleBase) HUMAN_ENTITY_FIELD.get(player); if (permissible instanceof LPPermissible) { - /* The player is most likely leaving. Bukkit will attempt to call #clearPermissions, so we cannot set to null. - However, there's no need to re-inject a real PermissibleBase, so we just inject a dummy instead. - This saves tick time, pointlessly recalculating defaults when the instance will never be used. */ - HUMAN_ENTITY_FIELD.set(player, new DummyPermissibleBase()); + if (dummy) { + HUMAN_ENTITY_FIELD.set(player, new DummyPermissibleBase()); + } else { + LPPermissible lpp = ((LPPermissible) permissible); + List attachments = lpp.getAttachments(); + + PermissibleBase newPb = lpp.getOldPermissible(); + if (newPb == null) { + newPb = new PermissibleBase(player); + } + + List newAttachments = (List) PERMISSIBLEBASE_ATTACHMENTS.get(newPb); + newAttachments.addAll(attachments); + attachments.clear(); + lpp.clearPermissions(); + + HUMAN_ENTITY_FIELD.set(player, newPb); + } } INJECTED_PERMISSIBLES.remove(player.getUniqueId()); return true; diff --git a/bukkit/src/main/java/me/lucko/luckperms/bukkit/model/DefaultsProvider.java b/bukkit/src/main/java/me/lucko/luckperms/bukkit/model/DefaultsProvider.java index 62e3ad734..e885fb08f 100644 --- a/bukkit/src/main/java/me/lucko/luckperms/bukkit/model/DefaultsProvider.java +++ b/bukkit/src/main/java/me/lucko/luckperms/bukkit/model/DefaultsProvider.java @@ -111,6 +111,11 @@ public class DefaultsProvider { nonOpDefaults = ImmutableMap.copyOf(builder); } + public void close() { + unregisterDefaults(opDefaults, opDummy); + unregisterDefaults(nonOpDefaults, nonOpDummy); + } + public Tristate hasDefault(String permission, boolean isOp) { Map map = isOp ? opDefaults : nonOpDefaults; diff --git a/bukkit/src/main/java/me/lucko/luckperms/bukkit/model/LPPermissible.java b/bukkit/src/main/java/me/lucko/luckperms/bukkit/model/LPPermissible.java index 002fe7664..77fdd0603 100644 --- a/bukkit/src/main/java/me/lucko/luckperms/bukkit/model/LPPermissible.java +++ b/bukkit/src/main/java/me/lucko/luckperms/bukkit/model/LPPermissible.java @@ -24,6 +24,7 @@ package me.lucko.luckperms.bukkit.model; import lombok.Getter; import lombok.NonNull; +import lombok.Setter; import me.lucko.luckperms.api.Contexts; import me.lucko.luckperms.api.Tristate; @@ -39,12 +40,13 @@ import org.bukkit.permissions.PermissionAttachmentInfo; import org.bukkit.permissions.PermissionRemovedExecutor; import org.bukkit.plugin.Plugin; -import java.util.HashMap; +import java.util.Collections; import java.util.HashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.logging.Level; import java.util.stream.Collectors; @@ -61,10 +63,15 @@ public class LPPermissible extends PermissibleBase { private final LPBukkitPlugin plugin; + @Getter + @Setter + private PermissibleBase oldPermissible = null; + // Attachment stuff. @Getter - private final Map attachmentPermissions = new HashMap<>(); - private final List attachments = new LinkedList<>(); + private final Map attachmentPermissions = new ConcurrentHashMap<>(); + @Getter + private final List attachments = Collections.synchronizedList(new LinkedList<>()); public LPPermissible(@NonNull Player parent, User user, LPBukkitPlugin plugin) { super(parent); @@ -75,6 +82,12 @@ public class LPPermissible extends PermissibleBase { recalculatePermissions(); } + public void addAttachments(List attachments) { + for (PermissionAttachment attachment : attachments) { + this.attachments.add(attachment); + } + } + public Contexts calculateContexts() { return new Contexts( plugin.getContextManager().getApplicableContext(parent), diff --git a/common/src/main/java/me/lucko/luckperms/common/utils/AbstractListener.java b/common/src/main/java/me/lucko/luckperms/common/utils/AbstractListener.java index 4fd9a5cbe..1cd07ff55 100644 --- a/common/src/main/java/me/lucko/luckperms/common/utils/AbstractListener.java +++ b/common/src/main/java/me/lucko/luckperms/common/utils/AbstractListener.java @@ -39,7 +39,7 @@ import java.util.UUID; public class AbstractListener { private final LuckPermsPlugin plugin; - protected void onAsyncLogin(UUID u, String username) { + public void onAsyncLogin(UUID u, String username) { final long startTime = System.currentTimeMillis(); final UuidCache cache = plugin.getUuidCache(); diff --git a/common/src/main/java/me/lucko/luckperms/common/utils/DebugHandler.java b/common/src/main/java/me/lucko/luckperms/common/utils/DebugHandler.java index 959a4cf3f..cd5003cdd 100644 --- a/common/src/main/java/me/lucko/luckperms/common/utils/DebugHandler.java +++ b/common/src/main/java/me/lucko/luckperms/common/utils/DebugHandler.java @@ -58,6 +58,9 @@ public class DebugHandler { private final Map> listeners; private final Queue queue; + @Setter + private boolean shutdown = false; + public DebugHandler(Executor executor, String pluginVersion) { this.pluginVersion = "v" + pluginVersion; listeners = new ConcurrentHashMap<>(); @@ -69,6 +72,10 @@ public class DebugHandler { handleOutput(e.getChecked(), e.getNode(), e.getValue()); } + if (shutdown) { + return; + } + try { Thread.sleep(200); } catch (InterruptedException ignored) {} diff --git a/common/src/main/java/me/lucko/luckperms/common/utils/PermissionCache.java b/common/src/main/java/me/lucko/luckperms/common/utils/PermissionCache.java index 6cd7b7e8e..63a6c0849 100644 --- a/common/src/main/java/me/lucko/luckperms/common/utils/PermissionCache.java +++ b/common/src/main/java/me/lucko/luckperms/common/utils/PermissionCache.java @@ -24,6 +24,7 @@ package me.lucko.luckperms.common.utils; import lombok.Getter; import lombok.NonNull; +import lombok.Setter; import com.google.common.base.Splitter; @@ -41,6 +42,9 @@ public class PermissionCache { private final Node rootNode; private final Queue queue; + @Setter + private boolean shutdown = false; + public PermissionCache(Executor executor) { rootNode = new Node(); queue = new ConcurrentLinkedQueue<>(); @@ -51,10 +55,13 @@ public class PermissionCache { insert(e.toLowerCase()); } + if (shutdown) { + return; + } + try { Thread.sleep(5000); - } catch (InterruptedException ignored) { - } + } catch (InterruptedException ignored) {} } }); }