From 26fb7cc35aafecd5bf789c8ae4f9c26034527e52 Mon Sep 17 00:00:00 2001 From: Aikar Date: Sat, 18 Apr 2020 04:44:35 -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. --- ...-Chunk-Post-Processing-deadlock-risk.patch | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 Spigot-Server-Patches/0483-Fix-Chunk-Post-Processing-deadlock-risk.patch diff --git a/Spigot-Server-Patches/0483-Fix-Chunk-Post-Processing-deadlock-risk.patch b/Spigot-Server-Patches/0483-Fix-Chunk-Post-Processing-deadlock-risk.patch new file mode 100644 index 000000000..2e6fa844f --- /dev/null +++ b/Spigot-Server-Patches/0483-Fix-Chunk-Post-Processing-deadlock-risk.patch @@ -0,0 +1,42 @@ +From 0ad488560e10639c5fb359999c8b16487780fd21 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. + +diff --git a/src/main/java/net/minecraft/server/PlayerChunkMap.java b/src/main/java/net/minecraft/server/PlayerChunkMap.java +index c38d31faf..12639dfb9 100644 +--- a/src/main/java/net/minecraft/server/PlayerChunkMap.java ++++ b/src/main/java/net/minecraft/server/PlayerChunkMap.java +@@ -1002,7 +1002,7 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d { + return Either.left(chunk); + }); + }, (runnable) -> { +- this.mailboxMain.a(ChunkTaskQueueSorter.a(playerchunk, runnable)); // CraftBukkit - decompile error ++ this.mailboxMain.a(ChunkTaskQueueSorter.a(playerchunk, () -> this.executor.addTask(runnable))); // CraftBukkit - decompile error // Paper - delay running Chunk post processing until outside of the sorter to prevent a deadlock scenario when post processing causes another chunk request. + }); + + completablefuture1.thenAcceptAsync((either) -> { +-- +2.25.1 +