Upstream/Paper
1
0
Fork 0
mirror of https://github.com/PaperMC/Paper.git synced 2024-12-29 12:27:59 +01:00
Code Issues Projects Releases Wiki Activity

Synchronize DataPaletteBlock instead of ReentrantLock

Browse Source 
Mojang has flaws in their logic about chunks being concurrently
wrote to. So we constantly see crashes around multiple threads writing.

Additionally, java has optimized synchronization so well that its
in many times faster than trying to manage read wrote locks for low
contention situations.

And this is extremely a low contention situation.

Fixes #3293
Fixes #2493
This commit is contained in:
Aikar 2020-05-29 20:36:42 -04:00
parent 47af5acd7a
commit 7ffde5cb24
3 changed files with 105 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
Spigot-Server-Patches/Anti-Xray.patch
View File

@ -1119,7 +1119,7 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000
private DataPalette<T> h; private DataPalette<T> getDataPalette() { return this.h; } // Paper - OBFHELPER
private int i; private int getBitsPerObject() { return this.i; } // Paper - OBFHELPER
@@ -0,0 +0,0 @@ public class DataPaletteBlock<T> implements DataPaletteExpandable<T> {
this.j.unlock();
//this.j.unlock(); // Paper - disable this
}
- public DataPaletteBlock(DataPalette<T> datapalette, RegistryBlockID<T> registryblockid, Function<NBTTagCompound, T> function, Function<T, NBTTagCompound> function1, T t0) {
@ -1181,12 +1181,12 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000
}
- public void writeDataPaletteBlock(PacketDataSerializer packetDataSerializer) { this.b(packetDataSerializer); } // Paper - OBFHELPER
- public void b(PacketDataSerializer packetdataserializer) {
- public synchronized void b(PacketDataSerializer packetdataserializer) { // Paper - synchronize
+ // Paper start - Anti-Xray - Add chunk packet info
+ @Deprecated public void writeDataPaletteBlock(PacketDataSerializer packetDataSerializer) { this.b(packetDataSerializer); } // OBFHELPER // Notice for updates: Please make sure this method isn't used anywhere
+ @Deprecated public void b(PacketDataSerializer packetdataserializer) { this.writeDataPaletteBlock(packetdataserializer, null, 0); } // Notice for updates: Please make sure this method isn't used anywhere
+ public void writeDataPaletteBlock(PacketDataSerializer packetDataSerializer, ChunkPacketInfo<T> chunkPacketInfo, int chunkSectionIndex) { this.b(packetDataSerializer, chunkPacketInfo, chunkSectionIndex); } // OBFHELPER
+ public void b(PacketDataSerializer packetdataserializer, ChunkPacketInfo<T> chunkPacketInfo, int chunkSectionIndex) {
+ public synchronized void b(PacketDataSerializer packetdataserializer, ChunkPacketInfo<T> chunkPacketInfo, int chunkSectionIndex) { // Paper - synchronize
+ // Paper end
this.a();
packetdataserializer.writeByte(this.i);
@ -1203,7 +1203,7 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000
this.b();
}
public void a(NBTTagList nbttaglist, long[] along) {
public synchronized void a(NBTTagList nbttaglist, long[] along) { // Paper - synchronize
this.a();
- int i = Math.max(4, MathHelper.d(nbttaglist.size()));
+ // Paper - Anti-Xray - TODO: Should this.predefinedObjects.length just be added here (faster) or should the contents be compared to calculate the size (less RAM)?

48
Spigot-Server-Patches/Log-other-thread-in-DataPaletteBlock-lock-failure.patch
View File

@ -1,48 +0,0 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Spottedleaf <Spottedleaf@users.noreply.github.com>
Date: Fri, 21 Jun 2019 14:42:48 -0700
Subject: [PATCH] Log other thread in DataPaletteBlock lock failure
diff --git a/src/main/java/com/destroystokyo/paper/util/ReentrantLockWithGetOwner.java b/src/main/java/com/destroystokyo/paper/util/ReentrantLockWithGetOwner.java
new file mode 100644
index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000
--- /dev/null
+++ b/src/main/java/com/destroystokyo/paper/util/ReentrantLockWithGetOwner.java
@@ -0,0 +0,0 @@
+package com.destroystokyo.paper.util;
+
+import java.util.concurrent.locks.ReentrantLock;
+
+public class ReentrantLockWithGetOwner extends ReentrantLock {
+
+ @Override
+ public Thread getOwner() {
+ return super.getOwner();
+ }
+}
diff --git a/src/main/java/net/minecraft/server/DataPaletteBlock.java b/src/main/java/net/minecraft/server/DataPaletteBlock.java
index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644
--- a/src/main/java/net/minecraft/server/DataPaletteBlock.java
+++ b/src/main/java/net/minecraft/server/DataPaletteBlock.java
@@ -0,0 +0,0 @@ public class DataPaletteBlock<T> implements DataPaletteExpandable<T> {
protected DataBits a; protected DataBits getDataBits() { return this.a; } // Paper - OBFHELPER
private DataPalette<T> h; private DataPalette<T> getDataPalette() { return this.h; } // Paper - OBFHELPER
private int i; private int getBitsPerObject() { return this.i; } // Paper - OBFHELPER
- private final ReentrantLock j = new ReentrantLock();
+ private final com.destroystokyo.paper.util.ReentrantLockWithGetOwner j = new com.destroystokyo.paper.util.ReentrantLockWithGetOwner(); private com.destroystokyo.paper.util.ReentrantLockWithGetOwner getLock() { return this.j; } // Paper - change type to ReentrantLockWithGetOwner // Paper - OBFHELPER
public void a() {
- if (this.j.isLocked() && !this.j.isHeldByCurrentThread()) {
+ // Paper start - log other thread
+ Thread owningThread;
+ if (this.j.isLocked() && (owningThread = this.getLock().getOwner()) != null && owningThread != Thread.currentThread()) {
+ // Paper end
String s = (String) Thread.getAllStackTraces().keySet().stream().filter(Objects::nonNull).map((thread) -> {
return thread.getName() + ": \n\tat " + (String) Arrays.stream(thread.getStackTrace()).map(Object::toString).collect(Collectors.joining("\n\tat "));
}).collect(Collectors.joining("\n"));
- CrashReport crashreport = new CrashReport("Writing into PalettedContainer from multiple threads", new IllegalStateException());
+ CrashReport crashreport = new CrashReport("Writing into PalettedContainer from multiple threads (other thread: name: " + owningThread.getName() + ", class: " + owningThread.getClass().toString() + ")", new IllegalStateException()); // Paper - log other thread
CrashReportSystemDetails crashreportsystemdetails = crashreport.a("Thread dumps");
crashreportsystemdetails.a("Thread dumps", (Object) s);

101
Spigot-Server-Patches/Synchronize-DataPaletteBlock-instead-of-ReentrantLoc.patch Normal file
View File

@ -0,0 +1,101 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Aikar <aikar@aikar.co>
Date: Fri, 29 May 2020 20:29:02 -0400
Subject: [PATCH] Synchronize DataPaletteBlock instead of ReentrantLock
Mojang has flaws in their logic about chunks being concurrently
wrote to. So we constantly see crashes around multiple threads writing.
Additionally, java has optimized synchronization so well that its
in many times faster than trying to manage read wrote locks for low
contention situations.
And this is extremely a low contention situation.
diff --git a/src/main/java/net/minecraft/server/DataPaletteBlock.java b/src/main/java/net/minecraft/server/DataPaletteBlock.java
index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644
--- a/src/main/java/net/minecraft/server/DataPaletteBlock.java
+++ b/src/main/java/net/minecraft/server/DataPaletteBlock.java
@@ -0,0 +0,0 @@ public class DataPaletteBlock<T> implements DataPaletteExpandable<T> {
private int i; private int getBitsPerObject() { return this.i; } // Paper - OBFHELPER
private final ReentrantLock j = new ReentrantLock();
- public void a() {
+ public void a() { /* // Paper start - disable this - use proper synchronization
if (this.j.isLocked() && !this.j.isHeldByCurrentThread()) {
String s = (String) Thread.getAllStackTraces().keySet().stream().filter(Objects::nonNull).map((thread) -> {
return thread.getName() + ": \n\tat " + (String) Arrays.stream(thread.getStackTrace()).map(Object::toString).collect(Collectors.joining("\n\tat "));
@@ -0,0 +0,0 @@ public class DataPaletteBlock<T> implements DataPaletteExpandable<T> {
throw new ReportedException(crashreport);
} else {
this.j.lock();
- }
+ } */ // Paper end
}
public void b() {
- this.j.unlock();
+ //this.j.unlock(); // Paper - disable this
}
public DataPaletteBlock(DataPalette<T> datapalette, RegistryBlockID<T> registryblockid, Function<NBTTagCompound, T> function, Function<T, NBTTagCompound> function1, T t0) {
@@ -0,0 +0,0 @@ public class DataPaletteBlock<T> implements DataPaletteExpandable<T> {
}
@Override
- public int onResize(int i, T t0) {
+ public synchronized int onResize(int i, T t0) { // Paper - synchronize
this.a();
DataBits databits = this.a;
DataPalette<T> datapalette = this.h;
@@ -0,0 +0,0 @@ public class DataPaletteBlock<T> implements DataPaletteExpandable<T> {
}
public T setBlock(int i, int j, int k, T t0) {
- this.a();
- T t1 = this.a(b(i, j, k), t0);
+ //this.a(); // Paper - remove to reduce ops - synchronize handled below
+ return this.a(b(i, j, k), t0); // Paper
- this.b();
- return t1;
+ //this.b(); // Paper
+ //return t1; // PAper
}
public T b(int i, int j, int k, T t0) {
return this.a(b(i, j, k), t0);
}
- protected T a(int i, T t0) {
+ protected synchronized T a(int i, T t0) { // Paper - synchronize - writes
int j = this.h.a(t0);
int k = this.a.a(i, j);
T t1 = this.h.a(k);
@@ -0,0 +0,0 @@ public class DataPaletteBlock<T> implements DataPaletteExpandable<T> {
}
public void writeDataPaletteBlock(PacketDataSerializer packetDataSerializer) { this.b(packetDataSerializer); } // Paper - OBFHELPER
- public void b(PacketDataSerializer packetdataserializer) {
+ public synchronized void b(PacketDataSerializer packetdataserializer) { // Paper - synchronize
this.a();
packetdataserializer.writeByte(this.i);
this.h.b(packetdataserializer);
@@ -0,0 +0,0 @@ public class DataPaletteBlock<T> implements DataPaletteExpandable<T> {
this.b();
}
- public void a(NBTTagList nbttaglist, long[] along) {
+ public synchronized void a(NBTTagList nbttaglist, long[] along) { // Paper - synchronize
this.a();
int i = Math.max(4, MathHelper.d(nbttaglist.size()));
@@ -0,0 +0,0 @@ public class DataPaletteBlock<T> implements DataPaletteExpandable<T> {
this.b();
}
- public void a(NBTTagCompound nbttagcompound, String s, String s1) {
+ public synchronized void a(NBTTagCompound nbttagcompound, String s, String s1) { // Paper - synchronize
this.a();
DataPaletteHash<T> datapalettehash = new DataPaletteHash<>(this.d, this.i, this.c, this.e, this.f);
T t0 = this.g;