From 10680668dbe9d48bf6108d9148721537152e0a78 Mon Sep 17 00:00:00 2001 From: Luck Date: Thu, 26 Dec 2019 20:25:14 +0000 Subject: [PATCH] Fix NPEs caused by RepeatingTask initialisation race condition --- .../plugin/AbstractLuckPermsPlugin.java | 4 +- .../common/treeview/PermissionRegistry.java | 21 +++++--- .../luckperms/common/util/RepeatingTask.java | 53 ------------------- .../common/verbose/VerboseHandler.java | 25 +++++---- 4 files changed, 30 insertions(+), 73 deletions(-) delete mode 100644 common/src/main/java/me/lucko/luckperms/common/util/RepeatingTask.java 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 4043b722e..4daa943e9 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 @@ -206,8 +206,8 @@ public abstract class AbstractLuckPermsPlugin implements LuckPermsPlugin { public final void disable() { // shutdown permission vault and verbose handler tasks - this.permissionRegistry.stop(); - this.verboseHandler.stop(); + this.permissionRegistry.close(); + this.verboseHandler.close(); // unload extensions this.extensionManager.close(); diff --git a/common/src/main/java/me/lucko/luckperms/common/treeview/PermissionRegistry.java b/common/src/main/java/me/lucko/luckperms/common/treeview/PermissionRegistry.java index f010354d4..8315993c3 100644 --- a/common/src/main/java/me/lucko/luckperms/common/treeview/PermissionRegistry.java +++ b/common/src/main/java/me/lucko/luckperms/common/treeview/PermissionRegistry.java @@ -28,8 +28,8 @@ package me.lucko.luckperms.common.treeview; import com.google.common.base.Splitter; import me.lucko.luckperms.common.plugin.scheduler.SchedulerAdapter; +import me.lucko.luckperms.common.plugin.scheduler.SchedulerTask; import me.lucko.luckperms.common.util.ImmutableCollectors; -import me.lucko.luckperms.common.util.RepeatingTask; import java.util.List; import java.util.Map; @@ -40,19 +40,20 @@ import java.util.concurrent.TimeUnit; /** * Stores a collection of all permissions known to the platform. */ -public class PermissionRegistry extends RepeatingTask { +public class PermissionRegistry implements AutoCloseable { private static final Splitter DOT_SPLIT = Splitter.on('.').omitEmptyStrings(); - // the root node in the tree + /** The root node in the tree */ private final TreeNode rootNode; - - // a queue of permission strings to be processed by the tree + /** A queue of permission strings to be added to the tree */ private final Queue queue; + /** The tick task */ + private final SchedulerTask task; public PermissionRegistry(SchedulerAdapter scheduler) { - super(scheduler, 1, TimeUnit.SECONDS); this.rootNode = new TreeNode(); this.queue = new ConcurrentLinkedQueue<>(); + this.task = scheduler.asyncRepeating(this::tick, 1, TimeUnit.SECONDS); } public TreeNode getRootNode() { @@ -70,13 +71,17 @@ public class PermissionRegistry extends RepeatingTask { this.queue.offer(permission); } - @Override - protected void tick() { + private void tick() { for (String e; (e = this.queue.poll()) != null; ) { insert(e); } } + @Override + public void close() { + this.task.cancel(); + } + public void insert(String permission) { try { doInsert(permission); diff --git a/common/src/main/java/me/lucko/luckperms/common/util/RepeatingTask.java b/common/src/main/java/me/lucko/luckperms/common/util/RepeatingTask.java deleted file mode 100644 index f8a81730e..000000000 --- a/common/src/main/java/me/lucko/luckperms/common/util/RepeatingTask.java +++ /dev/null @@ -1,53 +0,0 @@ -/* - * 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.util; - -import me.lucko.luckperms.common.plugin.scheduler.SchedulerAdapter; -import me.lucko.luckperms.common.plugin.scheduler.SchedulerTask; - -import java.util.concurrent.TimeUnit; - -public abstract class RepeatingTask { - private final SchedulerTask task; - - protected RepeatingTask(SchedulerAdapter scheduler, long time, TimeUnit unit) { - this.task = scheduler.asyncRepeating(this::run, time, unit); - } - - private void run() { - try { - tick(); - } catch (Exception e) { - e.printStackTrace(); - } - } - - protected abstract void tick(); - - public void stop() { - this.task.cancel(); - } -} 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 c8294cb37..1da03b8a7 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 @@ -27,8 +27,8 @@ package me.lucko.luckperms.common.verbose; import me.lucko.luckperms.common.calculator.result.TristateResult; import me.lucko.luckperms.common.plugin.scheduler.SchedulerAdapter; +import me.lucko.luckperms.common.plugin.scheduler.SchedulerTask; import me.lucko.luckperms.common.sender.Sender; -import me.lucko.luckperms.common.util.RepeatingTask; import me.lucko.luckperms.common.verbose.event.MetaCheckEvent; import me.lucko.luckperms.common.verbose.event.PermissionCheckEvent; import me.lucko.luckperms.common.verbose.event.VerboseEvent; @@ -45,21 +45,21 @@ import java.util.concurrent.TimeUnit; /** * Accepts {@link VerboseEvent}s and passes them onto registered {@link VerboseListener}s. */ -public class VerboseHandler extends RepeatingTask { +public class VerboseHandler implements AutoCloseable { - // the listeners currently registered + /** A map of currently registered listeners */ private final Map listeners; - - // a queue of events + /** A queue of verbose events to be handled */ private final Queue queue; - - // if there are any listeners currently registered + /** If there are any listeners registered */ private boolean listening = false; + /** The tick task */ + private final SchedulerTask task; public VerboseHandler(SchedulerAdapter scheduler) { - super(scheduler, 100, TimeUnit.MILLISECONDS); this.listeners = new ConcurrentHashMap<>(); this.queue = new ConcurrentLinkedQueue<>(); + this.task = scheduler.asyncRepeating(this::tick, 100, TimeUnit.MILLISECONDS); } /** @@ -137,8 +137,7 @@ public class VerboseHandler extends RepeatingTask { return this.listeners.remove(uuid); } - @Override - protected void tick() { + private void tick() { // remove listeners where the sender is no longer valid this.listeners.values().removeIf(l -> !l.getNotifiedSender().isValid()); @@ -159,4 +158,10 @@ public class VerboseHandler extends RepeatingTask { } } } + + @Override + public void close() { + this.task.cancel(); + } + }