Paper/Spigot-Server-Patches/0481-Fix-Chunk-Post-Processing-deadlock-risk.patch
Aikar 210a32f26f
Unload leaked Cached Chunks
Due to some complexity in mojangs complicated chain of juggling
whether or not a chunk should be unloaded when the last ticket is
removed, many chunks are remaining around in the cache.

These chunks are never being targetted for unload because they are
vastly out of view distance range and have no reason to be looked at.

This is a huge issue for performance because we have to iterate these
chunks EVERY TICK... This is what's been leading to high SELF time in
Ticking Chunks timings/profiler results.

We will now detect these chunks in that iteration, and automatically
add it to the unload queue when the chunk is found without any tickets.
2020-05-25 11:12:22 -04:00

61 lines
3.3 KiB
Diff

From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Aikar <aikar@aikar.co>
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/ChunkProviderServer.java b/src/main/java/net/minecraft/server/ChunkProviderServer.java
index 722db939971fe395d8250c388fbd7f3b5e87804d..ba99e9949c6fdfc4f49b6b6716100eb51697227d 100644
--- a/src/main/java/net/minecraft/server/ChunkProviderServer.java
+++ b/src/main/java/net/minecraft/server/ChunkProviderServer.java
@@ -1040,6 +1040,7 @@ public class ChunkProviderServer extends IChunkProvider {
return super.executeNext() || execChunkTask; // Paper
}
} finally {
+ playerChunkMap.chunkLoadConversionCallbackExecutor.run(); // Paper - Add chunk load conversion callback executor to prevent deadlock due to recursion in the chunk task queue sorter
playerChunkMap.callbackExecutor.run();
}
// CraftBukkit end
diff --git a/src/main/java/net/minecraft/server/PlayerChunkMap.java b/src/main/java/net/minecraft/server/PlayerChunkMap.java
index 00ad80bb74bd581e3fa1bf82356ee5b7bc656bfe..9e2998f2214d6129e38acc0cbb59ce19f4c38759 100644
--- a/src/main/java/net/minecraft/server/PlayerChunkMap.java
+++ b/src/main/java/net/minecraft/server/PlayerChunkMap.java
@@ -131,6 +131,8 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
};
// CraftBukkit end
+ final CallbackExecutor chunkLoadConversionCallbackExecutor = new CallbackExecutor(); // Paper
+
// Paper start - distance maps
private final com.destroystokyo.paper.util.misc.PooledLinkedHashSets<EntityPlayer> pooledLinkedPlayerHashSets = new com.destroystokyo.paper.util.misc.PooledLinkedHashSets<>();
@@ -987,7 +989,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, () -> PlayerChunkMap.this.chunkLoadConversionCallbackExecutor.execute(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) -> {