Upstream
/
Paper
mirror of https://github.com/PaperMC/Paper.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

#640: Fix chunk load/unload callbacks for chunk load cancellations 

When a chunk goes from a ticket level where it is loading a
full chunk to an inactive state (i.e ticket level 33 to
ticket level 45) the full status future will be completed
with a "Right" Either (indicating unloaded). However, this
will also schedule the unload callback immediately.

However, the callback is not immediately executed. This means
the next unload/load callback that needs to be scheduled will
fail. The fix applied is to not schedule a callback if the
chunk is not loaded - if the Either is "right."

Even better, due to how completablefuture works, exceptions
are not printed by default. So the exception thrown by the
callback executor was not printed and the failure
hidden from console. This explains why no-one has tracked this issue.
Now the exception is printed so future failures with the
callback system (if any) can be tracked easier.
This commit is contained in:
Spottedleaf 2020-03-02 17:19:06 -08:00 committed by md_5
parent 13ed05decd
commit 7f61a2526e
1 changed files with 31 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
nms-patches/PlayerChunk.patch
View File

@ -46,23 +46,29 @@
if (either == null || either.left().isPresent()) {
return completablefuture;
@@ -278,6 +287,24 @@
@@ -278,6 +287,30 @@
boolean flag1 = this.ticketLevel <= PlayerChunkMap.GOLDEN_TICKET;
PlayerChunk.State playerchunk_state = getChunkState(this.oldTicketLevel);
PlayerChunk.State playerchunk_state1 = getChunkState(this.ticketLevel);
+ // CraftBukkit start
+ // ChunkUnloadEvent: Called before the chunk is unloaded: isChunkLoaded is still true and chunk can still be modified by plugins.
+ if (playerchunk_state.isAtLeast(PlayerChunk.State.BORDER) && !playerchunk_state1.isAtLeast(PlayerChunk.State.BORDER)) {
+ this.getStatusFutureUnchecked(ChunkStatus.FULL).thenAcceptAsync((either) -> {
+ either.ifLeft((chunkAccess) -> {
+ Chunk chunk = (Chunk) chunkAccess;
+ // Minecraft will apply the chunks tick lists to the world once the chunk got loaded, and then store the tick
+ // lists again inside the chunk once the chunk becomes inaccessible and set the chunk's needsSaving flag.
+ // These actions may however happen deferred, so we manually set the needsSaving flag already here.
+ chunk.setNeedsSaving(true);
+ chunk.unloadCallback();
+ });
+ }, playerchunkmap.callbackExecutor);
+ this.getStatusFutureUnchecked(ChunkStatus.FULL).thenAccept((either) -> {
+ Chunk chunk = (Chunk)either.left().orElse(null);
+ if (chunk != null) {
+ playerchunkmap.callbackExecutor.execute(() -> {
+ // Minecraft will apply the chunks tick lists to the world once the chunk got loaded, and then store the tick
+ // lists again inside the chunk once the chunk becomes inaccessible and set the chunk's needsSaving flag.
+ // These actions may however happen deferred, so we manually set the needsSaving flag already here.
+ chunk.setNeedsSaving(true);
+ chunk.unloadCallback();
+ });
+ }
+ }).exceptionally((throwable) -> {
+ // ensure exceptions are printed, by default this is not the case
+ MinecraftServer.LOGGER.fatal("Failed to schedule unload callback for chunk " + PlayerChunk.this.location, throwable);
+ return null;
+ });
+
+ // Run callback right away if the future was already done
+ playerchunkmap.callbackExecutor.run();
@ -71,7 +77,7 @@
CompletableFuture completablefuture;
if (flag) {
@@ -309,7 +336,7 @@
@@ -309,7 +342,7 @@
if (flag2 && !flag3) {
completablefuture = this.fullChunkFuture;
this.fullChunkFuture = PlayerChunk.UNLOADED_CHUNK_FUTURE;
@ -80,19 +86,25 @@
playerchunkmap.getClass();
return either1.ifLeft(playerchunkmap::a);
}));
@@ -347,6 +374,20 @@
@@ -347,6 +380,26 @@
this.w.a(this.location, this::k, this.ticketLevel, this::d);
this.oldTicketLevel = this.ticketLevel;
+ // CraftBukkit start
+ // ChunkLoadEvent: Called after the chunk is loaded: isChunkLoaded returns true and chunk is ready to be modified by plugins.
+ if (!playerchunk_state.isAtLeast(PlayerChunk.State.BORDER) && playerchunk_state1.isAtLeast(PlayerChunk.State.BORDER)) {
+ this.getStatusFutureUnchecked(ChunkStatus.FULL).thenAcceptAsync((either) -> {
+ either.ifLeft((chunkAccess) -> {
+ Chunk chunk = (Chunk) chunkAccess;
+ chunk.loadCallback();
+ });
+ }, playerchunkmap.callbackExecutor);
+ this.getStatusFutureUnchecked(ChunkStatus.FULL).thenAccept((either) -> {
+ Chunk chunk = (Chunk)either.left().orElse(null);
+ if (chunk != null) {
+ playerchunkmap.callbackExecutor.execute(() -> {
+ chunk.loadCallback();
+ });
+ }
+ }).exceptionally((throwable) -> {
+ // ensure exceptions are printed, by default this is not the case
+ MinecraftServer.LOGGER.fatal("Failed to schedule load callback for chunk " + PlayerChunk.this.location, throwable);
+ return null;
+ });
+
+ // Run callback right away if the future was already done
+ playerchunkmap.callbackExecutor.run();