mirror of
https://github.com/PaperMC/Paper.git
synced 2024-07-02 17:54:57 +02:00
c11668aca1
Appending to the tail of the chunk tasks leaves a window for the chunk to be moved to a non-ticking status. Additionally, use CB's callback executor so we can ensure that we are not incorrectly scheduling.
85 lines
4.9 KiB
Diff
85 lines
4.9 KiB
Diff
From 5895b7d41d8065551cc27a225ccf2c5494d3b491 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 ba6bdc40a..a72f821f2 100644
|
|
--- a/src/main/java/net/minecraft/server/ChunkProviderServer.java
|
|
+++ b/src/main/java/net/minecraft/server/ChunkProviderServer.java
|
|
@@ -938,6 +938,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;
|
|
@@ -961,7 +962,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 c38d31faf..e19342eb8 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.26.0
|
|
|