Fix shutdown hang (#881)

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;
     }

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.verboseHandler = new VerboseHandler();
-        this.permissionVault = new PermissionVault(getBootstrap().getScheduler().platformAsync());
+        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.permissionVault.stop();
-        this.verboseHandler.shutdown();
+        this.verboseHandler.stop();
 
         // remove any hooks into the platform
         removePlatformHooks();

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<String> queue;
 
-    // if the handler should shutdown
+    public PermissionVault() {
-    private boolean shutdown = false;
+        super(1000, TimeUnit.MILLISECONDS, "luckperms-permission-vault");
-
-    public PermissionVault(Executor executor) {
         this.rootNode = new TreeNode();
         this.knownPermissions = ConcurrentHashMap.newKeySet(3000);
         this.queue = new ConcurrentLinkedQueue<>();
-
-        executor.execute(this);
     }
 
     public TreeNode getRootNode() {
@@ -69,8 +67,7 @@ public class PermissionVault implements Runnable {
     }
 
     @Override
-    public void run() {
+    protected void tick() {
-        while (true) {
         for (String e; (e = this.queue.poll()) != null; ) {
             try {
                 String s = e.toLowerCase();
@@ -82,15 +79,6 @@ public class PermissionVault implements Runnable {
                 ex.printStackTrace();
             }
         }
-
-            if (this.shutdown) {
-                return;
-            }
-
-            try {
-                Thread.sleep(5000);
-            } catch (InterruptedException ignored) {}
-        }
     }
 
     public void offer(String permission) {
@@ -127,8 +115,4 @@ public class PermissionVault implements Runnable {
         }
     }
 
-    public void shutdown() {
-        this.shutdown = true;
-    }
-
 }

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) <luck@lucko.me>
+ *  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();
+        }
+    }
+}

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<UUID, VerboseListener> 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
+    public VerboseHandler() {
-    private boolean shutdown = false;
+        super(100, TimeUnit.MILLISECONDS, "luckperms-verbose");
-
-    public VerboseHandler(Executor executor) {
         this.listeners = new ConcurrentHashMap<>();
         this.queue = new ConcurrentLinkedQueue<>();
-
-        executor.execute(this);
     }
 
     /**
@@ -111,27 +108,15 @@ public class VerboseHandler implements Runnable {
     }
 
     @Override
-    public void run() {
+    protected void tick() {
-        while (true) {
-
         // remove listeners where the sender is no longer valid
         this.listeners.values().removeIf(l -> !l.getNotifiedSender().isValid());
 
         // 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) {}
-        }
     }
 
     /**
@@ -144,8 +129,4 @@ public class VerboseHandler implements Runnable {
             }
         }
     }
-
-    public void shutdown() {
-        this.shutdown = true;
-    }
 }

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;
     }