From 64ef6a6233d32906a3d21c37d43f7c9b9bd2bf61 Mon Sep 17 00:00:00 2001 From: Shane Freeder Date: Sun, 15 Oct 2017 14:00:42 +0100 Subject: [PATCH] allow keepalive to wait longer for a client response and also provide a bit more information in the server logs so we can work out what is going on. --- Spigot-Server-Patches/0005-MC-Utils.patch | 27 +++++++++- .../0184-IllegalPacketEvent.patch | 14 ++--- ...85-Properly-fix-item-duplication-bug.patch | 6 +-- ...-handle-PacketPlayInKeepAlive-async.patch} | 25 +++------ ...e-time-allowed-for-a-keepalive-reply.patch | 54 +++++++++++++++++++ 5 files changed, 98 insertions(+), 28 deletions(-) rename Spigot-Server-Patches/{0242-handle-PacketPlayInKeepAlive-async-and-revert-keepal.patch => 0242-handle-PacketPlayInKeepAlive-async.patch} (66%) create mode 100644 Spigot-Server-Patches/0245-Increase-time-allowed-for-a-keepalive-reply.patch diff --git a/Spigot-Server-Patches/0005-MC-Utils.patch b/Spigot-Server-Patches/0005-MC-Utils.patch index 6cde1ce2fd..93465ae6ff 100644 --- a/Spigot-Server-Patches/0005-MC-Utils.patch +++ b/Spigot-Server-Patches/0005-MC-Utils.patch @@ -1,4 +1,4 @@ -From 6b1f6a4395784e4baa167d1552735368f74404a2 Mon Sep 17 00:00:00 2001 +From f3d56b8ac9188096f2ada41c09f03d05f57f490f Mon Sep 17 00:00:00 2001 From: Aikar Date: Mon, 28 Mar 2016 20:55:47 -0400 Subject: [PATCH] MC Utils @@ -273,6 +273,31 @@ index e0cb6aa6e..bc6383669 100644 private byte type = 0; public NBTTagList() {} +diff --git a/src/main/java/net/minecraft/server/PlayerConnection.java b/src/main/java/net/minecraft/server/PlayerConnection.java +index aa9d024fa..516673c0e 100644 +--- a/src/main/java/net/minecraft/server/PlayerConnection.java ++++ b/src/main/java/net/minecraft/server/PlayerConnection.java +@@ -65,9 +65,9 @@ public class PlayerConnection implements PacketListenerPlayIn, ITickable { + private final MinecraftServer minecraftServer; + public EntityPlayer player; + private int e; +- private long f; +- private boolean g; +- private long h; ++ private long f; private void setLastPing(long lastPing) { this.f = lastPing;}; private long getLastPing() { return this.f;}; // Paper - OBFHELPER ++ private boolean g; private void setPendingPing(boolean isPending) { this.g = isPending;}; private boolean isPendingPing() { return this.g;}; // Paper - OBFHELPER ++ private long h; private void setKeepAliveID(long keepAliveID) { this.h = keepAliveID;}; private long getKeepAliveID() {return this.h; }; // Paper - OBFHELPER + // CraftBukkit start - multithreaded fields + private volatile int chatThrottle; + private static final AtomicIntegerFieldUpdater chatSpamField = AtomicIntegerFieldUpdater.newUpdater(PlayerConnection.class, "chatThrottle"); +@@ -2153,6 +2153,7 @@ public class PlayerConnection implements PacketListenerPlayIn, ITickable { + + } + ++ private long getCurrentMillis() { return d(); } // Paper - OBFHELPER + private long d() { + return System.nanoTime() / 1000000L; + } -- 2.14.2 diff --git a/Spigot-Server-Patches/0184-IllegalPacketEvent.patch b/Spigot-Server-Patches/0184-IllegalPacketEvent.patch index d454a2eeaf..852beb99c4 100644 --- a/Spigot-Server-Patches/0184-IllegalPacketEvent.patch +++ b/Spigot-Server-Patches/0184-IllegalPacketEvent.patch @@ -1,4 +1,4 @@ -From ff2be849ba11f9a48cf7c1f157cf31114231cb6f Mon Sep 17 00:00:00 2001 +From 0182b294be4001dd1821f7900ee729adc6bdaca7 Mon Sep 17 00:00:00 2001 From: Aikar Date: Thu, 23 Jun 2016 23:33:57 -0400 Subject: [PATCH] IllegalPacketEvent @@ -6,7 +6,7 @@ Subject: [PATCH] IllegalPacketEvent Fired for invalid data from players that represents hacking attempts diff --git a/src/main/java/net/minecraft/server/PlayerConnection.java b/src/main/java/net/minecraft/server/PlayerConnection.java -index 02b5e3cad..52e8458ab 100644 +index 9f05c7da2..a04df7d88 100644 --- a/src/main/java/net/minecraft/server/PlayerConnection.java +++ b/src/main/java/net/minecraft/server/PlayerConnection.java @@ -56,6 +56,7 @@ import org.bukkit.inventory.CraftingInventory; @@ -17,7 +17,7 @@ index 02b5e3cad..52e8458ab 100644 import co.aikar.timings.MinecraftTimings; // Paper // CraftBukkit end -@@ -2282,8 +2283,7 @@ public class PlayerConnection implements PacketListenerPlayIn, ITickable { +@@ -2283,8 +2284,7 @@ public class PlayerConnection implements PacketListenerPlayIn, ITickable { CraftEventFactory.handleEditBookEvent(player, itemstack1); // CraftBukkit } } catch (Exception exception) { @@ -27,7 +27,7 @@ index 02b5e3cad..52e8458ab 100644 } } else { String s1; -@@ -2332,8 +2332,7 @@ public class PlayerConnection implements PacketListenerPlayIn, ITickable { +@@ -2333,8 +2333,7 @@ public class PlayerConnection implements PacketListenerPlayIn, ITickable { CraftEventFactory.handleEditBookEvent(player, itemstack2); // CraftBukkit } } catch (Exception exception1) { @@ -37,7 +37,7 @@ index 02b5e3cad..52e8458ab 100644 } } else if ("MC|TrSel".equals(s)) { try { -@@ -2344,8 +2343,7 @@ public class PlayerConnection implements PacketListenerPlayIn, ITickable { +@@ -2345,8 +2344,7 @@ public class PlayerConnection implements PacketListenerPlayIn, ITickable { ((ContainerMerchant) container).d(j); } } catch (Exception exception2) { @@ -47,7 +47,7 @@ index 02b5e3cad..52e8458ab 100644 } } else { TileEntity tileentity; -@@ -2486,8 +2484,7 @@ public class PlayerConnection implements PacketListenerPlayIn, ITickable { +@@ -2487,8 +2485,7 @@ public class PlayerConnection implements PacketListenerPlayIn, ITickable { iinventory.update(); } } catch (Exception exception5) { @@ -57,7 +57,7 @@ index 02b5e3cad..52e8458ab 100644 } } } else if ("MC|ItemName".equals(s)) { -@@ -2586,8 +2583,7 @@ public class PlayerConnection implements PacketListenerPlayIn, ITickable { +@@ -2587,8 +2584,7 @@ public class PlayerConnection implements PacketListenerPlayIn, ITickable { this.player.playerConnection.sendPacket(new PacketPlayOutSetSlot(-2, k, this.player.inventory.getItem(k))); this.player.playerConnection.sendPacket(new PacketPlayOutHeldItemSlot(this.player.inventory.itemInHandIndex)); } catch (Exception exception7) { diff --git a/Spigot-Server-Patches/0185-Properly-fix-item-duplication-bug.patch b/Spigot-Server-Patches/0185-Properly-fix-item-duplication-bug.patch index 30f0f06255..ac15a38d84 100644 --- a/Spigot-Server-Patches/0185-Properly-fix-item-duplication-bug.patch +++ b/Spigot-Server-Patches/0185-Properly-fix-item-duplication-bug.patch @@ -1,4 +1,4 @@ -From 2b98398add2b76cd4ee71f4f4733cf3fa8efdefa Mon Sep 17 00:00:00 2001 +From abaa022c2bdabf7f90dba095f049b34d509858b6 Mon Sep 17 00:00:00 2001 From: Alfie Cleveland Date: Tue, 27 Dec 2016 01:57:57 +0000 Subject: [PATCH] Properly fix item duplication bug @@ -19,10 +19,10 @@ index 53147c6e2..5fbb99b7e 100644 @Override diff --git a/src/main/java/net/minecraft/server/PlayerConnection.java b/src/main/java/net/minecraft/server/PlayerConnection.java -index 52e8458ab..bcc6c9707 100644 +index a04df7d88..22ead6533 100644 --- a/src/main/java/net/minecraft/server/PlayerConnection.java +++ b/src/main/java/net/minecraft/server/PlayerConnection.java -@@ -2626,6 +2626,6 @@ public class PlayerConnection implements PacketListenerPlayIn, ITickable { +@@ -2627,6 +2627,6 @@ public class PlayerConnection implements PacketListenerPlayIn, ITickable { // CraftBukkit start - Add "isDisconnected" method public final boolean isDisconnected() { diff --git a/Spigot-Server-Patches/0242-handle-PacketPlayInKeepAlive-async-and-revert-keepal.patch b/Spigot-Server-Patches/0242-handle-PacketPlayInKeepAlive-async.patch similarity index 66% rename from Spigot-Server-Patches/0242-handle-PacketPlayInKeepAlive-async-and-revert-keepal.patch rename to Spigot-Server-Patches/0242-handle-PacketPlayInKeepAlive-async.patch index ffec6085d8..2a361f890f 100644 --- a/Spigot-Server-Patches/0242-handle-PacketPlayInKeepAlive-async-and-revert-keepal.patch +++ b/Spigot-Server-Patches/0242-handle-PacketPlayInKeepAlive-async.patch @@ -1,7 +1,7 @@ -From 0e86c660e9a4442f15351c28b2beb151d70af09f Mon Sep 17 00:00:00 2001 +From 38a1ad0ace2a044aae7d2b04e679d74b6c7d5a42 Mon Sep 17 00:00:00 2001 From: Shane Freeder Date: Thu, 5 Oct 2017 01:54:07 +0100 -Subject: [PATCH] handle PacketPlayInKeepAlive async and revert keepalive limit +Subject: [PATCH] handle PacketPlayInKeepAlive async In 1.12.2, Mojang moved the processing of PacketPlayInKeepAlive off the main thread, while entirely correct for the server, this causes issues with @@ -11,25 +11,14 @@ In order to counteract some bad behavior, we will post handling of the disconnection to the main thread, but leave the actual processing of the packet off the main thread. -We also revert the bump on the servers built in keepalive back to 15 seconds, -this solves a read timeout in scenarios where the client isn't sending data to -the server, e.g. spectating an entity. +also adding some additional logging in order to help work out what is causing +random disconnections for clients. diff --git a/src/main/java/net/minecraft/server/PlayerConnection.java b/src/main/java/net/minecraft/server/PlayerConnection.java -index 69ed84af9..b18bf4245 100644 +index a07904143..26fbb30f9 100644 --- a/src/main/java/net/minecraft/server/PlayerConnection.java +++ b/src/main/java/net/minecraft/server/PlayerConnection.java -@@ -180,8 +180,7 @@ public class PlayerConnection implements PacketListenerPlayIn, ITickable { - - this.minecraftServer.methodProfiler.a("keepAlive"); - long i = this.d(); -- -- if (i - this.f >= 25000L) { // CraftBukkit -+ if (i - this.f >= 15000L) { // CraftBukkit // Paper - revert to 15 - if (this.g) { - this.disconnect(new ChatMessage("disconnect.timeout", new Object[0])); - } else { -@@ -2225,14 +2224,18 @@ public class PlayerConnection implements PacketListenerPlayIn, ITickable { +@@ -2225,14 +2225,20 @@ public class PlayerConnection implements PacketListenerPlayIn, ITickable { } public void a(PacketPlayInKeepAlive packetplayinkeepalive) { @@ -43,6 +32,8 @@ index 69ed84af9..b18bf4245 100644 } else if (!this.player.getName().equals(this.minecraftServer.Q())) { - this.disconnect(new ChatMessage("disconnect.timeout", new Object[0])); + // Paper start - This needs to be handled on the main thread for plugins ++ PlayerConnection.LOGGER.warn("{} sent an invalid keepalive! pending keepalive: {} got id: {} expected id: {}", ++ this.player.getName(), this.isPendingPing(), packetplayinkeepalive.a(), this.getKeepAliveID()); + minecraftServer.postToMainThread(() -> { + this.disconnect(new ChatMessage("disconnect.timeout", new Object[0])); + }); diff --git a/Spigot-Server-Patches/0245-Increase-time-allowed-for-a-keepalive-reply.patch b/Spigot-Server-Patches/0245-Increase-time-allowed-for-a-keepalive-reply.patch new file mode 100644 index 0000000000..8f3002067c --- /dev/null +++ b/Spigot-Server-Patches/0245-Increase-time-allowed-for-a-keepalive-reply.patch @@ -0,0 +1,54 @@ +From 584178f424f4af724835fd985b7c841bde8a3998 Mon Sep 17 00:00:00 2001 +From: Shane Freeder +Date: Sun, 15 Oct 2017 00:29:07 +0100 +Subject: [PATCH] Increase time allowed for a keepalive reply + +This patch intends to bump up the time that a client has to reply to the +server back to 30 seconds as per pre 1.12.2, which allowed clients +more than enough time to reply potentially allowing them to be less +tempermental due to lag spikes on the network thread, e.g. that caused +by plugins that are interacting with netty. + +diff --git a/src/main/java/net/minecraft/server/PlayerConnection.java b/src/main/java/net/minecraft/server/PlayerConnection.java +index 26fbb30f9..5bb6d9fac 100644 +--- a/src/main/java/net/minecraft/server/PlayerConnection.java ++++ b/src/main/java/net/minecraft/server/PlayerConnection.java +@@ -179,18 +179,25 @@ public class PlayerConnection implements PacketListenerPlayIn, ITickable { + } + + this.minecraftServer.methodProfiler.a("keepAlive"); +- long i = this.d(); +- +- if (i - this.f >= 25000L) { // CraftBukkit +- if (this.g) { +- this.disconnect(new ChatMessage("disconnect.timeout", new Object[0])); +- } else { +- this.g = true; +- this.f = i; +- this.h = i; +- this.sendPacket(new PacketPlayOutKeepAlive(this.h)); ++ // Paper Start - give clients a longer time to respond to pings as per pre 1.12.2 timings ++ // This should effectively place the keepalive handling back to "as it was" before 1.12.2 ++ long currentTime = this.getCurrentMillis(); ++ long elapsedTime = currentTime - this.getLastPing(); ++ if (this.isPendingPing()) { ++ // We're pending a ping from the client ++ if (elapsedTime >= 30000L) { // 30 seconds for a ping reply ++ PlayerConnection.LOGGER.warn("{} was kicked due to keepalive timeout!", this.player.getName()); // more info ++ this.disconnect(new ChatMessage("disconnect.timeout")); ++ } ++ } else { ++ if (elapsedTime >= 15000L) { // 15 seconds ++ this.setPendingPing(true); ++ this.setLastPing(currentTime); ++ this.setKeepAliveID(currentTime); ++ this.sendPacket(new PacketPlayOutKeepAlive(this.getKeepAliveID())); + } + } ++ // Paper end + + this.minecraftServer.methodProfiler.b(); + // CraftBukkit start +-- +2.14.2 +