From ac68c094cef3d7e7d662e837f56f6a374c8feec5 Mon Sep 17 00:00:00 2001 From: LeoDog896 Date: Sun, 17 Jan 2021 19:26:20 -0500 Subject: [PATCH 1/5] Better keepalive and connection handling --- .../net/minestom/server/MinecraftServer.java | 14 ---- .../net/minestom/server/UpdateManager.java | 9 +- .../minestom/server/entity/EntityManager.java | 82 ------------------- .../net/minestom/server/entity/Player.java | 2 +- .../server/network/ConnectionManager.java | 69 +++++++++++++++- .../utils/collection/ConcurrentStack.java | 48 +++++++++++ 6 files changed, 121 insertions(+), 103 deletions(-) delete mode 100644 src/main/java/net/minestom/server/entity/EntityManager.java create mode 100644 src/main/java/net/minestom/server/utils/collection/ConcurrentStack.java diff --git a/src/main/java/net/minestom/server/MinecraftServer.java b/src/main/java/net/minestom/server/MinecraftServer.java index ef81e9a69..698b1c15c 100644 --- a/src/main/java/net/minestom/server/MinecraftServer.java +++ b/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. * diff --git a/src/main/java/net/minestom/server/UpdateManager.java b/src/main/java/net/minestom/server/UpdateManager.java index ef22e6987..3cd810856 100644 --- a/src/main/java/net/minestom/server/UpdateManager.java +++ b/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); diff --git a/src/main/java/net/minestom/server/entity/EntityManager.java b/src/main/java/net/minestom/server/entity/EntityManager.java deleted file mode 100644 index e1cb9ba74..000000000 --- a/src/main/java/net/minestom/server/entity/EntityManager.java +++ /dev/null @@ -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 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); - } -} diff --git a/src/main/java/net/minestom/server/entity/Player.java b/src/main/java/net/minestom/server/entity/Player.java index ba2ce704e..1004d8ab6 100644 --- a/src/main/java/net/minestom/server/entity/Player.java +++ b/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(); diff --git a/src/main/java/net/minestom/server/network/ConnectionManager.java b/src/main/java/net/minestom/server/network/ConnectionManager.java index 6a4ea5e16..0b12474c4 100644 --- a/src/main/java/net/minestom/server/network/ConnectionManager.java +++ b/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 waitingPlayers = new ConcurrentStack<>(); private final Set players = new CopyOnWriteArraySet<>(); private final Map 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); + } } diff --git a/src/main/java/net/minestom/server/utils/collection/ConcurrentStack.java b/src/main/java/net/minestom/server/utils/collection/ConcurrentStack.java new file mode 100644 index 000000000..ee194c9b5 --- /dev/null +++ b/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 { + private final AtomicReference> top = new AtomicReference<>(); + + public ConcurrentStack() { + + } + + public void push(E item) { + Node newHead = new Node<>(item); + Node oldHead; + do { + oldHead = top.get(); + newHead.next = oldHead; + } while (!top.compareAndSet(oldHead, newHead)); + } + + public E pop() { + Node oldHead; + Node 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 { + public final E item; + public Node next; + + public Node(E item) { + this.item = item; + } + } +} \ No newline at end of file From 83111cb8bb3f20f6bee0ba0032b87252b6d9c709 Mon Sep 17 00:00:00 2001 From: LeoDog896 Date: Mon, 18 Jan 2021 10:35:43 -0500 Subject: [PATCH 2/5] Add javadocs to ConcurrentStack --- .../server/utils/collection/ConcurrentStack.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/main/java/net/minestom/server/utils/collection/ConcurrentStack.java b/src/main/java/net/minestom/server/utils/collection/ConcurrentStack.java index ee194c9b5..3199c4498 100644 --- a/src/main/java/net/minestom/server/utils/collection/ConcurrentStack.java +++ b/src/main/java/net/minestom/server/utils/collection/ConcurrentStack.java @@ -16,6 +16,11 @@ public final class ConcurrentStack { } + /** + * Adds a new element to the top of the stack + * + * @param item The item to add to the top of the stack + */ public void push(E item) { Node newHead = new Node<>(item); Node oldHead; @@ -25,6 +30,11 @@ public final class ConcurrentStack { } while (!top.compareAndSet(oldHead, newHead)); } + /** + * Removes an element from the top of the stack + * + * @return The removed element. + */ public E pop() { Node oldHead; Node newHead; @@ -37,6 +47,10 @@ public final class ConcurrentStack { return oldHead.item; } + public void clear() { + top.set(null); + } + private static class Node { public final E item; public Node next; From 4aed6ffa5c09d663d745c47c4fbaf67e8888a1f4 Mon Sep 17 00:00:00 2001 From: LeoDog896 Date: Mon, 18 Jan 2021 10:42:01 -0500 Subject: [PATCH 3/5] Revert ConcurrentStack --- .../server/network/ConnectionManager.java | 8 +-- .../utils/collection/ConcurrentStack.java | 62 ------------------- 2 files changed, 4 insertions(+), 66 deletions(-) delete mode 100644 src/main/java/net/minestom/server/utils/collection/ConcurrentStack.java diff --git a/src/main/java/net/minestom/server/network/ConnectionManager.java b/src/main/java/net/minestom/server/network/ConnectionManager.java index 0b12474c4..e9f415ab0 100644 --- a/src/main/java/net/minestom/server/network/ConnectionManager.java +++ b/src/main/java/net/minestom/server/network/ConnectionManager.java @@ -22,7 +22,6 @@ 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; @@ -30,6 +29,7 @@ import org.jetbrains.annotations.Nullable; import java.util.*; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.CopyOnWriteArraySet; import java.util.function.Consumer; @@ -43,7 +43,7 @@ public final class ConnectionManager { private static final long KEEP_ALIVE_KICK = 30_000; private static final ColoredText TIMEOUT_TEXT = ColoredText.of(ChatColor.RED + "Timeout"); - private final ConcurrentStack waitingPlayers = new ConcurrentStack<>(); + private final Queue waitingPlayers = new ConcurrentLinkedQueue<>(); private final Set players = new CopyOnWriteArraySet<>(); private final Map connectionPlayerMap = new ConcurrentHashMap<>(); @@ -495,7 +495,7 @@ public final class ConnectionManager { */ private void waitingPlayersTick() { Player waitingPlayer; - while ((waitingPlayer = waitingPlayers.pop()) != null) { + while ((waitingPlayer = waitingPlayers.remove()) != null) { PlayerLoginEvent loginEvent = new PlayerLoginEvent(waitingPlayer); waitingPlayer.callEvent(PlayerLoginEvent.class, loginEvent); @@ -516,6 +516,6 @@ public final class ConnectionManager { * @param player the {@link Player player} to add into the waiting list */ public void addWaitingPlayer(@NotNull Player player) { - this.waitingPlayers.push(player); + this.waitingPlayers.add(player); } } diff --git a/src/main/java/net/minestom/server/utils/collection/ConcurrentStack.java b/src/main/java/net/minestom/server/utils/collection/ConcurrentStack.java deleted file mode 100644 index 3199c4498..000000000 --- a/src/main/java/net/minestom/server/utils/collection/ConcurrentStack.java +++ /dev/null @@ -1,62 +0,0 @@ -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 { - private final AtomicReference> top = new AtomicReference<>(); - - public ConcurrentStack() { - - } - - /** - * Adds a new element to the top of the stack - * - * @param item The item to add to the top of the stack - */ - public void push(E item) { - Node newHead = new Node<>(item); - Node oldHead; - do { - oldHead = top.get(); - newHead.next = oldHead; - } while (!top.compareAndSet(oldHead, newHead)); - } - - /** - * Removes an element from the top of the stack - * - * @return The removed element. - */ - public E pop() { - Node oldHead; - Node newHead; - do { - oldHead = top.get(); - if (oldHead == null) - return null; - newHead = oldHead.next; - } while (!top.compareAndSet(oldHead, newHead)); - return oldHead.item; - } - - public void clear() { - top.set(null); - } - - private static class Node { - public final E item; - public Node next; - - public Node(E item) { - this.item = item; - } - } -} \ No newline at end of file From 78abfb9657377cfac76304653afa8f2a32eb659c Mon Sep 17 00:00:00 2001 From: LeoDog896 Date: Mon, 18 Jan 2021 10:44:12 -0500 Subject: [PATCH 4/5] Make init function UNSAFE --- src/main/java/net/minestom/server/entity/Player.java | 3 ++- .../java/net/minestom/server/network/ConnectionManager.java | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/net/minestom/server/entity/Player.java b/src/main/java/net/minestom/server/entity/Player.java index 1004d8ab6..aa882a1a7 100644 --- a/src/main/java/net/minestom/server/entity/Player.java +++ b/src/main/java/net/minestom/server/entity/Player.java @@ -229,10 +229,11 @@ public class Player extends LivingEntity implements CommandSender { * Init the player and spawn him. *

* WARNING: executed in the main update thread + * UNSAFE: Only meant to be used when a netty player connects through the server. * * @param spawnInstance the player spawn instance (defined in {@link PlayerLoginEvent}) */ - public void init(@NotNull Instance spawnInstance) { + public void UNSAFE_init(@NotNull Instance spawnInstance) { this.dimensionType = spawnInstance.getDimensionType(); JoinGamePacket joinGamePacket = new JoinGamePacket(); diff --git a/src/main/java/net/minestom/server/network/ConnectionManager.java b/src/main/java/net/minestom/server/network/ConnectionManager.java index e9f415ab0..a6c635645 100644 --- a/src/main/java/net/minestom/server/network/ConnectionManager.java +++ b/src/main/java/net/minestom/server/network/ConnectionManager.java @@ -503,7 +503,7 @@ public final class ConnectionManager { Check.notNull(spawningInstance, "You need to specify a spawning instance in the PlayerLoginEvent"); - waitingPlayer.init(spawningInstance); + waitingPlayer.UNSAFE_init(spawningInstance); // Spawn the player at Player#getRespawnPoint during the next instance tick spawningInstance.scheduleNextTick(waitingPlayer::setInstance); From fa7fe1398f115369ba39ec3fbe35eb4dcf57e0de Mon Sep 17 00:00:00 2001 From: LeoDog896 Date: Mon, 18 Jan 2021 10:45:22 -0500 Subject: [PATCH 5/5] Change remove to poll --- .../java/net/minestom/server/network/ConnectionManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/net/minestom/server/network/ConnectionManager.java b/src/main/java/net/minestom/server/network/ConnectionManager.java index a6c635645..d429a1595 100644 --- a/src/main/java/net/minestom/server/network/ConnectionManager.java +++ b/src/main/java/net/minestom/server/network/ConnectionManager.java @@ -495,7 +495,7 @@ public final class ConnectionManager { */ private void waitingPlayersTick() { Player waitingPlayer; - while ((waitingPlayer = waitingPlayers.remove()) != null) { + while ((waitingPlayer = waitingPlayers.poll()) != null) { PlayerLoginEvent loginEvent = new PlayerLoginEvent(waitingPlayer); waitingPlayer.callEvent(PlayerLoginEvent.class, loginEvent);