From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Aikar Date: Sat, 18 Apr 2020 04:36:11 -0400 Subject: [PATCH] Fix Chunk Post Processing deadlock risk See: https://gist.github.com/aikar/dd22bbd2a3d78a2fd3d92e95e9f28dc6 as part of post processing a chunk, we can call ChunkConverter. ChunkConverter then kicks off major physics updates, and when blocks that have connections across chunk boundries occur, a recursive risk can occur where A updates a block that triggers a physics request. That physics request may trigger a chunk request, that then enqueues a task into the Mailbox ChunkTaskQueueSorter. If anything requests that same chunk that is in the middle of conversion, it's mailbox queue is going to be held up, so the subsequent chunk request will be unable to proceed. We delay post processing of Chunk.A() 1 "pass" by re stuffing it back into the executor so that the mailbox ChunkQueue is now considered empty. This successfully fixed a reoccurring and highly reproduceable crash for heightmaps. TODO FOR LEAF: 1.18.2 changed this OLD CompletableFuture> completablefuture1 = completablefuture.thenApplyAsync((either) -> { return either.flatMap((list) -> { LevelChunk chunk = (LevelChunk) list.get(list.size() / 2); chunk.postProcessGeneration(); this.level.startTickingChunk(chunk); return Either.left(chunk); }); }, (runnable) -> { this.mainThreadMailbox.tell(ChunkTaskPriorityQueueSorter.message(holder, () -> ChunkMap.this.chunkLoadConversionCallbackExecutor.execute(runnable))); // Paper - delay running Chunk post processing until outside of the sorter to prevent a deadlock scenario when post processing causes another chunk request. }); NEW CompletableFuture> completablefuture1 = completablefuture.thenApplyAsync((either) -> { return either.mapLeft((list) -> { return (LevelChunk) list.get(list.size() / 2); }); }, (runnable) -> { this.mainThreadMailbox.tell(ChunkTaskPriorityQueueSorter.message(holder, () -> ChunkMap.this.chunkLoadConversionCallbackExecutor.execute(runnable))); // Paper - delay running Chunk post processing until outside of the sorter to prevent a deadlock scenario when post processing causes another chunk request. }).thenApplyAsync((either) -> { return either.ifLeft((chunk) -> { chunk.postProcessGeneration(); this.level.startTickingChunk(chunk); }); }, this.mainThreadExecutor); diff --git a/src/main/java/net/minecraft/server/level/ChunkMap.java b/src/main/java/net/minecraft/server/level/ChunkMap.java index 7e3f7b69fc7a608dd82b471d832cc401a77f0738..aeae3fb3d05b513031e4af217fe3db7fa3ec4a75 100644 --- a/src/main/java/net/minecraft/server/level/ChunkMap.java +++ b/src/main/java/net/minecraft/server/level/ChunkMap.java @@ -178,6 +178,7 @@ public class ChunkMap extends ChunkStorage implements ChunkHolder.PlayerProvider }; // CraftBukkit end + final CallbackExecutor chunkLoadConversionCallbackExecutor = new CallbackExecutor(); // Paper // Paper start - distance maps private final com.destroystokyo.paper.util.misc.PooledLinkedHashSets pooledLinkedPlayerHashSets = new com.destroystokyo.paper.util.misc.PooledLinkedHashSets<>(); @@ -1012,7 +1013,7 @@ public class ChunkMap extends ChunkStorage implements ChunkHolder.PlayerProvider return (LevelChunk) list.get(list.size() / 2); }); }, (runnable) -> { - this.mainThreadMailbox.tell(ChunkTaskPriorityQueueSorter.message(holder, runnable)); + this.mainThreadMailbox.tell(ChunkTaskPriorityQueueSorter.message(holder, () -> ChunkMap.this.chunkLoadConversionCallbackExecutor.execute(runnable))); // Paper - delay running Chunk post processing until outside of the sorter to prevent a deadlock scenario when post processing causes another chunk request. }).thenApplyAsync((either) -> { return either.ifLeft((chunk) -> { chunk.postProcessGeneration(); diff --git a/src/main/java/net/minecraft/server/level/ServerChunkCache.java b/src/main/java/net/minecraft/server/level/ServerChunkCache.java index cf56b2ed508352205ef6ec71519b0626aa698987..1b6fb81079d3ad5d3c33be67a1c05111f9dd5f2d 100644 --- a/src/main/java/net/minecraft/server/level/ServerChunkCache.java +++ b/src/main/java/net/minecraft/server/level/ServerChunkCache.java @@ -1141,6 +1141,7 @@ public class ServerChunkCache extends ChunkSource { return super.pollTask() || execChunkTask; // Paper } } finally { + chunkMap.chunkLoadConversionCallbackExecutor.run(); // Paper - Add chunk load conversion callback executor to prevent deadlock due to recursion in the chunk task queue sorter chunkMap.callbackExecutor.run(); } // CraftBukkit end