SteamWar/Paper
13
0
Fork 0
geforkt von Mirrors/Paper
Code Issues Pull-Requests 1 Projekte Releases Wiki Aktivität
018d860243
Paper/Spigot-Server-Patches/0459-Fix-Chunk-Post-Processing-deadlock-risk.patch

61 Zeilen
3.3 KiB
Diff
Originalformat Normale Ansicht Verlauf

Rebuild all patches using the new rebuild pattern
2020-05-06 11:48:49 +02:00
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
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.
2020-04-18 10:44:35 +02:00
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.
Make sure the chunk conversion task is executed immediately 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.
2020-04-18 17:41:02 +02:00
diff --git a/src/main/java/net/minecraft/server/ChunkProviderServer.java b/src/main/java/net/minecraft/server/ChunkProviderServer.java
Re-Add per player mob spawning
2020-06-27 06:57:36 +02:00
index 91755cd7313d87bae85584dff140acbc6467428f..0080a0cbe58d1a81c434ef97659428c8bf1ec290 100644
Make sure the chunk conversion task is executed immediately 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.
2020-04-18 17:41:02 +02:00
--- a/src/main/java/net/minecraft/server/ChunkProviderServer.java
+++ b/src/main/java/net/minecraft/server/ChunkProviderServer.java
Re-Add per player mob spawning
2020-06-27 06:57:36 +02:00
@@ -994,6 +994,7 @@ public class ChunkProviderServer extends IChunkProvider {
Improve mid tick chunk loading, Fix Oversleep, other improvements Process loads outside of any canSleep check. Original intent was to only apply those restrictions to generations but realized I had some checks higher up the call chain. Reworked the back off strategy to just run every 1 millisecond per world, and to apply the per tick limit to generations only. This guarantees that your chunk will load with at most around 1ms delay. Additionally, fire midTick processing in a few more places, notably the oversleep section so we can keep processing loads here too which has a large up to 50ms window... Speaking of oversleep, we had a bug in our implementation changes for Timings that caused oversleep to not sleep the correct amount. Because we now moved it into the NEXT tick instead of THIS tick, the value of nextTick had already been increased to +50ms, resulting in the risk of sleeping more than it should, but, more importantly, this caused every task that was trying to NOT run during oversleep to actually run during oversleep. This is now fixed. Another small tweak is to the /tps command, to no longer show the star when TPS is right at 20. Due to ineffeciencies in the sleep precision, TPS is commonly 20.02. This causes the star to show up almost constantly, so now only show it if we actually hit a real "catchup". This commit also improves the changes to the CallbackExecutor, in that it now is also recursion safe. It was possible that the executor could run tasks out of desired order if the executor task scheduled more executor tasks. We solve this by ensuring new additions do not enter the currently iterated queue. Each depth level will have its own queue. Fixes #3220
2020-04-26 05:47:29 +02:00
return super.executeNext() || execChunkTask; // Paper
Make sure the chunk conversion task is executed immediately 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.
2020-04-18 17:41:02 +02:00
}
} finally {
Improve mid tick chunk loading, Fix Oversleep, other improvements Process loads outside of any canSleep check. Original intent was to only apply those restrictions to generations but realized I had some checks higher up the call chain. Reworked the back off strategy to just run every 1 millisecond per world, and to apply the per tick limit to generations only. This guarantees that your chunk will load with at most around 1ms delay. Additionally, fire midTick processing in a few more places, notably the oversleep section so we can keep processing loads here too which has a large up to 50ms window... Speaking of oversleep, we had a bug in our implementation changes for Timings that caused oversleep to not sleep the correct amount. Because we now moved it into the NEXT tick instead of THIS tick, the value of nextTick had already been increased to +50ms, resulting in the risk of sleeping more than it should, but, more importantly, this caused every task that was trying to NOT run during oversleep to actually run during oversleep. This is now fixed. Another small tweak is to the /tps command, to no longer show the star when TPS is right at 20. Due to ineffeciencies in the sleep precision, TPS is commonly 20.02. This causes the star to show up almost constantly, so now only show it if we actually hit a real "catchup". This commit also improves the changes to the CallbackExecutor, in that it now is also recursion safe. It was possible that the executor could run tasks out of desired order if the executor task scheduled more executor tasks. We solve this by ensuring new additions do not enter the currently iterated queue. Each depth level will have its own queue. Fixes #3220
2020-04-26 05:47:29 +02:00
+ playerChunkMap.chunkLoadConversionCallbackExecutor.run(); // Paper - Add chunk load conversion callback executor to prevent deadlock due to recursion in the chunk task queue sorter
Make sure the chunk conversion task is executed immediately 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.
2020-04-18 17:41:02 +02:00
playerChunkMap.callbackExecutor.run();
Improve mid tick chunk loading, Fix Oversleep, other improvements Process loads outside of any canSleep check. Original intent was to only apply those restrictions to generations but realized I had some checks higher up the call chain. Reworked the back off strategy to just run every 1 millisecond per world, and to apply the per tick limit to generations only. This guarantees that your chunk will load with at most around 1ms delay. Additionally, fire midTick processing in a few more places, notably the oversleep section so we can keep processing loads here too which has a large up to 50ms window... Speaking of oversleep, we had a bug in our implementation changes for Timings that caused oversleep to not sleep the correct amount. Because we now moved it into the NEXT tick instead of THIS tick, the value of nextTick had already been increased to +50ms, resulting in the risk of sleeping more than it should, but, more importantly, this caused every task that was trying to NOT run during oversleep to actually run during oversleep. This is now fixed. Another small tweak is to the /tps command, to no longer show the star when TPS is right at 20. Due to ineffeciencies in the sleep precision, TPS is commonly 20.02. This causes the star to show up almost constantly, so now only show it if we actually hit a real "catchup". This commit also improves the changes to the CallbackExecutor, in that it now is also recursion safe. It was possible that the executor could run tasks out of desired order if the executor task scheduled more executor tasks. We solve this by ensuring new additions do not enter the currently iterated queue. Each depth level will have its own queue. Fixes #3220
2020-04-26 05:47:29 +02:00
}
// CraftBukkit end
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.
2020-04-18 10:44:35 +02:00
diff --git a/src/main/java/net/minecraft/server/PlayerChunkMap.java b/src/main/java/net/minecraft/server/PlayerChunkMap.java
Re-Add per player mob spawning
2020-06-27 06:57:36 +02:00
index 09f94bd242318155dbb46e12224ad3e47eed40cf..4ab719930f5b35c0ae221e9345f1e2eda7d9d719 100644
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.
2020-04-18 10:44:35 +02:00
--- a/src/main/java/net/minecraft/server/PlayerChunkMap.java
+++ b/src/main/java/net/minecraft/server/PlayerChunkMap.java
Re-Add per player mob spawning
2020-06-27 06:57:36 +02:00
@@ -133,6 +133,8 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
Make sure the chunk conversion task is executed immediately 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.
2020-04-18 17:41:02 +02:00
};
// 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<>();
Re-Add per player mob spawning
2020-06-27 06:57:36 +02:00
@@ -996,7 +998,7 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
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.
2020-04-18 10:44:35 +02:00
return Either.left(chunk);
});
}, (runnable) -> {
even even even even even even more work
2020-06-26 03:53:21 +02:00
- this.mailboxMain.a(ChunkTaskQueueSorter.a(playerchunk, runnable));
+ this.mailboxMain.a(ChunkTaskQueueSorter.a(playerchunk, () -> PlayerChunkMap.this.chunkLoadConversionCallbackExecutor.execute(runnable))); // Paper - delay running Chunk post processing until outside of the sorter to prevent a deadlock scenario when post processing causes another chunk request.
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.
2020-04-18 10:44:35 +02:00
});
completablefuture1.thenAcceptAsync((either) -> {
In neuem Issue referenzieren Permalink kopieren