From 7ec2cf09a31e9bf32c3d87e7a11461e2d5cbe968 Mon Sep 17 00:00:00 2001 From: Spottedleaf Date: Wed, 16 Oct 2024 06:41:32 -0700 Subject: [PATCH] Add proper async player disconnections Blocking can cause performance problems --- .../minecraft/network/Connection.java.patch | 21 +++++++-- .../ServerCommonPacketListenerImpl.java.patch | 47 ++++++++++++++----- .../ServerGamePacketListenerImpl.java.patch | 20 ++++---- 3 files changed, 63 insertions(+), 25 deletions(-) diff --git a/paper-server/patches/sources/net/minecraft/network/Connection.java.patch b/paper-server/patches/sources/net/minecraft/network/Connection.java.patch index c1cdf37d89..f1e414f6b1 100644 --- a/paper-server/patches/sources/net/minecraft/network/Connection.java.patch +++ b/paper-server/patches/sources/net/minecraft/network/Connection.java.patch @@ -269,7 +269,22 @@ public void write(ChannelHandlerContext channelhandlercontext, Object object, ChannelPromise channelpromise) throws Exception { super.write(channelhandlercontext, object, channelpromise); } -@@ -633,6 +763,7 @@ +@@ -613,6 +743,14 @@ + + } + ++ // Paper start - add proper async disconnect ++ public void enableAutoRead() { ++ if (this.channel != null) { ++ this.channel.config().setAutoRead(true); ++ } ++ } ++ // Paper end - add proper async disconnect ++ + public void setupCompression(int compressionThreshold, boolean rejectsBadPackets) { + if (compressionThreshold >= 0) { + ChannelHandler channelhandler = this.channel.pipeline().get("decompress"); +@@ -633,6 +771,7 @@ } else { this.channel.pipeline().addAfter("prepender", "compress", new CompressionEncoder(compressionThreshold)); } @@ -277,7 +292,7 @@ } else { if (this.channel.pipeline().get("decompress") instanceof CompressionDecoder) { this.channel.pipeline().remove("decompress"); -@@ -641,6 +772,7 @@ +@@ -641,6 +780,7 @@ if (this.channel.pipeline().get("compress") instanceof CompressionEncoder) { this.channel.pipeline().remove("compress"); } @@ -285,7 +300,7 @@ } } -@@ -661,6 +793,27 @@ +@@ -661,6 +801,27 @@ packetlistener1.onDisconnect(disconnectiondetails); } diff --git a/paper-server/patches/sources/net/minecraft/server/network/ServerCommonPacketListenerImpl.java.patch b/paper-server/patches/sources/net/minecraft/server/network/ServerCommonPacketListenerImpl.java.patch index 56b0b2d861..4ba7794d53 100644 --- a/paper-server/patches/sources/net/minecraft/server/network/ServerCommonPacketListenerImpl.java.patch +++ b/paper-server/patches/sources/net/minecraft/server/network/ServerCommonPacketListenerImpl.java.patch @@ -123,7 +123,7 @@ if (this.isSingleplayerOwner()) { ServerCommonPacketListenerImpl.LOGGER.info("Stopping singleplayer server as player logged out"); this.server.halt(false); -@@ -80,13 +136,18 @@ +@@ -80,13 +136,14 @@ @Override public void handleKeepAlive(ServerboundKeepAlivePacket packet) { @@ -135,15 +135,11 @@ this.keepAlivePending = false; } else if (!this.isSingleplayerOwner()) { - this.disconnect(ServerCommonPacketListenerImpl.TIMEOUT_DISCONNECTION_MESSAGE); -+ // Paper start - This needs to be handled on the main thread for plugins -+ server.submit(() -> { -+ this.disconnect(ServerCommonPacketListenerImpl.TIMEOUT_DISCONNECTION_MESSAGE, PlayerKickEvent.Cause.TIMEOUT); // Paper - kick event cause -+ }); -+ // Paper end - This needs to be handled on the main thread for plugins ++ this.disconnectAsync(ServerCommonPacketListenerImpl.TIMEOUT_DISCONNECTION_MESSAGE, PlayerKickEvent.Cause.TIMEOUT); // Paper - add proper async disconnect } } -@@ -94,38 +155,127 @@ +@@ -94,38 +151,127 @@ @Override public void handlePong(ServerboundPongPacket packet) {} @@ -165,7 +161,7 @@ + PacketUtils.ensureRunningOnSameThread(packet, this, this.player.serverLevel()); + ResourceLocation identifier = packet.payload().type().id(); + ByteBuf payload = ((DiscardedPayload)packet.payload()).data(); -+ + + if (identifier.equals(ServerCommonPacketListenerImpl.CUSTOM_REGISTER)) { + try { + String channels = payload.toString(com.google.common.base.Charsets.UTF_8); @@ -212,7 +208,7 @@ + return (!this.player.joining && !this.connection.isConnected()) || this.processedDisconnect; // Paper - Fix duplication bugs + } + // CraftBukkit end - ++ @Override public void handleResourcePackResponse(ServerboundResourcePackPacket packet) { PacketUtils.ensureRunningOnSameThread(packet, this, (BlockableEventLoop) this.server); @@ -281,7 +277,7 @@ Profiler.get().pop(); } -@@ -133,7 +283,7 @@ +@@ -133,7 +279,7 @@ private boolean checkIfClosed(long time) { if (this.closed) { if (time - this.closedListenerTime >= 15000L) { @@ -290,7 +286,7 @@ } return false; -@@ -156,6 +306,14 @@ +@@ -156,6 +302,14 @@ } public void send(Packet packet, @Nullable PacketSendListener callbacks) { @@ -305,7 +301,7 @@ if (packet.isTerminal()) { this.close(); } -@@ -175,20 +333,82 @@ +@@ -175,22 +329,109 @@ } } @@ -392,4 +388,31 @@ + minecraftserver.scheduleOnMain(networkmanager::handleDisconnection); // Paper } ++ // Paper start - add proper async disconnect ++ public void disconnectAsync(net.kyori.adventure.text.Component reason, PlayerKickEvent.Cause cause) { ++ this.disconnectAsync(io.papermc.paper.adventure.PaperAdventure.asVanilla(reason), cause); ++ } ++ ++ public void disconnectAsync(Component reason, PlayerKickEvent.Cause cause) { ++ this.disconnectAsync(new DisconnectionDetails(reason), cause); ++ } ++ ++ public void disconnectAsync(DisconnectionDetails disconnectionInfo, PlayerKickEvent.Cause cause) { ++ if (this.cserver.isPrimaryThread()) { ++ this.disconnect(disconnectionInfo, cause); ++ return; ++ } ++ this.connection.setReadOnly(); ++ this.server.scheduleOnMain(() -> { ++ ServerCommonPacketListenerImpl.this.disconnect(disconnectionInfo, cause); ++ if (ServerCommonPacketListenerImpl.this.player.quitReason == null) { ++ // cancelled ++ ServerCommonPacketListenerImpl.this.connection.enableAutoRead(); ++ } ++ }); ++ } ++ // Paper end - add proper async disconnect ++ protected boolean isSingleplayerOwner() { + return this.server.isSingleplayerOwner(this.playerProfile()); + } diff --git a/paper-server/patches/sources/net/minecraft/server/network/ServerGamePacketListenerImpl.java.patch b/paper-server/patches/sources/net/minecraft/server/network/ServerGamePacketListenerImpl.java.patch index fe96cfa36c..e0a47487e4 100644 --- a/paper-server/patches/sources/net/minecraft/server/network/ServerGamePacketListenerImpl.java.patch +++ b/paper-server/patches/sources/net/minecraft/server/network/ServerGamePacketListenerImpl.java.patch @@ -456,7 +456,7 @@ + // PacketUtils.ensureRunningOnSameThread(packet, this, this.player.serverLevel()); // Paper - AsyncTabCompleteEvent; run this async + // CraftBukkit start + if (!this.tabSpamThrottler.isIncrementAndUnderThreshold() && !this.server.getPlayerList().isOp(this.player.getGameProfile()) && !this.server.isSingleplayerOwner(this.player.getGameProfile())) { // Paper - configurable tab spam limits -+ this.disconnect(Component.translatable("disconnect.spam"), org.bukkit.event.player.PlayerKickEvent.Cause.SPAM); // Paper - Kick event cause ++ this.disconnectAsync(Component.translatable("disconnect.spam"), org.bukkit.event.player.PlayerKickEvent.Cause.SPAM); // Paper - Kick event cause // Paper - add proper async disconnect + return; + } + // CraftBukkit end @@ -468,7 +468,7 @@ + // Paper start + final int index; + if (packet.getCommand().length() > 64 && ((index = packet.getCommand().indexOf(' ')) == -1 || index >= 64)) { -+ this.disconnect(Component.translatable("disconnect.spam"), org.bukkit.event.player.PlayerKickEvent.Cause.SPAM); ++ this.disconnectAsync(Component.translatable("disconnect.spam"), org.bukkit.event.player.PlayerKickEvent.Cause.SPAM); // Paper - add proper async disconnect + return; + } + // Paper end @@ -659,14 +659,14 @@ + + if (byteTotal > byteAllowed) { + ServerGamePacketListenerImpl.LOGGER.warn("{} tried to send a book too large. Book size: {} - Allowed: {} - Pages: {}", this.player.getScoreboardName(), byteTotal, byteAllowed, pageList.size()); -+ this.disconnect(Component.literal("Book too large!"), org.bukkit.event.player.PlayerKickEvent.Cause.ILLEGAL_ACTION); // Paper - kick event cause ++ this.disconnectAsync(Component.literal("Book too large!"), org.bukkit.event.player.PlayerKickEvent.Cause.ILLEGAL_ACTION); // Paper - kick event cause // Paper - add proper async disconnect + return; + } + } + // Paper end - Book size limits + // CraftBukkit start + if (this.lastBookTick + 20 > MinecraftServer.currentTick) { -+ this.disconnect(Component.literal("Book edited too quickly!"), org.bukkit.event.player.PlayerKickEvent.Cause.ILLEGAL_ACTION); // Paper - kick event cause ++ this.disconnectAsync(Component.literal("Book edited too quickly!"), org.bukkit.event.player.PlayerKickEvent.Cause.ILLEGAL_ACTION); // Paper - kick event cause // Paper - add proper async disconnect + return; + } + this.lastBookTick = MinecraftServer.currentTick; @@ -1544,7 +1544,7 @@ - } else if (this.player.getChatVisibility() == ChatVisiblity.HIDDEN) { + private void tryHandleChat(String s, Runnable runnable, boolean sync) { // CraftBukkit + if (ServerGamePacketListenerImpl.isChatMessageIllegal(s)) { -+ this.disconnect((Component) Component.translatable("multiplayer.disconnect.illegal_characters"), org.bukkit.event.player.PlayerKickEvent.Cause.ILLEGAL_CHARACTERS); // Paper ++ this.disconnectAsync((Component) Component.translatable("multiplayer.disconnect.illegal_characters"), org.bukkit.event.player.PlayerKickEvent.Cause.ILLEGAL_CHARACTERS); // Paper // Paper - add proper async disconnect + } else if (this.player.isRemoved() || this.player.getChatVisibility() == ChatVisiblity.HIDDEN) { // CraftBukkit - dead men tell no tales this.send(new ClientboundSystemChatPacket(Component.translatable("chat.disabled.options").withStyle(ChatFormatting.RED), false)); } else { @@ -1565,7 +1565,7 @@ if (optional.isEmpty()) { ServerGamePacketListenerImpl.LOGGER.warn("Failed to validate message acknowledgements from {}", this.player.getName().getString()); - this.disconnect(ServerGamePacketListenerImpl.CHAT_VALIDATION_FAILED); -+ this.disconnect(ServerGamePacketListenerImpl.CHAT_VALIDATION_FAILED, org.bukkit.event.player.PlayerKickEvent.Cause.CHAT_VALIDATION_FAILED); // Paper - kick event causes ++ this.disconnectAsync(ServerGamePacketListenerImpl.CHAT_VALIDATION_FAILED, org.bukkit.event.player.PlayerKickEvent.Cause.CHAT_VALIDATION_FAILED); // Paper - kick event causes // Paper - add proper async disconnect } return optional; @@ -1733,7 +1733,7 @@ + // this.chatSpamThrottler.increment(); + if (!this.chatSpamThrottler.isIncrementAndUnderThreshold() && !this.server.getPlayerList().isOp(this.player.getGameProfile()) && !this.server.isSingleplayerOwner(this.player.getGameProfile())) { + // CraftBukkit end -+ this.disconnect((Component) Component.translatable("disconnect.spam"), org.bukkit.event.player.PlayerKickEvent.Cause.SPAM); // Paper - kick event cause ++ this.disconnectAsync((Component) Component.translatable("disconnect.spam"), org.bukkit.event.player.PlayerKickEvent.Cause.SPAM); // Paper - kick event cause // Paper - add proper async disconnect + } } @@ -1743,7 +1743,7 @@ if (!this.lastSeenMessages.applyOffset(packet.offset())) { ServerGamePacketListenerImpl.LOGGER.warn("Failed to validate message acknowledgements from {}", this.player.getName().getString()); - this.disconnect(ServerGamePacketListenerImpl.CHAT_VALIDATION_FAILED); -+ this.disconnect(ServerGamePacketListenerImpl.CHAT_VALIDATION_FAILED, org.bukkit.event.player.PlayerKickEvent.Cause.CHAT_VALIDATION_FAILED); // Paper - kick event causes ++ this.disconnectAsync(ServerGamePacketListenerImpl.CHAT_VALIDATION_FAILED, org.bukkit.event.player.PlayerKickEvent.Cause.CHAT_VALIDATION_FAILED); // Paper - kick event causes // Paper - add proper async disconnect } } @@ -1835,7 +1835,7 @@ if (i > 4096) { - this.disconnect((Component) Component.translatable("multiplayer.disconnect.too_many_pending_chats")); -+ this.disconnect((Component) Component.translatable("multiplayer.disconnect.too_many_pending_chats"), org.bukkit.event.player.PlayerKickEvent.Cause.TOO_MANY_PENDING_CHATS); // Paper - kick event cause ++ this.disconnectAsync((Component) Component.translatable("multiplayer.disconnect.too_many_pending_chats"), org.bukkit.event.player.PlayerKickEvent.Cause.TOO_MANY_PENDING_CHATS); // Paper - kick event cause // Paper - add proper async disconnect } } @@ -2406,7 +2406,7 @@ + // Paper start - auto recipe limit + if (!org.bukkit.Bukkit.isPrimaryThread()) { + if (!this.recipeSpamPackets.isIncrementAndUnderThreshold()) { -+ this.disconnect(net.minecraft.network.chat.Component.translatable("disconnect.spam"), org.bukkit.event.player.PlayerKickEvent.Cause.SPAM); // Paper - kick event cause ++ this.disconnectAsync(net.minecraft.network.chat.Component.translatable("disconnect.spam"), org.bukkit.event.player.PlayerKickEvent.Cause.SPAM); // Paper - kick event cause // Paper - add proper async disconnect + return; + } + }