From 71b6e8df90652931f0279536e79440895b47e723 Mon Sep 17 00:00:00 2001 From: themode Date: Sun, 20 Mar 2022 03:22:38 +0100 Subject: [PATCH] Misc network improvement --- .../server/network/ConnectionManager.java | 39 +------------- .../network/player/FakePlayerConnection.java | 8 --- .../network/player/PlayerConnection.java | 15 +++--- .../player/PlayerSocketConnection.java | 51 +++++++------------ .../server/network/socket/Worker.java | 39 +++++++------- 5 files changed, 48 insertions(+), 104 deletions(-) diff --git a/src/main/java/net/minestom/server/network/ConnectionManager.java b/src/main/java/net/minestom/server/network/ConnectionManager.java index c8c449334..996779c1e 100644 --- a/src/main/java/net/minestom/server/network/ConnectionManager.java +++ b/src/main/java/net/minestom/server/network/ConnectionManager.java @@ -2,14 +2,12 @@ package net.minestom.server.network; import net.kyori.adventure.text.Component; import net.kyori.adventure.text.format.NamedTextColor; -import net.minestom.server.MinecraftServer; import net.minestom.server.entity.Player; import net.minestom.server.event.EventDispatcher; import net.minestom.server.event.player.AsyncPlayerPreLoginEvent; import net.minestom.server.event.player.PlayerLoginEvent; import net.minestom.server.instance.Instance; import net.minestom.server.network.packet.server.login.LoginSuccessPacket; -import net.minestom.server.network.packet.server.play.DisconnectPacket; import net.minestom.server.network.packet.server.play.KeepAlivePacket; import net.minestom.server.network.player.PlayerConnection; import net.minestom.server.utils.StringUtils; @@ -45,8 +43,6 @@ public final class ConnectionManager { // The player provider to have your own Player implementation private volatile PlayerProvider playerProvider = Player::new; - private Component shutdownText = Component.text("The server is shutting down.", NamedTextColor.RED); - /** * Gets the {@link Player} linked to a {@link PlayerConnection}. * @@ -167,25 +163,6 @@ public final class ConnectionManager { return playerProvider; } - /** - * Gets the kick reason when the server is shutdown using {@link MinecraftServer#stopCleanly()}. - * - * @return the kick reason in case on a shutdown - */ - public @NotNull Component getShutdownText() { - return shutdownText; - } - - /** - * Changes the kick reason in case of a shutdown. - * - * @param shutdownText the new shutdown kick reason - * @see #getShutdownText() - */ - public void setShutdownText(@NotNull Component shutdownText) { - this.shutdownText = shutdownText; - } - public synchronized void registerPlayer(@NotNull Player player) { this.players.add(player); this.connectionPlayerMap.put(player.getPlayerConnection(), player); @@ -220,12 +197,8 @@ public final class ConnectionManager { // Call pre login event AsyncPlayerPreLoginEvent asyncPlayerPreLoginEvent = new AsyncPlayerPreLoginEvent(player); EventDispatcher.call(asyncPlayerPreLoginEvent); - // Close the player channel if he has been disconnected (kick) - if (!player.isOnline()) { - playerConnection.flush(); - //playerConnection.disconnect(); - return; - } + if (!player.isOnline()) + return; // Player has been kicked // Change UUID/Username based on the event { final String eventUsername = asyncPlayerPreLoginEvent.getUsername(); @@ -265,14 +238,6 @@ public final class ConnectionManager { * Shutdowns the connection manager by kicking all the currently connected players. */ public synchronized void shutdown() { - DisconnectPacket disconnectPacket = new DisconnectPacket(shutdownText); - for (Player player : getOnlinePlayers()) { - final PlayerConnection playerConnection = player.getPlayerConnection(); - playerConnection.sendPacket(disconnectPacket); - playerConnection.flush(); - player.remove(); - playerConnection.disconnect(); - } this.players.clear(); this.connectionPlayerMap.clear(); } diff --git a/src/main/java/net/minestom/server/network/player/FakePlayerConnection.java b/src/main/java/net/minestom/server/network/player/FakePlayerConnection.java index df6aa3394..6e879f6e5 100644 --- a/src/main/java/net/minestom/server/network/player/FakePlayerConnection.java +++ b/src/main/java/net/minestom/server/network/player/FakePlayerConnection.java @@ -1,6 +1,5 @@ package net.minestom.server.network.player; -import net.minestom.server.MinecraftServer; import net.minestom.server.entity.Player; import net.minestom.server.entity.fakeplayer.FakePlayer; import net.minestom.server.entity.fakeplayer.FakePlayerController; @@ -27,13 +26,6 @@ public class FakePlayerConnection extends PlayerConnection { return new InetSocketAddress(0); } - @Override - public void disconnect() { - super.disconnect(); - if (getFakePlayer().getOption().isRegistered()) - MinecraftServer.getConnectionManager().removePlayer(this); - } - public FakePlayer getFakePlayer() { return (FakePlayer) getPlayer(); } diff --git a/src/main/java/net/minestom/server/network/player/PlayerConnection.java b/src/main/java/net/minestom/server/network/player/PlayerConnection.java index dea7c2b2a..c9f1a6e51 100644 --- a/src/main/java/net/minestom/server/network/player/PlayerConnection.java +++ b/src/main/java/net/minestom/server/network/player/PlayerConnection.java @@ -3,6 +3,7 @@ package net.minestom.server.network.player; import net.kyori.adventure.text.Component; import net.kyori.adventure.text.format.NamedTextColor; import net.minestom.server.MinecraftServer; +import net.minestom.server.entity.Entity; import net.minestom.server.entity.Player; import net.minestom.server.listener.manager.PacketListenerManager; import net.minestom.server.network.ConnectionState; @@ -95,15 +96,6 @@ public abstract class PlayerConnection { sendPackets(List.of(packets)); } - /** - * Flush waiting data to the connection. - *

- * Might not do anything depending on the implementation. - */ - public void flush() { - // Empty - } - /** * Gets the remote address of the client. * @@ -148,6 +140,11 @@ public abstract class PlayerConnection { */ public void disconnect() { this.online = false; + MinecraftServer.getConnectionManager().removePlayer(this); + final Player player = getPlayer(); + if (player != null && !player.isRemoved()) { + player.scheduleNextTick(Entity::remove); + } } /** diff --git a/src/main/java/net/minestom/server/network/player/PlayerSocketConnection.java b/src/main/java/net/minestom/server/network/player/PlayerSocketConnection.java index 544d3ff61..8fe49588d 100644 --- a/src/main/java/net/minestom/server/network/player/PlayerSocketConnection.java +++ b/src/main/java/net/minestom/server/network/player/PlayerSocketConnection.java @@ -182,11 +182,6 @@ public class PlayerSocketConnection extends PlayerConnection { write(buffer, buffer.position(), buffer.remaining()); } - @Override - public void flush() { - this.workerQueue.relaxedOffer(this::flushSync); - } - @Override public @NotNull SocketAddress getRemoteAddress() { return remoteAddress; @@ -377,7 +372,6 @@ public class PlayerSocketConnection extends PlayerConnection { } var buffer = PacketUtils.createFramedPacket(serverPacket, compressed); writeBufferSync0(buffer, 0, buffer.limit()); - if (player == null) flushSync(); // Player is probably not logged yet } private void writeBufferSync(@NotNull ByteBuffer buffer, int index, int length) { @@ -418,35 +412,26 @@ public class PlayerSocketConnection extends PlayerConnection { } } - public void flushSync() { + public void flushSync() throws IOException { + if (!channel.isConnected()) throw new ClosedChannelException(); + if (waitingBuffers.isEmpty() && tickBuffer.getPlain().writeChannel(channel)) + return; // Fast exit if the tick buffer can be reused + try { - if (!channel.isConnected()) throw new ClosedChannelException(); - try { - if (waitingBuffers.isEmpty() && tickBuffer.getPlain().writeChannel(channel)) - return; // Fast exit if the tick buffer can be reused + updateLocalBuffer(); + } catch (OutOfMemoryError e) { + this.waitingBuffers.clear(); + System.gc(); // Explicit gc forcing buffers to be collected + throw new ClosedChannelException(); + } - try { - updateLocalBuffer(); - } catch (OutOfMemoryError e) { - this.waitingBuffers.clear(); - System.gc(); // Explicit gc forcing buffers to be collected - throw new ClosedChannelException(); - } - - // Write as much as possible from the waiting list - Iterator iterator = waitingBuffers.iterator(); - while (iterator.hasNext()) { - BinaryBuffer waitingBuffer = iterator.next(); - if (!waitingBuffer.writeChannel(channel)) break; - iterator.remove(); - PooledBuffers.add(waitingBuffer); - } - } catch (IOException e) { // Couldn't write to the socket - MinecraftServer.getExceptionManager().handleException(e); - throw new ClosedChannelException(); - } - } catch (ClosedChannelException e) { - disconnect(); + // Write as much as possible from the waiting list + Iterator iterator = waitingBuffers.iterator(); + while (iterator.hasNext()) { + BinaryBuffer waitingBuffer = iterator.next(); + if (!waitingBuffer.writeChannel(channel)) break; + iterator.remove(); + PooledBuffers.add(waitingBuffer); } } diff --git a/src/main/java/net/minestom/server/network/socket/Worker.java b/src/main/java/net/minestom/server/network/socket/Worker.java index c9a540f1e..488be242c 100644 --- a/src/main/java/net/minestom/server/network/socket/Worker.java +++ b/src/main/java/net/minestom/server/network/socket/Worker.java @@ -1,8 +1,6 @@ package net.minestom.server.network.socket; import net.minestom.server.MinecraftServer; -import net.minestom.server.entity.Entity; -import net.minestom.server.entity.Player; import net.minestom.server.network.player.PlayerSocketConnection; import net.minestom.server.thread.MinestomThread; import net.minestom.server.utils.binary.BinaryBuffer; @@ -49,17 +47,27 @@ public final class Worker extends MinestomThread { MinecraftServer.getExceptionManager().handleException(e); } // Flush all connections if needed - try { - connectionMap.values().forEach(PlayerSocketConnection::flushSync); - } catch (Exception e) { - MinecraftServer.getExceptionManager().handleException(e); + for (PlayerSocketConnection connection : connectionMap.values()) { + try { + connection.flushSync(); + } catch (Exception e) { + connection.disconnect(); + } } // Wait for an event this.selector.select(key -> { final SocketChannel channel = (SocketChannel) key.channel(); if (!channel.isOpen()) return; if (!key.isReadable()) return; - PlayerSocketConnection connection = connectionMap.get(channel); + final PlayerSocketConnection connection = connectionMap.get(channel); + if (connection == null) { + try { + channel.close(); + } catch (IOException e) { + // Empty + } + return; + } try { BinaryBuffer readBuffer = BinaryBuffer.wrap(PooledBuffers.packetBuffer()); // Consume last incomplete packet @@ -84,17 +92,14 @@ public final class Worker extends MinestomThread { public void disconnect(PlayerSocketConnection connection, SocketChannel channel) { assert !connection.isOnline(); assert Thread.currentThread() == this; - connection.flushSync(); - try { - channel.close(); - this.connectionMap.remove(channel); - MinecraftServer.getConnectionManager().removePlayer(connection); - Player player = connection.getPlayer(); - if (player != null && !player.isRemoved()) { - player.scheduleNextTick(Entity::remove); + this.connectionMap.remove(channel); + if (channel.isOpen()) { + try { + connection.flushSync(); + channel.close(); + } catch (IOException e) { + // Socket operation may fail if the socket is already closed } - } catch (IOException e) { - e.printStackTrace(); } }