From fa9c2a0cddf8975d4801d93820bd888b42e76a48 Mon Sep 17 00:00:00 2001 From: Aikar Date: Thu, 7 Feb 2019 00:04:29 -0500 Subject: [PATCH] Fix logic error in async chunk loading if a chunk load was cancelled after generating stage started it would short circuit return with a null. however this skipped the creation of the loadTask, which some code would then invoke in requestChunk and trigger an NPE. This then likely left an incomplete corrupt request in the chunk map which then crashes servers. It should fix these isseues Fixes #1775 Fixes #1743 --- .../Async-Chunk-Loading-and-Generation.patch | 27 ++++++++++--------- ...-after-profile-lookups-if-not-needed.patch | 26 +++++++++--------- 2 files changed, 27 insertions(+), 26 deletions(-) diff --git a/Spigot-Server-Patches/Async-Chunk-Loading-and-Generation.patch b/Spigot-Server-Patches/Async-Chunk-Loading-and-Generation.patch index 2286b2d9f2..6ed6688f04 100644 --- a/Spigot-Server-Patches/Async-Chunk-Loading-and-Generation.patch +++ b/Spigot-Server-Patches/Async-Chunk-Loading-and-Generation.patch @@ -1070,7 +1070,7 @@ index 68743a6b7..1dbb03a9a 100644 if (true || worldserver.worldProvider.getDimensionManager() == DimensionManager.OVERWORLD || this.getAllowNether()) { // CraftBukkit diff --git a/src/main/java/net/minecraft/server/PaperAsyncChunkProvider.java b/src/main/java/net/minecraft/server/PaperAsyncChunkProvider.java new file mode 100644 -index 000000000..e9a38f9d9 +index 000000000..896fc94a6 --- /dev/null +++ b/src/main/java/net/minecraft/server/PaperAsyncChunkProvider.java @@ -0,0 +0,0 @@ @@ -1617,6 +1617,18 @@ index 000000000..e9a38f9d9 + } + + synchronized PendingChunkRequest addListener(CompletableFuture future, boolean gen, boolean autoSubmit) { ++ requests.incrementAndGet(); ++ if (loadTask == null) { ++ // Take care of a race condition in that a request could be cancelled after the synchronize ++ // on pendingChunks, but before a listener is added, which would erase these pending tasks. ++ genTask = generationExecutor.createPendingTask(this::generateChunk, taskPriority); ++ loadTask = EXECUTOR.createPendingTask(this, taskPriority); ++ if (autoSubmit) { ++ // We will execute it outside of the synchronized context immediately after ++ loadTask.submit(); ++ } ++ } ++ + if (hasFinished) { + future.complete(chunk); + return new PendingChunkRequest(this); @@ -1632,17 +1644,6 @@ index 000000000..e9a38f9d9 + } + } + -+ requests.incrementAndGet(); -+ if (loadTask == null) { -+ // Take care of a race condition in that a request could be cancelled after the synchronize -+ // on pendingChunks, but before a listener is added, which would erase these pending tasks. -+ genTask = generationExecutor.createPendingTask(this::generateChunk, taskPriority); -+ loadTask = EXECUTOR.createPendingTask(this, taskPriority); -+ if (autoSubmit) { -+ // We will execute it outside of the synchronized context immediately after -+ loadTask.submit(); -+ } -+ } + return new PendingChunkRequest(this, gen); + } + @@ -2315,7 +2316,7 @@ index a9fffa544..19ce77c4a 100644 } diff --git a/src/main/java/org/bukkit/craftbukkit/CraftServer.java b/src/main/java/org/bukkit/craftbukkit/CraftServer.java -index 5fca9e4bd..78adc8714 100644 +index 8dccf9498..75c4592c2 100644 --- a/src/main/java/org/bukkit/craftbukkit/CraftServer.java +++ b/src/main/java/org/bukkit/craftbukkit/CraftServer.java @@ -0,0 +0,0 @@ public final class CraftServer implements Server { diff --git a/Spigot-Server-Patches/Don-t-sleep-after-profile-lookups-if-not-needed.patch b/Spigot-Server-Patches/Don-t-sleep-after-profile-lookups-if-not-needed.patch index 3e96dbafbe..83275ab228 100644 --- a/Spigot-Server-Patches/Don-t-sleep-after-profile-lookups-if-not-needed.patch +++ b/Spigot-Server-Patches/Don-t-sleep-after-profile-lookups-if-not-needed.patch @@ -7,28 +7,28 @@ Mojang was sleeping even if we had no more requests to go after the current one finished, resulting in 100ms lost per profile lookup diff --git a/src/main/java/com/mojang/authlib/yggdrasil/YggdrasilGameProfileRepository.java b/src/main/java/com/mojang/authlib/yggdrasil/YggdrasilGameProfileRepository.java -index 26a743722..6ed3199c3 100644 +index 71e48e87b..23f1447cf 100644 --- a/src/main/java/com/mojang/authlib/yggdrasil/YggdrasilGameProfileRepository.java +++ b/src/main/java/com/mojang/authlib/yggdrasil/YggdrasilGameProfileRepository.java @@ -0,0 +0,0 @@ public class YggdrasilGameProfileRepository implements GameProfileRepository { - } - - final int page = 0; + } + + final int page = 0; + boolean hasRequested = false; // Paper - - for (final List request : Iterables.partition(criteria, ENTRIES_PER_PAGE)) { - int failCount = 0; + + for (final List request : Iterables.partition(criteria, ENTRIES_PER_PAGE)) { + int failCount = 0; @@ -0,0 +0,0 @@ public class YggdrasilGameProfileRepository implements GameProfileRepository { - LOGGER.debug("Couldn't find profile {}", name); - callback.onProfileLookupFailed(new GameProfile(null, name), new ProfileNotFoundException("Server did not find the requested profile")); - } + LOGGER.debug("Couldn't find profile {}", name); + callback.onProfileLookupFailed(new GameProfile(null, name), new ProfileNotFoundException("Server did not find the requested profile")); + } + // Paper start + if (!hasRequested) { + hasRequested = true; + continue; + } + // Paper end - - try { - Thread.sleep(DELAY_BETWEEN_PAGES); + + try { + Thread.sleep(DELAY_BETWEEN_PAGES); -- \ No newline at end of file