Merge pull request #919 from electronicboy/improve-keepalive-handling

allow keepalive to wait longer for a client response (#895)
This commit is contained in:
Shane Freeder 2017-10-16 16:58:44 +01:00 committed by GitHub
commit 50d69c6328
5 changed files with 98 additions and 28 deletions

View file

@ -1,4 +1,4 @@
From 6b1f6a4395784e4baa167d1552735368f74404a2 Mon Sep 17 00:00:00 2001
From f3d56b8ac9188096f2ada41c09f03d05f57f490f Mon Sep 17 00:00:00 2001
From: Aikar <aikar@aikar.co>
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

View file

@ -1,4 +1,4 @@
From ff2be849ba11f9a48cf7c1f157cf31114231cb6f Mon Sep 17 00:00:00 2001
From 0182b294be4001dd1821f7900ee729adc6bdaca7 Mon Sep 17 00:00:00 2001
From: Aikar <aikar@aikar.co>
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) {

View file

@ -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 <alfeh@me.com>
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() {

View file

@ -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 <theboyetronic@gmail.com>
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]));
+ });

View file

@ -0,0 +1,54 @@
From 584178f424f4af724835fd985b7c841bde8a3998 Mon Sep 17 00:00:00 2001
From: Shane Freeder <theboyetronic@gmail.com>
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