From f43b9c96de97b546ce9f5f08fb8d9119435ba41c Mon Sep 17 00:00:00 2001 From: Luck Date: Fri, 21 Apr 2017 19:10:25 +0100 Subject: [PATCH] cleanup login handling & add CountdownLatch to ensure the plugin has started before logins are handled --- .../luckperms/bukkit/BukkitListener.java | 13 ++- .../luckperms/bukkit/LPBukkitPlugin.java | 16 ++- .../luckperms/bungee/BungeeListener.java | 109 ++++++------------ .../luckperms/common/utils/LoginHelper.java | 13 ++- .../luckperms/sponge/SpongeListener.java | 2 +- 5 files changed, 72 insertions(+), 81 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 107a7ea91..3a2a295cd 100644 --- a/bukkit/src/main/java/me/lucko/luckperms/bukkit/BukkitListener.java +++ b/bukkit/src/main/java/me/lucko/luckperms/bukkit/BukkitListener.java @@ -50,6 +50,7 @@ import java.util.Collections; import java.util.HashSet; import java.util.Set; import java.util.UUID; +import java.util.concurrent.TimeUnit; @RequiredArgsConstructor public class BukkitListener implements Listener { @@ -63,6 +64,14 @@ public class BukkitListener implements Listener { /* Called when the player first attempts a connection with the server. Listening on LOW priority to allow plugins to modify username / UUID data here. (auth plugins) */ + /* wait for the plugin to enable. because these events are fired async, they can be called before + the plugin has enabled. */ + try { + plugin.getEnableLatch().await(30, TimeUnit.SECONDS); + } catch (InterruptedException ex) { + ex.printStackTrace(); + } + /* the player was denied entry to the server before this priority. log this, so we can handle appropriately later. */ if (e.getLoginResult() != AsyncPlayerPreLoginEvent.Result.ALLOWED) { @@ -71,7 +80,7 @@ public class BukkitListener implements Listener { return; } - /* either the plugin hasn't finished starting yet, or there was an issue connecting to the DB, performing file i/o, etc. + /* there was an issue connecting to the DB, performing file i/o, etc. we don't let players join in this case, because it means they can connect to the server without their permissions data. some server admins rely on negating perms to stop users from causing damage etc, so it's really important that this data is loaded. */ @@ -96,7 +105,7 @@ public class BukkitListener implements Listener { - creating a user instance in the UserManager for this connection. - setting up cached data. */ try { - LoginHelper.loadUser(plugin, e.getUniqueId(), e.getName()); + LoginHelper.loadUser(plugin, e.getUniqueId(), e.getName(), false); } catch (Exception ex) { ex.printStackTrace(); 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 a43900687..c99202bde 100644 --- a/bukkit/src/main/java/me/lucko/luckperms/bukkit/LPBukkitPlugin.java +++ b/bukkit/src/main/java/me/lucko/luckperms/bukkit/LPBukkitPlugin.java @@ -102,6 +102,7 @@ import java.util.Optional; import java.util.Set; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CountDownLatch; import java.util.stream.Collectors; @Getter @@ -130,6 +131,7 @@ public class LPBukkitPlugin extends JavaPlugin implements LuckPermsPlugin { private CalculatorFactory calculatorFactory; private BufferedRequest updateTaskBuffer; private boolean started = false; + private CountDownLatch enableLatch = new CountDownLatch(1); private VerboseHandler verboseHandler; private BukkitSenderFactory senderFactory; private PermissionVault permissionVault; @@ -147,6 +149,17 @@ public class LPBukkitPlugin extends JavaPlugin implements LuckPermsPlugin { @Override public void onEnable() { + try { + enable(); + started = true; + } finally { + // count down the latch when onEnable has been called + // we don't care about the result here + enableLatch.countDown(); + } + } + + private void enable() { LuckPermsPlugin.sendStartupBanner(getConsoleSender(), this); ignoringLogs = ConcurrentHashMap.newKeySet(); @@ -324,7 +337,7 @@ public class LPBukkitPlugin extends JavaPlugin implements LuckPermsPlugin { // Load any online users (in the case of a reload) for (Player player : getServer().getOnlinePlayers()) { scheduler.doAsync(() -> { - LoginHelper.loadUser(this, player.getUniqueId(), player.getName()); + LoginHelper.loadUser(this, player.getUniqueId(), player.getName(), false); User user = getUserManager().get(getUuidCache().getUUID(player.getUniqueId())); if (user != null) { scheduler.doSync(() -> { @@ -339,7 +352,6 @@ public class LPBukkitPlugin extends JavaPlugin implements LuckPermsPlugin { }); } - started = true; getLog().info("Successfully loaded."); } diff --git a/bungee/src/main/java/me/lucko/luckperms/bungee/BungeeListener.java b/bungee/src/main/java/me/lucko/luckperms/bungee/BungeeListener.java index adb0fb57b..d4447ab45 100644 --- a/bungee/src/main/java/me/lucko/luckperms/bungee/BungeeListener.java +++ b/bungee/src/main/java/me/lucko/luckperms/bungee/BungeeListener.java @@ -32,9 +32,8 @@ import me.lucko.luckperms.api.caching.UserData; import me.lucko.luckperms.api.context.MutableContextSet; import me.lucko.luckperms.common.config.ConfigKeys; import me.lucko.luckperms.common.constants.Message; -import me.lucko.luckperms.common.core.UuidCache; import me.lucko.luckperms.common.core.model.User; -import me.lucko.luckperms.common.defaults.Rule; +import me.lucko.luckperms.common.utils.LoginHelper; import net.md_5.bungee.api.chat.TextComponent; import net.md_5.bungee.api.connection.PendingConnection; @@ -69,94 +68,58 @@ public class BungeeListener implements Listener { This means that a player will have the same UUID across the network, even if parts of the network are running in Offline mode. */ - // registers the plugins intent to modify this events state going forward. - // this will prevent the event from completing until we're finished handling. + /* registers the plugins intent to modify this events state going forward. + this will prevent the event from completing until we're finished handling. */ e.registerIntent(plugin); - final long startTime = System.currentTimeMillis(); final PendingConnection c = e.getConnection(); - /* either the plugin hasn't finished starting yet, or there was an issue connecting to the DB, performing file i/o, etc. - as this is bungeecord, we will still allow the login, as players can't really do much harm without permissions data. - the proxy will just fallback to using the config file perms. */ - if (!plugin.getStorage().isAcceptingLogins()) { - // log that the user tried to login, but was denied at this stage. - deniedLogin.add(c.getUniqueId()); - return; - } - /* another plugin (or the proxy itself) has cancelled this connection already */ if (e.isCancelled()) { + + // log that we are not loading any data plugin.getLog().warn("Connection from " + c.getUniqueId() + " was already denied. No permissions data will be loaded."); deniedLogin.add(c.getUniqueId()); + + e.completeIntent(plugin); + return; + } + + /* there was an issue connecting to the DB, performing file i/o, etc. + as this is bungeecord, we will still allow the login, as players can't really do much harm without permissions data. + the proxy will just fallback to using the config file perms. */ + if (!plugin.getStorage().isAcceptingLogins()) { + + // log that the user tried to login, but was denied at this stage. + plugin.getLog().warn("Permissions storage is not loaded yet. No permissions data will be loaded for: " + c.getUniqueId() + " - " + c.getName()); + deniedLogin.add(c.getUniqueId()); + + e.completeIntent(plugin); return; } - /* Actually process the login for the connection. - We do this here to delay the login until the data is ready. - If the login gets cancelled later on, then this will be cleaned up. - This includes: - - loading uuid data - - loading permissions - - creating a user instance in the UserManager for this connection. - - setting up cached data. */ plugin.doAsync(() -> { - final UuidCache cache = plugin.getUuidCache(); + /* Actually process the login for the connection. + We do this here to delay the login until the data is ready. + If the login gets cancelled later on, then this will be cleaned up. - if (!plugin.getConfiguration().get(ConfigKeys.USE_SERVER_UUIDS)) { - UUID uuid = plugin.getStorage().getUUID(c.getName()).join(); - if (uuid != null) { - cache.addToCache(c.getUniqueId(), uuid); - } else { - // No previous data for this player - plugin.getApiProvider().getEventFactory().handleUserFirstLogin(c.getUniqueId(), c.getName()); - cache.addToCache(c.getUniqueId(), c.getUniqueId()); + This includes: + - loading uuid data + - loading permissions + - creating a user instance in the UserManager for this connection. + - setting up cached data. */ + try { + LoginHelper.loadUser(plugin, c.getUniqueId(), c.getName(), true); + } catch (Exception ex) { + ex.printStackTrace(); - // Join this call, as we want this to be set for when the player connects to the backend. - plugin.getStorage().force().saveUUIDData(c.getName(), c.getUniqueId()).join(); - } - } else { - String name = plugin.getStorage().getName(c.getUniqueId()).join(); - if (name == null) { - plugin.getApiProvider().getEventFactory().handleUserFirstLogin(c.getUniqueId(), c.getName()); - } - - // Online mode, no cache needed. This is just for name -> uuid lookup. - // Again, join this call so the data is available for the backend. - plugin.getStorage().force().saveUUIDData(c.getName(), c.getUniqueId()).join(); + // there was some error loading + plugin.getLog().warn("Error loading data. No permissions data will be loaded for: " + c.getUniqueId() + " - " + c.getName()); + deniedLogin.add(c.getUniqueId()); } - /* We have to make a new user on this thread whilst the connection is being held, or we get concurrency issues - as the Bukkit server and the BungeeCord server try to make a new user at the same time. */ - plugin.getStorage().force().loadUser(cache.getUUID(c.getUniqueId()), c.getName()).join(); - - User user = plugin.getUserManager().get(cache.getUUID(c.getUniqueId())); - if (user == null) { - plugin.getLog().warn("Failed to load user: " + c.getName()); - } else { - // Setup defaults for the user - boolean save = false; - for (Rule rule : plugin.getConfiguration().get(ConfigKeys.DEFAULT_ASSIGNMENTS)) { - if (rule.apply(user)) { - save = true; - } - } - - // If they were given a default, persist the new assignments back to the storage. - if (save) { - plugin.getStorage().force().saveUser(user).join(); - } - - user.setupData(false); // Pretty nasty calculation call. Sets up the caching system so data is ready when the user joins. - } - - final long time = System.currentTimeMillis() - startTime; - if (time >= 1000) { - plugin.getLog().warn("Processing login for " + c.getName() + " took " + time + "ms."); - } - - // finally, complete out intent to modify state, so the proxy can continue handling the connection. + // finally, complete our intent to modify state, so the proxy can continue handling the connection. e.completeIntent(plugin); // schedule a cleanup of the users data in a few seconds. diff --git a/common/src/main/java/me/lucko/luckperms/common/utils/LoginHelper.java b/common/src/main/java/me/lucko/luckperms/common/utils/LoginHelper.java index ead93452e..9d68d6eb6 100644 --- a/common/src/main/java/me/lucko/luckperms/common/utils/LoginHelper.java +++ b/common/src/main/java/me/lucko/luckperms/common/utils/LoginHelper.java @@ -34,6 +34,7 @@ import me.lucko.luckperms.common.defaults.Rule; import me.lucko.luckperms.common.plugin.LuckPermsPlugin; import java.util.UUID; +import java.util.concurrent.CompletableFuture; /** * Utilities for use in platform listeners @@ -41,7 +42,7 @@ import java.util.UUID; @UtilityClass public class LoginHelper { - public static void loadUser(LuckPermsPlugin plugin, UUID u, String username) { + public static void loadUser(LuckPermsPlugin plugin, UUID u, String username, boolean joinUuidSave) { final long startTime = System.currentTimeMillis(); final UuidCache cache = plugin.getUuidCache(); @@ -53,7 +54,10 @@ public class LoginHelper { // No previous data for this player plugin.getApiProvider().getEventFactory().handleUserFirstLogin(u, username); cache.addToCache(u, u); - plugin.getStorage().force().saveUUIDData(username, u); + CompletableFuture future = plugin.getStorage().force().saveUUIDData(username, u); + if (joinUuidSave) { + future.join(); + } } } else { String name = plugin.getStorage().force().getName(u).join(); @@ -62,7 +66,10 @@ public class LoginHelper { } // Online mode, no cache needed. This is just for name -> uuid lookup. - plugin.getStorage().force().saveUUIDData(username, u); + CompletableFuture future = plugin.getStorage().force().saveUUIDData(username, u); + if (joinUuidSave) { + future.join(); + } } plugin.getStorage().force().loadUser(cache.getUUID(u), username).join(); diff --git a/sponge/src/main/java/me/lucko/luckperms/sponge/SpongeListener.java b/sponge/src/main/java/me/lucko/luckperms/sponge/SpongeListener.java index dae781735..94f1a2b94 100644 --- a/sponge/src/main/java/me/lucko/luckperms/sponge/SpongeListener.java +++ b/sponge/src/main/java/me/lucko/luckperms/sponge/SpongeListener.java @@ -108,7 +108,7 @@ public class SpongeListener { - creating a user instance in the UserManager for this connection. - setting up cached data. */ try { - LoginHelper.loadUser(plugin, p.getUniqueId(), p.getName().orElseThrow(() -> new RuntimeException("No username present for user " + p.getUniqueId()))); + LoginHelper.loadUser(plugin, p.getUniqueId(), p.getName().orElseThrow(() -> new RuntimeException("No username present for user " + p.getUniqueId())), false); } catch (Exception ex) { ex.printStackTrace();