cleanup login handling & add CountdownLatch to ensure the plugin has started before logins are handled

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();
 

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<Void> 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.");
     }
 

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.
+        /* registers the plugins intent to modify this events state going forward.
-        // this will prevent the event from completing until we're finished handling.
+           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)) {
+               This includes:
-                UUID uuid = plugin.getStorage().getUUID(c.getName()).join();
+               - loading uuid data
-                if (uuid != null) {
+               - loading permissions
-                    cache.addToCache(c.getUniqueId(), uuid);
+               - creating a user instance in the UserManager for this connection.
-                } else {
+               - setting up cached data. */
-                    // No previous data for this player
+            try {
-                    plugin.getApiProvider().getEventFactory().handleUserFirstLogin(c.getUniqueId(), c.getName());
+                LoginHelper.loadUser(plugin, c.getUniqueId(), c.getName(), true);
-                    cache.addToCache(c.getUniqueId(), c.getUniqueId());
+            } catch (Exception ex) {
+                ex.printStackTrace();
 
-                    // Join this call, as we want this to be set for when the player connects to the backend.
+                // there was some error loading
-                    plugin.getStorage().force().saveUUIDData(c.getName(), c.getUniqueId()).join();
+                plugin.getLog().warn("Error loading data. No permissions data will be loaded for: " + c.getUniqueId() + " - " + c.getName());
-                }
+                deniedLogin.add(c.getUniqueId());
-            } 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();
             }
 
-            /* We have to make a new user on this thread whilst the connection is being held, or we get concurrency issues
+            // finally, complete our intent to modify state, so the proxy can continue handling the connection.
-               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.
             e.completeIntent(plugin);
 
             // schedule a cleanup of the users data in a few seconds.

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<Boolean> 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<Boolean> future = plugin.getStorage().force().saveUUIDData(username, u);
+            if (joinUuidSave) {
+                future.join();
+            }
         }
 
         plugin.getStorage().force().loadUser(cache.getUUID(u), username).join();

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();