Start working on thread safety of on join checks

src/main/java/fr/xephi/authme/annotation/MightBeAsync.java

@@ -0,0 +1,11 @@
+package fr.xephi.authme.annotation;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+@Target({ElementType.METHOD, ElementType.CONSTRUCTOR})
+@Retention(RetentionPolicy.RUNTIME)
+public @interface MightBeAsync {
+}

src/main/java/fr/xephi/authme/annotation/ShouldBeAsync.java

@@ -0,0 +1,11 @@
+package fr.xephi.authme.annotation;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+@Target({ElementType.METHOD, ElementType.CONSTRUCTOR})
+@Retention(RetentionPolicy.RUNTIME)
+public @interface ShouldBeAsync {
+}

src/main/java/fr/xephi/authme/annotation/Sync.java

@@ -0,0 +1,11 @@
+package fr.xephi.authme.annotation;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+@Target({ElementType.METHOD, ElementType.CONSTRUCTOR})
+@Retention(RetentionPolicy.RUNTIME)
+public @interface Sync {
+}

src/main/java/fr/xephi/authme/listener/OnJoinVerifier.java

@@ -1,6 +1,8 @@
 package fr.xephi.authme.listener;
 
 import fr.xephi.authme.ConsoleLogger;
+import fr.xephi.authme.annotation.ShouldBeAsync;
+import fr.xephi.authme.annotation.Sync;
 import fr.xephi.authme.data.auth.PlayerAuth;
 import fr.xephi.authme.datasource.DataSource;
 import fr.xephi.authme.initialization.Reloadable;
@@ -71,6 +73,7 @@ public class OnJoinVerifier implements Reloadable {
      * @param isAuthAvailable whether or not the player is registered
      * @throws FailedVerificationException if the verification fails
      */
+    @ShouldBeAsync
     public void checkAntibot(String name, boolean isAuthAvailable) throws FailedVerificationException {
         if (isAuthAvailable || permissionsManager.hasPermissionOffline(name, PlayerStatePermission.BYPASS_ANTIBOT)) {
             return;
@@ -87,6 +90,7 @@ public class OnJoinVerifier implements Reloadable {
      * @param isAuthAvailable whether or not the player is registered
      * @throws FailedVerificationException if the verification fails
      */
+    @ShouldBeAsync
     public void checkKickNonRegistered(boolean isAuthAvailable) throws FailedVerificationException {
         if (!isAuthAvailable && settings.getProperty(RestrictionSettings.KICK_NON_REGISTERED)) {
             throw new FailedVerificationException(MessageKey.MUST_REGISTER_MESSAGE);
@@ -99,6 +103,7 @@ public class OnJoinVerifier implements Reloadable {
      * @param name the name to verify
      * @throws FailedVerificationException if the verification fails
      */
+    @ShouldBeAsync
     public void checkIsValidName(String name) throws FailedVerificationException {
         if (name.length() > settings.getProperty(RestrictionSettings.MAX_NICKNAME_LENGTH)
             || name.length() < settings.getProperty(RestrictionSettings.MIN_NICKNAME_LENGTH)) {
@@ -119,6 +124,7 @@ public class OnJoinVerifier implements Reloadable {
      * @return true if the player's connection should be refused (i.e. the event does not need to be processed
      *         further), false if the player is not refused
      */
+    @Sync
     public boolean refusePlayerForFullServer(PlayerLoginEvent event) {
         final Player player = event.getPlayer();
         if (event.getResult() != PlayerLoginEvent.Result.KICK_FULL) {
@@ -155,6 +161,7 @@ public class OnJoinVerifier implements Reloadable {
      * @param auth the auth object associated with the player
      * @throws FailedVerificationException if the verification fails
      */
+    @ShouldBeAsync
     public void checkNameCasing(String connectingName, PlayerAuth auth) throws FailedVerificationException {
         if (auth != null && settings.getProperty(RegistrationSettings.PREVENT_OTHER_CASE)) {
             String realName = auth.getRealName(); // might be null or "Player"
@@ -175,6 +182,7 @@ public class OnJoinVerifier implements Reloadable {
      * @param isAuthAvailable whether or not the user is registered
      * @throws FailedVerificationException if the verification fails
      */
+    @ShouldBeAsync
     public void checkPlayerCountry(String name, String address,
                                    boolean isAuthAvailable) throws FailedVerificationException {
         if ((!isAuthAvailable || settings.getProperty(ProtectionSettings.ENABLE_PROTECTION_REGISTERED))
@@ -192,6 +200,7 @@ public class OnJoinVerifier implements Reloadable {
      * @param name the player name to check
      * @throws FailedVerificationException if the verification fails
      */
+    @Sync
     public void checkSingleSession(String name) throws FailedVerificationException {
         if (!settings.getProperty(RestrictionSettings.FORCE_SINGLE_SESSION)) {
             return;
@@ -210,6 +219,7 @@ public class OnJoinVerifier implements Reloadable {
      *
      * @return the player to kick, or null if none applicable
      */
+    @Sync
     private Player generateKickPlayer(Collection<Player> onlinePlayers) {
         for (Player player : onlinePlayers) {
             if (!permissionsManager.hasPermission(player, PlayerStatePermission.IS_VIP)) {

src/main/java/fr/xephi/authme/listener/PlayerListener.java

@@ -112,7 +112,6 @@ public class PlayerListener implements Listener {
 
         // Non-blocking checks
         try {
-            onJoinVerifier.checkSingleSession(name);
             onJoinVerifier.checkIsValidName(name);
         } catch (FailedVerificationException e) {
             event.setKickMessage(messages.retrieveSingle(name, e.getReason(), e.getArgs()));
@@ -161,6 +160,14 @@ public class PlayerListener implements Listener {
         final Player player = event.getPlayer();
         final String name = player.getName();
 
+        try {
+            onJoinVerifier.checkSingleSession(name);
+        } catch (FailedVerificationException e) {
+            event.setKickMessage(messages.retrieveSingle(name, e.getReason(), e.getArgs()));
+            event.setResult(PlayerLoginEvent.Result.KICK_OTHER);
+            return;
+        }
+
         if (validationService.isUnrestricted(name)) {
             return;
         }

src/main/java/fr/xephi/authme/service/AntiBotService.java

@@ -1,5 +1,8 @@
 package fr.xephi.authme.service;
 
+import fr.xephi.authme.annotation.MightBeAsync;
+import fr.xephi.authme.annotation.ShouldBeAsync;
+import fr.xephi.authme.annotation.Sync;
 import fr.xephi.authme.initialization.SettingsDependent;
 import fr.xephi.authme.message.MessageKey;
 import fr.xephi.authme.message.Messages;
@@ -7,11 +10,10 @@ import fr.xephi.authme.permission.AdminPermission;
 import fr.xephi.authme.permission.PermissionsManager;
 import fr.xephi.authme.settings.Settings;
 import fr.xephi.authme.settings.properties.ProtectionSettings;
+import fr.xephi.authme.util.AtomicCounter;
 import org.bukkit.scheduler.BukkitTask;
 
 import javax.inject.Inject;
-import java.time.Instant;
-import java.time.temporal.ChronoUnit;
 import java.util.concurrent.CopyOnWriteArrayList;
 
 import static fr.xephi.authme.service.BukkitService.TICKS_PER_MINUTE;
@@ -29,14 +31,11 @@ public class AntiBotService implements SettingsDependent {
     private final CopyOnWriteArrayList<String> antibotKicked = new CopyOnWriteArrayList<>();
     // Settings
     private int duration;
-    private int sensibility;
-    private int interval;
     // Service status
     private AntiBotStatus antiBotStatus;
     private boolean startup;
     private BukkitTask disableTask;
-    private Instant lastFlaggedJoin;
-    private int flagged = 0;
+    private AtomicCounter flaggedCounter;
 
     @Inject
     AntiBotService(Settings settings, Messages messages, PermissionsManager permissionsManager,
@@ -47,7 +46,6 @@ public class AntiBotService implements SettingsDependent {
         this.bukkitService = bukkitService;
         // Initial status
         disableTask = null;
-        flagged = 0;
         antiBotStatus = AntiBotStatus.DISABLED;
         startup = true;
         // Load settings and start if required
@@ -58,8 +56,9 @@ public class AntiBotService implements SettingsDependent {
     public void reload(Settings settings) {
         // Load settings
         duration = settings.getProperty(ProtectionSettings.ANTIBOT_DURATION);
-        sensibility = settings.getProperty(ProtectionSettings.ANTIBOT_SENSIBILITY);
-        interval = settings.getProperty(ProtectionSettings.ANTIBOT_INTERVAL);
+        int sensibility = settings.getProperty(ProtectionSettings.ANTIBOT_SENSIBILITY);
+        int interval = settings.getProperty(ProtectionSettings.ANTIBOT_INTERVAL) * 1000;
+        flaggedCounter = new AtomicCounter(sensibility, interval);
 
         // Stop existing protection
         stopProtection();
@@ -83,24 +82,32 @@ public class AntiBotService implements SettingsDependent {
         }
     }
 
-    private void startProtection() {
-        // Disable existing antibot session
-        stopProtection();
-        // Enable the new session
-        antiBotStatus = AntiBotStatus.ACTIVE;
-
-        // Inform admins
-        bukkitService.getOnlinePlayers().stream()
-            .filter(player -> permissionsManager.hasPermission(player, AdminPermission.ANTIBOT_MESSAGES))
-            .forEach(player -> messages.send(player, MessageKey.ANTIBOT_AUTO_ENABLED_MESSAGE));
-
+    /**
+     * Transitions the anti bot service to an active status.
+     */
+    @MightBeAsync
+    private synchronized void startProtection() {
+        if (antiBotStatus == AntiBotStatus.ACTIVE) {
+            return; // Already activating/active
+        }
+        if (disableTask != null) {
+            disableTask.cancel();
+        }
         // Schedule auto-disable
         disableTask = bukkitService.runTaskLater(this::stopProtection, duration * TICKS_PER_MINUTE);
+        antiBotStatus = AntiBotStatus.ACTIVE;
+        bukkitService.scheduleSyncDelayedTask(() -> {
+            // Inform admins
+            bukkitService.getOnlinePlayers().stream()
+                .filter(player -> permissionsManager.hasPermission(player, AdminPermission.ANTIBOT_MESSAGES))
+                .forEach(player -> messages.send(player, MessageKey.ANTIBOT_AUTO_ENABLED_MESSAGE));
+        });
     }
 
     /**
      * Transitions the anti bot service from active status back to listening.
      */
+    @Sync
     private void stopProtection() {
         if (antiBotStatus != AntiBotStatus.ACTIVE) {
             return;
@@ -108,7 +115,7 @@ public class AntiBotService implements SettingsDependent {
 
         // Change status
         antiBotStatus = AntiBotStatus.LISTENING;
-        flagged = 0;
+        flaggedCounter.reset();
         antibotKicked.clear();
 
         // Cancel auto-disable task
@@ -151,24 +158,14 @@ public class AntiBotService implements SettingsDependent {
      *
      * @return if the player should be kicked
      */
+    @ShouldBeAsync
     public boolean shouldKick() {
         if (antiBotStatus == AntiBotStatus.DISABLED) {
             return false;
         } else if (antiBotStatus == AntiBotStatus.ACTIVE) {
             return true;
         }
-
-        if (lastFlaggedJoin == null) {
-            lastFlaggedJoin = Instant.now();
-        }
-        if (ChronoUnit.SECONDS.between(lastFlaggedJoin, Instant.now()) <= interval) {
-            flagged++;
-        } else {
-            // reset to 1 because this player is also count as not registered
-            flagged = 1;
-            lastFlaggedJoin = null;
-        }
-        if (flagged > sensibility) {
+        if (flaggedCounter.handle()) {
             startProtection();
             return true;
         }
@@ -180,9 +177,9 @@ public class AntiBotService implements SettingsDependent {
      * when antibot is deactivated.
      *
      * @param name the name to check
-     *
      * @return true if the given name has been kicked because of Antibot
      */
+    @MightBeAsync
     public boolean wasPlayerKicked(String name) {
         return antibotKicked.contains(name.toLowerCase());
     }
@@ -193,6 +190,7 @@ public class AntiBotService implements SettingsDependent {
      *
      * @param name the name to add
      */
+    @MightBeAsync
     public void addPlayerKick(String name) {
         antibotKicked.addIfAbsent(name.toLowerCase());
     }

src/main/java/fr/xephi/authme/util/AtomicCounter.java

@@ -0,0 +1,34 @@
+package fr.xephi.authme.util;
+
+public class AtomicCounter {
+    private final int threshold;
+    private final int interval;
+    private int count;
+    private long lastInsert;
+
+    public AtomicCounter(int threshold, int interval) {
+        this.threshold = threshold;
+        this.interval = interval;
+        reset();
+    }
+
+    public synchronized void reset() {
+        count = 0;
+        lastInsert = 0;
+    }
+
+    public synchronized boolean handle() {
+        long now = System.currentTimeMillis();
+        if (now - lastInsert > interval) {
+            count = 1;
+        } else {
+            count++;
+        }
+        if (count > threshold) {
+            reset();
+            return true;
+        }
+        lastInsert = now;
+        return false;
+    }
+}