85 lines
4.9 KiB
Diff
85 lines
4.9 KiB
Diff
From 7efd9e913ddf547057c364e553e4a79c69c4c397 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 59568224..fd84807a 100644
|
|
--- a/src/main/java/net/minecraft/server/ChunkProviderServer.java
|
|
+++ b/src/main/java/net/minecraft/server/ChunkProviderServer.java
|
|
@@ -1017,6 +1017,7 @@ public class ChunkProviderServer extends IChunkProvider {
|
|
// we do not want to use this.executeNext as that also processes chunk loads and might count against task counter.
|
|
// We also have already ticked the distance manager above too.
|
|
if (server.chunksTasksRan < 200 && now - lastChunkTask > 100000 && super.executeNext()) {
|
|
+ ChunkProviderServer.this.playerChunkMap.chunkLoadConversionCallbackExecutor.run(); // run immediately after a task is potentially queued
|
|
ChunkProviderServer.this.lightEngine.queueUpdate();
|
|
server.chunksTasksRan++;
|
|
lastChunkTask = now;
|
|
@@ -1040,7 +1041,11 @@ public class ChunkProviderServer extends IChunkProvider {
|
|
return true;
|
|
} else {
|
|
ChunkProviderServer.this.lightEngine.queueUpdate();
|
|
- return super.executeNext() || execChunkTask; // Paper
|
|
+ // Paper start - Add chunk load conversion callback executor to prevent deadlock due to recursion in the chunk task queue sorter
|
|
+ boolean executed = super.executeNext();
|
|
+ ChunkProviderServer.this.playerChunkMap.chunkLoadConversionCallbackExecutor.run(); // run immediately after a task is potentially queued
|
|
+ return executed || execChunkTask;
|
|
+ // Paper end - Add chunk load conversion callback executor to prevent deadlock due to recursion in the chunk task queue sorter
|
|
}
|
|
} finally {
|
|
playerChunkMap.callbackExecutor.run();
|
|
diff --git a/src/main/java/net/minecraft/server/PlayerChunkMap.java b/src/main/java/net/minecraft/server/PlayerChunkMap.java
|
|
index c38d31fa..e19342eb 100644
|
|
--- a/src/main/java/net/minecraft/server/PlayerChunkMap.java
|
|
+++ b/src/main/java/net/minecraft/server/PlayerChunkMap.java
|
|
@@ -92,6 +92,7 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
|
|
@Override
|
|
public void execute(Runnable runnable) {
|
|
if (queued != null) {
|
|
+ MinecraftServer.LOGGER.fatal("Failed to schedule runnable", new IllegalStateException("Already queued")); // Paper - make sure this is printed
|
|
throw new IllegalStateException("Already queued");
|
|
}
|
|
queued = runnable;
|
|
@@ -108,6 +109,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<>();
|
|
|
|
@@ -1002,7 +1005,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) -> {
|
|
--
|
|
2.25.1.windows.1
|
|
|