mirrors/papermc
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Make sure the chunk conversion task is executed immediately

Browse source 
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.
This commit is contained in:
Spottedleaf 2020-04-18 08:41:02 -07:00 committed by Aikar
parent 26fb7cc35a
commit c11668aca1
No known key found for this signature in database
GPG key ID: 401ADFC9891FAAFE
1 changed files with 47 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

52
Spigot-Server-Patches/0483-Fix-Chunk-Post-Processing-deadlock-risk.patch
View file

@ -1,4 +1,4 @@
From 0ad488560e10639c5fb359999c8b16487780fd21 Mon Sep 17 00:00:00 2001
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
@ -24,19 +24,61 @@ 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..12639dfb9 100644
index c38d31faf..e19342eb8 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 {
@@ -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, () -> 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.
+ 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
2.26.0