From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Aikar Date: Fri, 29 May 2020 20:29:02 -0400 Subject: [PATCH] Synchronize PalettedContainer 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/world/level/chunk/PalettedContainer.java b/src/main/java/net/minecraft/world/level/chunk/PalettedContainer.java index ac51089aae57a5f1d2411367ff177e058702894c..554474d4b2e57d8a005b3c3b9b23f32a62243058 100644 --- a/src/main/java/net/minecraft/world/level/chunk/PalettedContainer.java +++ b/src/main/java/net/minecraft/world/level/chunk/PalettedContainer.java @@ -37,16 +37,18 @@ public class PalettedContainer implements PaletteResize { private final DebugBuffer> traces = null; public void acquire() { + /* // Paper start - disable this - use proper synchronization if (this.traces != null) { Thread thread = Thread.currentThread(); this.traces.push(Pair.of(thread, thread.getStackTrace())); } ThreadingDetector.checkAndLock(this.lock, this.traces, "PalettedContainer"); + */ // Paper end } public void release() { - this.lock.release(); + //this.lock.release(); // Paper - disable this } // Paper start - Anti-Xray - Add preset values @@ -129,7 +131,7 @@ public class PalettedContainer implements PaletteResize { return this.palette.idFor(objectAdded); } - public T getAndSet(int x, int y, int z, T value) { + public synchronized T getAndSet(int x, int y, int z, T value) { // Paper - synchronize Object var6; try { this.acquire(); @@ -153,7 +155,7 @@ public class PalettedContainer implements PaletteResize { return (T)(object == null ? this.defaultValue : object); } - public void set(int i, int j, int k, T object) { + public synchronized void set(int i, int j, int k, T object) { // Paper - synchronize try { this.acquire(); this.set(getIndex(i, j, k), object); @@ -177,7 +179,7 @@ public class PalettedContainer implements PaletteResize { return (T)(object == null ? this.defaultValue : object); } - public void read(FriendlyByteBuf buf) { + public synchronized void read(FriendlyByteBuf buf) { // Paper - synchronize try { this.acquire(); int i = buf.readByte(); @@ -201,7 +203,7 @@ public class PalettedContainer implements PaletteResize { // Paper start - Anti-Xray - Add chunk packet info @Deprecated public void write(FriendlyByteBuf buf) { write(buf, null, 0); } // Notice for updates: Please make sure this method isn't used anywhere - public void write(FriendlyByteBuf buf, com.destroystokyo.paper.antixray.ChunkPacketInfo chunkPacketInfo, int bottomBlockY) { + public synchronized void write(FriendlyByteBuf buf, com.destroystokyo.paper.antixray.ChunkPacketInfo chunkPacketInfo, int bottomBlockY) { // Paper - synchronize // Paper end try { this.acquire(); @@ -226,7 +228,7 @@ public class PalettedContainer implements PaletteResize { } - public void read(ListTag paletteNbt, long[] data) { + public synchronized void read(ListTag paletteNbt, long[] data) { // Paper - synchronize try { this.acquire(); // Paper - Anti-Xray - TODO: Should this.presetValues.length just be added here (faster) or should the contents be compared to calculate the size (less RAM)? @@ -261,7 +263,7 @@ public class PalettedContainer implements PaletteResize { } - public void write(CompoundTag nbt, String paletteKey, String dataKey) { + public synchronized void write(CompoundTag nbt, String paletteKey, String dataKey) { // Paper - synchronize try { this.acquire(); HashMapPalette hashMapPalette = new HashMapPalette<>(this.registry, this.bits, this.dummyPaletteResize, this.reader, this.writer);