Better keepalive and connection handling

src/main/java/net/minestom/server/MinecraftServer.java

@@ -6,7 +6,6 @@ import net.minestom.server.command.CommandManager;
 import net.minestom.server.data.DataManager;
 import net.minestom.server.data.DataType;
 import net.minestom.server.data.SerializableData;
-import net.minestom.server.entity.EntityManager;
 import net.minestom.server.entity.EntityType;
 import net.minestom.server.entity.Player;
 import net.minestom.server.event.GlobalEventHandler;
@@ -56,7 +55,6 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
-import java.util.Collection;
 
 /**
  * The main server class used to start the server and retrieve all the managers.
@@ -105,7 +103,6 @@ public final class MinecraftServer {
     private static ConnectionManager connectionManager;
     private static InstanceManager instanceManager;
     private static BlockManager blockManager;
-    private static EntityManager entityManager;
     private static CommandManager commandManager;
     private static RecipeManager recipeManager;
     private static StorageManager storageManager;
@@ -167,7 +164,6 @@ public final class MinecraftServer {
 
         instanceManager = new InstanceManager();
         blockManager = new BlockManager();
-        entityManager = new EntityManager();
         commandManager = new CommandManager();
         recipeManager = new RecipeManager();
         storageManager = new StorageManager();
@@ -335,16 +331,6 @@ public final class MinecraftServer {
         return blockManager;
     }
 
-    /**
-     * Gets the manager handling waiting players.
-     *
-     * @return the entity manager
-     */
-    public static EntityManager getEntityManager() {
-        checkInitStatus(entityManager);
-        return entityManager;
-    }
-
     /**
      * Gets the manager handling commands.
      *

src/main/java/net/minestom/server/UpdateManager.java

@@ -1,12 +1,11 @@
 package net.minestom.server;
 
 import com.google.common.collect.Queues;
-import net.minestom.server.entity.EntityManager;
 import net.minestom.server.instance.Instance;
 import net.minestom.server.instance.InstanceManager;
+import net.minestom.server.network.ConnectionManager;
 import net.minestom.server.thread.PerInstanceThreadProvider;
 import net.minestom.server.thread.ThreadProvider;
-import net.minestom.server.utils.validate.Check;
 import org.jetbrains.annotations.NotNull;
 
 import java.util.List;
@@ -50,7 +49,7 @@ public final class UpdateManager {
      * Starts the server loop in the update thread.
      */
     protected void start() {
-        final EntityManager entityManager = MinecraftServer.getEntityManager();
+        final ConnectionManager connectionManager = MinecraftServer.getConnectionManager();
 
         updateExecutionService.scheduleAtFixedRate(() -> {
             try {
@@ -66,10 +65,10 @@ public final class UpdateManager {
                 doTickCallback(tickStartCallbacks, tickStart);
 
                 // Waiting players update (newly connected clients waiting to get into the server)
-                entityManager.updateWaitingPlayers();
+                connectionManager.updateWaitingPlayers();
 
                 // Keep Alive Handling
-                entityManager.handleKeepAlive(tickStart);
+                connectionManager.handleKeepAlive(tickStart);
 
                 // Server tick (chunks/entities)
                 serverTick(tickStart);

src/main/java/net/minestom/server/entity/EntityManager.java

@@ -1,82 +0,0 @@
-package net.minestom.server.entity;
-
-import com.google.common.collect.Queues;
-import net.minestom.server.MinecraftServer;
-import net.minestom.server.chat.ChatColor;
-import net.minestom.server.chat.ColoredText;
-import net.minestom.server.event.player.PlayerLoginEvent;
-import net.minestom.server.instance.Instance;
-import net.minestom.server.network.ConnectionManager;
-import net.minestom.server.network.packet.server.play.KeepAlivePacket;
-import net.minestom.server.network.player.PlayerConnection;
-import net.minestom.server.utils.validate.Check;
-import org.jetbrains.annotations.NotNull;
-
-import java.util.Queue;
-
-public final class EntityManager {
-
-    private static final ConnectionManager CONNECTION_MANAGER = MinecraftServer.getConnectionManager();
-
-    private static final long KEEP_ALIVE_DELAY = 10_000;
-    private static final long KEEP_ALIVE_KICK = 30_000;
-    private static final ColoredText TIMEOUT_TEXT = ColoredText.of(ChatColor.RED + "Timeout");
-
-    private final Queue<Player> waitingPlayers = Queues.newConcurrentLinkedQueue();
-
-    /**
-     * Connects waiting players.
-     */
-    public void updateWaitingPlayers() {
-        // Connect waiting players
-        waitingPlayersTick();
-    }
-
-    /**
-     * Updates keep alive by checking the last keep alive packet and send a new one if needed.
-     *
-     * @param tickStart the time of the update in milliseconds, forwarded to the packet
-     */
-    public void handleKeepAlive(long tickStart) {
-        final KeepAlivePacket keepAlivePacket = new KeepAlivePacket(tickStart);
-        for (Player player : CONNECTION_MANAGER.getOnlinePlayers()) {
-            final long lastKeepAlive = tickStart - player.getLastKeepAlive();
-            if (lastKeepAlive > KEEP_ALIVE_DELAY && player.didAnswerKeepAlive()) {
-                final PlayerConnection playerConnection = player.getPlayerConnection();
-                player.refreshKeepAlive(tickStart);
-                playerConnection.sendPacket(keepAlivePacket);
-            } else if (lastKeepAlive >= KEEP_ALIVE_KICK) {
-                player.kick(TIMEOUT_TEXT);
-            }
-        }
-    }
-
-    /**
-     * Adds connected clients after the handshake (used to free the networking threads).
-     */
-    private void waitingPlayersTick() {
-        Player waitingPlayer;
-        while ((waitingPlayer = waitingPlayers.poll()) != null) {
-
-            PlayerLoginEvent loginEvent = new PlayerLoginEvent(waitingPlayer);
-            waitingPlayer.callEvent(PlayerLoginEvent.class, loginEvent);
-            final Instance spawningInstance = loginEvent.getSpawningInstance();
-
-            Check.notNull(spawningInstance, "You need to specify a spawning instance in the PlayerLoginEvent");
-
-            waitingPlayer.init(spawningInstance);
-
-            // Spawn the player at Player#getRespawnPoint during the next instance tick
-            spawningInstance.scheduleNextTick(waitingPlayer::setInstance);
-        }
-    }
-
-    /**
-     * Adds a player into the waiting list, to be handled during the next server tick.
-     *
-     * @param player the {@link Player player} to add into the waiting list
-     */
-    public void addWaitingPlayer(@NotNull Player player) {
-        this.waitingPlayers.add(player);
-    }
-}

src/main/java/net/minestom/server/entity/Player.java

@@ -232,7 +232,7 @@ public class Player extends LivingEntity implements CommandSender {
      *
      * @param spawnInstance the player spawn instance (defined in {@link PlayerLoginEvent})
      */
-    protected void init(@NotNull Instance spawnInstance) {
+    public void init(@NotNull Instance spawnInstance) {
         this.dimensionType = spawnInstance.getDimensionType();
 
         JoinGamePacket joinGamePacket = new JoinGamePacket();

src/main/java/net/minestom/server/network/ConnectionManager.java

@@ -8,17 +8,22 @@ import net.minestom.server.chat.JsonMessage;
 import net.minestom.server.entity.Player;
 import net.minestom.server.entity.fakeplayer.FakePlayer;
 import net.minestom.server.event.player.AsyncPlayerPreLoginEvent;
+import net.minestom.server.event.player.PlayerLoginEvent;
+import net.minestom.server.instance.Instance;
 import net.minestom.server.listener.manager.ClientPacketConsumer;
 import net.minestom.server.listener.manager.ServerPacketConsumer;
 import net.minestom.server.network.packet.client.login.LoginStartPacket;
 import net.minestom.server.network.packet.server.login.LoginSuccessPacket;
 import net.minestom.server.network.packet.server.play.ChatMessagePacket;
 import net.minestom.server.network.packet.server.play.DisconnectPacket;
+import net.minestom.server.network.packet.server.play.KeepAlivePacket;
 import net.minestom.server.network.player.NettyPlayerConnection;
 import net.minestom.server.network.player.PlayerConnection;
 import net.minestom.server.utils.PacketUtils;
 import net.minestom.server.utils.async.AsyncUtils;
 import net.minestom.server.utils.callback.validator.PlayerValidator;
+import net.minestom.server.utils.collection.ConcurrentStack;
+import net.minestom.server.utils.validate.Check;
 import org.apache.commons.lang3.StringUtils;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -34,6 +39,11 @@ import java.util.function.Consumer;
  */
 public final class ConnectionManager {
 
+    private static final long KEEP_ALIVE_DELAY = 10_000;
+    private static final long KEEP_ALIVE_KICK = 30_000;
+    private static final ColoredText TIMEOUT_TEXT = ColoredText.of(ChatColor.RED + "Timeout");
+
+    private final ConcurrentStack<Player> waitingPlayers = new ConcurrentStack<>();
     private final Set<Player> players = new CopyOnWriteArraySet<>();
     private final Map<PlayerConnection, Player> connectionPlayerMap = new ConcurrentHashMap<>();
 
@@ -408,7 +418,7 @@ public final class ConnectionManager {
             }
 
             // Add the player to the waiting list
-            MinecraftServer.getEntityManager().addWaitingPlayer(player);
+            addWaitingPlayer(player);
 
             if (register) {
                 registerPlayer(player);
@@ -451,4 +461,61 @@ public final class ConnectionManager {
         this.players.clear();
         this.connectionPlayerMap.clear();
     }
+
+    /**
+     * Connects waiting players.
+     */
+    public void updateWaitingPlayers() {
+        // Connect waiting players
+        waitingPlayersTick();
+    }
+
+    /**
+     * Updates keep alive by checking the last keep alive packet and send a new one if needed.
+     *
+     * @param tickStart the time of the update in milliseconds, forwarded to the packet
+     */
+    public void handleKeepAlive(long tickStart) {
+        final KeepAlivePacket keepAlivePacket = new KeepAlivePacket(tickStart);
+        for (Player player : getOnlinePlayers()) {
+            final long lastKeepAlive = tickStart - player.getLastKeepAlive();
+            // Occasionally a packet may be dropped, this is very useful for networks that experience lag / packet loss.
+            if (lastKeepAlive >= KEEP_ALIVE_KICK) {
+                player.kick(TIMEOUT_TEXT);
+            } else if (lastKeepAlive > KEEP_ALIVE_DELAY) {
+                final PlayerConnection playerConnection = player.getPlayerConnection();
+                player.refreshKeepAlive(tickStart);
+                playerConnection.sendPacket(keepAlivePacket);
+            }
+        }
+    }
+
+    /**
+     * Adds connected clients after the handshake (used to free the networking threads).
+     */
+    private void waitingPlayersTick() {
+        Player waitingPlayer;
+        while ((waitingPlayer = waitingPlayers.pop()) != null) {
+
+            PlayerLoginEvent loginEvent = new PlayerLoginEvent(waitingPlayer);
+            waitingPlayer.callEvent(PlayerLoginEvent.class, loginEvent);
+            final Instance spawningInstance = loginEvent.getSpawningInstance();
+
+            Check.notNull(spawningInstance, "You need to specify a spawning instance in the PlayerLoginEvent");
+
+            waitingPlayer.init(spawningInstance);
+
+            // Spawn the player at Player#getRespawnPoint during the next instance tick
+            spawningInstance.scheduleNextTick(waitingPlayer::setInstance);
+        }
+    }
+
+    /**
+     * Adds a player into the waiting list, to be handled during the next server tick.
+     *
+     * @param player the {@link Player player} to add into the waiting list
+     */
+    public void addWaitingPlayer(@NotNull Player player) {
+        this.waitingPlayers.push(player);
+    }
 }

src/main/java/net/minestom/server/utils/collection/ConcurrentStack.java

@@ -0,0 +1,48 @@
+package net.minestom.server.utils.collection;
+
+import java.util.concurrent.atomic.AtomicReference;
+
+/**
+ * ConcurrentStack
+ *
+ * Nonblocking stack using Treiber's algorithm
+ *
+ * @author Brian Goetz and Tim Peierls
+ */
+public final class ConcurrentStack<E> {
+    private final AtomicReference<Node<E>> top = new AtomicReference<>();
+
+    public ConcurrentStack() {
+
+    }
+
+    public void push(E item) {
+        Node<E> newHead = new Node<>(item);
+        Node<E> oldHead;
+        do {
+            oldHead = top.get();
+            newHead.next = oldHead;
+        } while (!top.compareAndSet(oldHead, newHead));
+    }
+
+    public E pop() {
+        Node<E> oldHead;
+        Node<E> newHead;
+        do {
+            oldHead = top.get();
+            if (oldHead == null)
+                return null;
+            newHead = oldHead.next;
+        } while (!top.compareAndSet(oldHead, newHead));
+        return oldHead.item;
+    }
+
+    private static class Node<E> {
+        public final E item;
+        public Node<E> next;
+
+        public Node(E item) {
+            this.item = item;
+        }
+    }
+}