Prevent some shutdown exceptions - closes #136

bukkit/src/main/java/me/lucko/luckperms/bukkit/BukkitListener.java

@@ -116,7 +116,7 @@ class BukkitListener extends AbstractListener implements Listener {
         final Player player = e.getPlayer();
 
         // Remove the custom permissible
-        Injector.unInject(player, true);
+        Injector.unInject(player, true, true);
 
         // Handle auto op
         if (plugin.getConfiguration().isAutoOp()) {

bukkit/src/main/java/me/lucko/luckperms/bukkit/LPBukkitPlugin.java

@@ -82,6 +82,7 @@ import org.bukkit.plugin.java.JavaPlugin;
 import java.io.File;
 import java.io.InputStream;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -91,14 +92,19 @@ import java.util.Set;
 import java.util.UUID;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.Executor;
+import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 
 @Getter
 public class LPBukkitPlugin extends JavaPlugin implements LuckPermsPlugin {
     private Set<UUID> ignoringLogs;
+    private Set<Runnable> shutdownHooks;
     private Executor syncExecutor;
     private Executor asyncExecutor;
+    private Executor asyncBukkitExecutor;
+    private ExecutorService asyncLpExecutor;
     private VaultHook vaultHook = null;
     private LPConfiguration configuration;
     private UserManager userManager;
@@ -126,16 +132,19 @@ public class LPBukkitPlugin extends JavaPlugin implements LuckPermsPlugin {
 
     @Override
     public void onEnable() {
-        // Used whilst the server is still starting
+        // Used whilst the plugin is enabling / disabling / disabled
-        asyncExecutor = Executors.newCachedThreadPool();
+        asyncLpExecutor = Executors.newCachedThreadPool();
+        asyncBukkitExecutor = r -> getServer().getScheduler().runTaskAsynchronously(this, r);
+
+        asyncExecutor = asyncLpExecutor;
         syncExecutor = r -> getServer().getScheduler().runTask(this, r);
-        Executor bukkitAsyncExecutor = r -> getServer().getScheduler().runTaskAsynchronously(this, r);
 
         log = LogFactory.wrap(getLogger());
         ignoringLogs = ConcurrentHashMap.newKeySet();
-        debugHandler = new DebugHandler(bukkitAsyncExecutor, getVersion());
+        shutdownHooks = Collections.synchronizedSet(new HashSet<>());
+        debugHandler = new DebugHandler(asyncBukkitExecutor, getVersion());
         senderFactory = new BukkitSenderFactory(this);
-        permissionCache = new PermissionCache(bukkitAsyncExecutor);
+        permissionCache = new PermissionCache(asyncBukkitExecutor);
 
         getLog().info("Loading configuration...");
         configuration = new BukkitConfig(this);
@@ -270,7 +279,7 @@ public class LPBukkitPlugin extends JavaPlugin implements LuckPermsPlugin {
 
         // replace the temporary executor when the Bukkit one starts
         getServer().getScheduler().runTaskAsynchronously(this, () -> {
-            asyncExecutor = bukkitAsyncExecutor;
+            asyncExecutor = asyncBukkitExecutor;
         });
 
         // Load any online users (in the case of a reload)
@@ -297,14 +306,19 @@ public class LPBukkitPlugin extends JavaPlugin implements LuckPermsPlugin {
 
     @Override
     public void onDisable() {
+        // Switch back to the LP executor, the bukkit one won't allow new tasks
+        asyncExecutor = asyncLpExecutor;
+
         started = false;
 
+        shutdownHooks.forEach(Runnable::run);
+
         defaultsProvider.close();
         permissionCache.setShutdown(true);
         debugHandler.setShutdown(true);
 
         for (Player player : getServer().getOnlinePlayers()) {
-            Injector.unInject(player, false);
+            Injector.unInject(player, false, false);
             if (getConfiguration().isAutoOp()) {
                 player.setOp(false);
             }
@@ -332,14 +346,20 @@ public class LPBukkitPlugin extends JavaPlugin implements LuckPermsPlugin {
             vaultHook.unhook(this);
         }
 
+        // wait for executor
+        asyncLpExecutor.shutdown();
+        try {
+            asyncLpExecutor.awaitTermination(30, TimeUnit.SECONDS);
+        } catch (InterruptedException e) {
+            e.printStackTrace();
+        }
+
         // 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;
@@ -615,6 +635,11 @@ public class LPBukkitPlugin extends JavaPlugin implements LuckPermsPlugin {
         return getServer().getPluginManager().isPluginEnabled(name);
     }
 
+    @Override
+    public void addShutdownHook(Runnable r) {
+        shutdownHooks.add(r);
+    }
+
     private void registerPermissions(PermissionDefault def) {
         PluginManager pm = getServer().getPluginManager();
 

bukkit/src/main/java/me/lucko/luckperms/bukkit/inject/Injector.java

@@ -88,13 +88,16 @@ public class Injector {
         }
     }
 
-    public static boolean unInject(Player player, boolean dummy) {
+    public static boolean unInject(Player player, boolean dummy, boolean unsubscribe) {
         try {
             PermissibleBase permissible = (PermissibleBase) HUMAN_ENTITY_FIELD.get(player);
             if (permissible instanceof LPPermissible) {
 
                 permissible.clearPermissions();
+
+                if (unsubscribe) {
                     ((LPPermissible) permissible).unsubscribeFromAllAsync();
+                }
 
                 if (dummy) {
                     HUMAN_ENTITY_FIELD.set(player, new DummyPermissibleBase());

bungee/src/main/java/me/lucko/luckperms/bungee/LPBungeePlugin.java

@@ -68,6 +68,7 @@ import net.md_5.bungee.api.plugin.Plugin;
 
 import java.io.File;
 import java.io.InputStream;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -80,6 +81,7 @@ import java.util.stream.Collectors;
 @Getter
 public class LPBungeePlugin extends Plugin implements LuckPermsPlugin {
     private final Set<UUID> ignoringLogs = ConcurrentHashMap.newKeySet();
+    private final Set<Runnable> shutdownHooks = Collections.synchronizedSet(new HashSet<>());
     private Executor executor;
     private LPConfiguration configuration;
     private UserManager userManager;
@@ -201,6 +203,8 @@ public class LPBungeePlugin extends Plugin implements LuckPermsPlugin {
 
     @Override
     public void onDisable() {
+        shutdownHooks.forEach(Runnable::run);
+
         getLog().info("Closing datastore...");
         storage.shutdown();
 
@@ -211,6 +215,9 @@ public class LPBungeePlugin extends Plugin implements LuckPermsPlugin {
 
         getLog().info("Unregistering API...");
         ApiHandler.unregisterProvider();
+
+        getProxy().getScheduler().cancel(this);
+        getProxy().getPluginManager().unregisterListeners(this);
     }
 
     @Override
@@ -344,9 +351,12 @@ public class LPBungeePlugin extends Plugin implements LuckPermsPlugin {
     @Override
     public boolean isPluginLoaded(String name) {
         return getProxy().getPluginManager().getPlugins().stream()
-                .filter(p -> p.getDescription().getName().equalsIgnoreCase(name))
+                .anyMatch(p -> p.getDescription().getName().equalsIgnoreCase(name));
-                .findAny()
+    }
-                .isPresent();
+
+    @Override
+    public void addShutdownHook(Runnable r) {
+        shutdownHooks.add(r);
     }
 
     @Override

common/src/main/java/me/lucko/luckperms/common/LuckPermsPlugin.java

@@ -387,6 +387,12 @@ public interface LuckPermsPlugin {
      */
     BufferedRequest<Void> getUpdateTaskBuffer();
 
+    /**
+     * Adds a runnable to be called when the plugin disables
+     * @param r the runnable to run
+     */
+    void addShutdownHook(Runnable r);
+
     /**
      * Called at the end of the sync task.
      */

sponge/src/main/java/me/lucko/luckperms/sponge/LPSpongePlugin.java

@@ -86,6 +86,7 @@ import org.spongepowered.api.scheduler.AsynchronousExecutor;
 import org.spongepowered.api.scheduler.Scheduler;
 import org.spongepowered.api.scheduler.SpongeExecutorService;
 import org.spongepowered.api.scheduler.SynchronousExecutor;
+import org.spongepowered.api.scheduler.Task;
 import org.spongepowered.api.service.permission.PermissionDescription;
 import org.spongepowered.api.service.permission.PermissionService;
 import org.spongepowered.api.service.permission.Subject;
@@ -96,6 +97,7 @@ import java.io.File;
 import java.io.InputStream;
 import java.nio.file.Path;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Optional;
@@ -111,6 +113,7 @@ import java.util.stream.StreamSupport;
 public class LPSpongePlugin implements LuckPermsPlugin {
 
     private final Set<UUID> ignoringLogs = ConcurrentHashMap.newKeySet();
+    private final Set<Runnable> shutdownHooks = Collections.synchronizedSet(new HashSet<>());
 
     @Inject
     private Logger logger;
@@ -251,18 +254,23 @@ public class LPSpongePlugin implements LuckPermsPlugin {
         // schedule update tasks
         int mins = getConfiguration().getSyncTime();
         if (mins > 0) {
-            scheduler.createTaskBuilder().async().interval(mins, TimeUnit.MINUTES).execute(new UpdateTask(this))
+            Task t = scheduler.createTaskBuilder().async().interval(mins, TimeUnit.MINUTES).execute(new UpdateTask(this))
                     .submit(LPSpongePlugin.this);
+            addShutdownHook(t::cancel);
         }
 
         // run an update instantly.
         updateTaskBuffer.requestDirectly();
 
         // register tasks
-        scheduler.createTaskBuilder().async().intervalTicks(60L).execute(new ExpireTemporaryTask(this)).submit(this);
+        Task t2 = scheduler.createTaskBuilder().async().intervalTicks(60L).execute(new ExpireTemporaryTask(this)).submit(this);
-        scheduler.createTaskBuilder().async().intervalTicks(2400L).execute(new CacheHousekeepingTask(this)).submit(this);
+        Task t3 = scheduler.createTaskBuilder().async().intervalTicks(2400L).execute(new CacheHousekeepingTask(this)).submit(this);
-        scheduler.createTaskBuilder().async().intervalTicks(2400L).execute(new ServiceCacheHousekeepingTask(service)).submit(this);
+        Task t4 = scheduler.createTaskBuilder().async().intervalTicks(2400L).execute(new ServiceCacheHousekeepingTask(service)).submit(this);
-        scheduler.createTaskBuilder().async().intervalTicks(2400L).execute(() -> userManager.performCleanup()).submit(this);
+        Task t5 = scheduler.createTaskBuilder().async().intervalTicks(2400L).execute(() -> userManager.performCleanup()).submit(this);
+        addShutdownHook(t2::cancel);
+        addShutdownHook(t3::cancel);
+        addShutdownHook(t4::cancel);
+        addShutdownHook(t5::cancel);
 
         getLog().info("Successfully loaded.");
     }
@@ -441,6 +449,11 @@ public class LPSpongePlugin implements LuckPermsPlugin {
         return game.getPluginManager().isLoaded(name);
     }
 
+    @Override
+    public void addShutdownHook(Runnable r) {
+        shutdownHooks.add(r);
+    }
+
     @Override
     public void doAsync(Runnable r) {
         scheduler.createTaskBuilder().async().execute(r).submit(this);