From cbeaaca7af5e93b1fa32856e902456e61c607707 Mon Sep 17 00:00:00 2001 From: Luck Date: Sat, 31 Mar 2018 11:11:42 +0100 Subject: [PATCH] Fix shutdown hang (#881) --- .../bukkit/BukkitSchedulerAdapter.java | 14 ++--- .../plugin/AbstractLuckPermsPlugin.java | 8 +-- .../common/treeview/PermissionVault.java | 48 ++++++----------- .../luckperms/common/utils/RepeatingTask.java | 54 +++++++++++++++++++ .../common/verbose/VerboseHandler.java | 43 +++++---------- .../nukkit/NukkitSchedulerAdapter.java | 14 ++--- 6 files changed, 96 insertions(+), 85 deletions(-) create mode 100644 common/src/main/java/me/lucko/luckperms/common/utils/RepeatingTask.java diff --git a/bukkit/src/main/java/me/lucko/luckperms/bukkit/BukkitSchedulerAdapter.java b/bukkit/src/main/java/me/lucko/luckperms/bukkit/BukkitSchedulerAdapter.java index 15b1af4bf..d260ad97e 100644 --- a/bukkit/src/main/java/me/lucko/luckperms/bukkit/BukkitSchedulerAdapter.java +++ b/bukkit/src/main/java/me/lucko/luckperms/bukkit/BukkitSchedulerAdapter.java @@ -116,15 +116,6 @@ public class BukkitSchedulerAdapter implements SchedulerAdapter { } } - - public ExecutorService asyncFallback() { - return this.asyncFallback; - } - - public Executor asyncBukkit() { - return this.asyncBukkit; - } - @Override public Executor sync() { return this.sync; @@ -135,6 +126,11 @@ public class BukkitSchedulerAdapter implements SchedulerAdapter { return this.async; } + @Override + public Executor platformAsync() { + return this.asyncBukkit; + } + public void setUseFallback(boolean useFallback) { this.useFallback = useFallback; } diff --git a/common/src/main/java/me/lucko/luckperms/common/plugin/AbstractLuckPermsPlugin.java b/common/src/main/java/me/lucko/luckperms/common/plugin/AbstractLuckPermsPlugin.java index ce022589e..fd9ddac95 100644 --- a/common/src/main/java/me/lucko/luckperms/common/plugin/AbstractLuckPermsPlugin.java +++ b/common/src/main/java/me/lucko/luckperms/common/plugin/AbstractLuckPermsPlugin.java @@ -102,8 +102,8 @@ public abstract class AbstractLuckPermsPlugin implements LuckPermsPlugin { displayBanner(getConsoleSender()); // load some utilities early - this.verboseHandler = new VerboseHandler(getBootstrap().getScheduler().platformAsync()); - this.permissionVault = new PermissionVault(getBootstrap().getScheduler().platformAsync()); + this.verboseHandler = new VerboseHandler(); + this.permissionVault = new PermissionVault(); this.logDispatcher = new LogDispatcher(this); // load configuration @@ -194,8 +194,8 @@ public abstract class AbstractLuckPermsPlugin implements LuckPermsPlugin { performEarlyDisableTasks(); // shutdown permission vault and verbose handler tasks - this.permissionVault.shutdown(); - this.verboseHandler.shutdown(); + this.permissionVault.stop(); + this.verboseHandler.stop(); // remove any hooks into the platform removePlatformHooks(); diff --git a/common/src/main/java/me/lucko/luckperms/common/treeview/PermissionVault.java b/common/src/main/java/me/lucko/luckperms/common/treeview/PermissionVault.java index 8987e3dbb..a9b401532 100644 --- a/common/src/main/java/me/lucko/luckperms/common/treeview/PermissionVault.java +++ b/common/src/main/java/me/lucko/luckperms/common/treeview/PermissionVault.java @@ -28,6 +28,8 @@ package me.lucko.luckperms.common.treeview; import com.google.common.base.Splitter; import com.google.common.collect.ImmutableSet; +import me.lucko.luckperms.common.utils.RepeatingTask; + import java.util.Collection; import java.util.List; import java.util.Map; @@ -35,13 +37,13 @@ import java.util.Queue; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedQueue; -import java.util.concurrent.Executor; +import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; /** * Stores a collection of all permissions known to the platform. */ -public class PermissionVault implements Runnable { +public class PermissionVault extends RepeatingTask { private static final Splitter DOT_SPLIT = Splitter.on('.').omitEmptyStrings(); // the root node in the tree @@ -53,15 +55,11 @@ public class PermissionVault implements Runnable { // a queue of permission strings to be processed by the tree private final Queue queue; - // if the handler should shutdown - private boolean shutdown = false; - - public PermissionVault(Executor executor) { + public PermissionVault() { + super(1000, TimeUnit.MILLISECONDS, "luckperms-permission-vault"); this.rootNode = new TreeNode(); this.knownPermissions = ConcurrentHashMap.newKeySet(3000); this.queue = new ConcurrentLinkedQueue<>(); - - executor.execute(this); } public TreeNode getRootNode() { @@ -69,27 +67,17 @@ public class PermissionVault implements Runnable { } @Override - public void run() { - while (true) { - for (String e; (e = this.queue.poll()) != null; ) { - try { - String s = e.toLowerCase(); - // only attempt an insert if we're not seen this permission before - if (this.knownPermissions.add(s)) { - insert(s); - } - } catch (Exception ex) { - ex.printStackTrace(); - } - } - - if (this.shutdown) { - return; - } - + protected void tick() { + for (String e; (e = this.queue.poll()) != null; ) { try { - Thread.sleep(5000); - } catch (InterruptedException ignored) {} + String s = e.toLowerCase(); + // only attempt an insert if we're not seen this permission before + if (this.knownPermissions.add(s)) { + insert(s); + } + } catch (Exception ex) { + ex.printStackTrace(); + } } } @@ -127,8 +115,4 @@ public class PermissionVault implements Runnable { } } - public void shutdown() { - this.shutdown = true; - } - } diff --git a/common/src/main/java/me/lucko/luckperms/common/utils/RepeatingTask.java b/common/src/main/java/me/lucko/luckperms/common/utils/RepeatingTask.java new file mode 100644 index 000000000..35e21db01 --- /dev/null +++ b/common/src/main/java/me/lucko/luckperms/common/utils/RepeatingTask.java @@ -0,0 +1,54 @@ +/* + * This file is part of LuckPerms, licensed under the MIT License. + * + * Copyright (c) lucko (Luck) + * Copyright (c) contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package me.lucko.luckperms.common.utils; + +import com.google.common.util.concurrent.ThreadFactoryBuilder; + +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; + +public abstract class RepeatingTask { + + // the executor thread + private final ScheduledExecutorService executor; + + protected RepeatingTask(long time, TimeUnit unit, String nameFormat) { + this.executor = Executors.newSingleThreadScheduledExecutor(new ThreadFactoryBuilder().setNameFormat(nameFormat).build()); + this.executor.scheduleAtFixedRate(this::tick, time, time, unit); + } + + protected abstract void tick(); + + public void stop() { + this.executor.shutdown(); + try { + this.executor.awaitTermination(10, TimeUnit.SECONDS); + } catch (InterruptedException e) { + e.printStackTrace(); + } + } +} diff --git a/common/src/main/java/me/lucko/luckperms/common/verbose/VerboseHandler.java b/common/src/main/java/me/lucko/luckperms/common/verbose/VerboseHandler.java index 75cb44d0c..35ab0cb75 100644 --- a/common/src/main/java/me/lucko/luckperms/common/verbose/VerboseHandler.java +++ b/common/src/main/java/me/lucko/luckperms/common/verbose/VerboseHandler.java @@ -28,18 +28,19 @@ package me.lucko.luckperms.common.verbose; import me.lucko.luckperms.api.Tristate; import me.lucko.luckperms.api.context.ContextSet; import me.lucko.luckperms.common.sender.Sender; +import me.lucko.luckperms.common.utils.RepeatingTask; import java.util.Map; import java.util.Queue; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedQueue; -import java.util.concurrent.Executor; +import java.util.concurrent.TimeUnit; /** * Accepts {@link CheckData} and passes it onto registered {@link VerboseListener}s. */ -public class VerboseHandler implements Runnable { +public class VerboseHandler extends RepeatingTask { // the listeners currently registered private final Map listeners; @@ -50,14 +51,10 @@ public class VerboseHandler implements Runnable { // if there are any listeners currently registered private boolean listening = false; - // if the handler should shutdown - private boolean shutdown = false; - - public VerboseHandler(Executor executor) { + public VerboseHandler() { + super(100, TimeUnit.MILLISECONDS, "luckperms-verbose"); this.listeners = new ConcurrentHashMap<>(); this.queue = new ConcurrentLinkedQueue<>(); - - executor.execute(this); } /** @@ -111,27 +108,15 @@ public class VerboseHandler implements Runnable { } @Override - public void run() { - while (true) { + protected void tick() { + // remove listeners where the sender is no longer valid + this.listeners.values().removeIf(l -> !l.getNotifiedSender().isValid()); - // remove listeners where the sender is no longer valid - this.listeners.values().removeIf(l -> !l.getNotifiedSender().isValid()); + // handle all checks in the queue + flush(); - // handle all checks in the queue - flush(); - - // break the loop if the handler has been shutdown - if (this.shutdown) { - return; - } - - // update listening state - this.listening = !this.listeners.isEmpty(); - - try { - Thread.sleep(100); - } catch (InterruptedException ignored) {} - } + // update listening state + this.listening = !this.listeners.isEmpty(); } /** @@ -144,8 +129,4 @@ public class VerboseHandler implements Runnable { } } } - - public void shutdown() { - this.shutdown = true; - } } diff --git a/nukkit/src/main/java/me/lucko/luckperms/nukkit/NukkitSchedulerAdapter.java b/nukkit/src/main/java/me/lucko/luckperms/nukkit/NukkitSchedulerAdapter.java index a06cb1f77..7b9aac3f9 100644 --- a/nukkit/src/main/java/me/lucko/luckperms/nukkit/NukkitSchedulerAdapter.java +++ b/nukkit/src/main/java/me/lucko/luckperms/nukkit/NukkitSchedulerAdapter.java @@ -116,15 +116,6 @@ public class NukkitSchedulerAdapter implements SchedulerAdapter { } } - - public ExecutorService asyncFallback() { - return this.asyncFallback; - } - - public Executor asyncNukkit() { - return this.asyncNukkit; - } - @Override public Executor sync() { return this.sync; @@ -135,6 +126,11 @@ public class NukkitSchedulerAdapter implements SchedulerAdapter { return this.async; } + @Override + public Executor platformAsync() { + return this.asyncNukkit; + } + public void setUseFallback(boolean useFallback) { this.useFallback = useFallback; }