From 989f9b3daa6e44ee177a65cbb183a3345a72ae35 Mon Sep 17 00:00:00 2001 From: Irmo van den Berge Date: Sat, 13 Jul 2019 20:19:44 +0200 Subject: [PATCH] SPIGOT-4849: Fix server crash when accessing chunks during chunk load/unload/populate events --- nms-patches/ChunkProviderServer.patch | 19 +++++++++ nms-patches/PlayerChunk.patch | 20 +++++---- nms-patches/PlayerChunkMap.patch | 58 +++++++++++++++++++++------ 3 files changed, 77 insertions(+), 20 deletions(-) diff --git a/nms-patches/ChunkProviderServer.patch b/nms-patches/ChunkProviderServer.patch index 58c1945976..f3f094d1d5 100644 --- a/nms-patches/ChunkProviderServer.patch +++ b/nms-patches/ChunkProviderServer.patch @@ -102,3 +102,22 @@ if (object2intmap.getInt(enumcreaturetype) <= k1) { SpawnerCreature.a(enumcreaturetype, (World) this.world, chunk, blockposition); +@@ -447,12 +489,18 @@ + + @Override + protected boolean executeNext() { ++ // CraftBukkit start - process pending Chunk loadCallback() and unloadCallback() after each run task ++ try { + if (ChunkProviderServer.this.tickDistanceManager()) { + return true; + } else { + ChunkProviderServer.this.lightEngine.queueUpdate(); + return super.executeNext(); + } ++ } finally { ++ playerChunkMap.callbackExecutor.run(); ++ } ++ // CraftBukkit end + } + } + } diff --git a/nms-patches/PlayerChunk.patch b/nms-patches/PlayerChunk.patch index ed35ece4ac..c66affdf41 100644 --- a/nms-patches/PlayerChunk.patch +++ b/nms-patches/PlayerChunk.patch @@ -46,14 +46,14 @@ if (either == null || either.left().isPresent()) { return completablefuture; -@@ -256,6 +265,21 @@ +@@ -256,6 +265,24 @@ 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).thenAccept((either) -> { ++ 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 @@ -62,13 +62,16 @@ + chunk.setNeedsSaving(true); + chunk.unloadCallback(); + }); -+ }); ++ }, playerchunkmap.callbackExecutor); ++ ++ // Run callback right away if the future was already done ++ playerchunkmap.callbackExecutor.run(); + } + // CraftBukkit end CompletableFuture completablefuture; if (flag) { -@@ -287,7 +311,7 @@ +@@ -287,7 +314,7 @@ if (flag2 && !flag3) { completablefuture = this.fullChunkFuture; this.fullChunkFuture = PlayerChunk.UNLOADED_CHUNK_FUTURE; @@ -77,19 +80,22 @@ playerchunkmap.getClass(); return either1.ifLeft(playerchunkmap::a); })); -@@ -325,6 +349,17 @@ +@@ -325,6 +352,20 @@ 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).thenAccept((either) -> { ++ this.getStatusFutureUnchecked(ChunkStatus.FULL).thenAcceptAsync((either) -> { + either.ifLeft((chunkAccess) -> { + Chunk chunk = (Chunk) chunkAccess; + chunk.loadCallback(); + }); -+ }); ++ }, playerchunkmap.callbackExecutor); ++ ++ // Run callback right away if the future was already done ++ playerchunkmap.callbackExecutor.run(); + } + // CraftBukkit end } diff --git a/nms-patches/PlayerChunkMap.patch b/nms-patches/PlayerChunkMap.patch index 78505d204d..0a70d13915 100644 --- a/nms-patches/PlayerChunkMap.patch +++ b/nms-patches/PlayerChunkMap.patch @@ -17,7 +17,39 @@ private final AtomicInteger v; private final DefinedStructureManager definedStructureManager; private final File x; -@@ -184,9 +185,12 @@ +@@ -67,6 +68,31 @@ + private final Queue A; + private int viewDistance; + ++ // CraftBukkit start - recursion-safe executor for Chunk loadCallback() and unloadCallback() ++ public final CallbackExecutor callbackExecutor = new CallbackExecutor(); ++ public static final class CallbackExecutor implements java.util.concurrent.Executor, Runnable { ++ ++ private Runnable queued; ++ ++ @Override ++ public void execute(Runnable runnable) { ++ if (queued != null) { ++ throw new IllegalStateException("Already queued"); ++ } ++ queued = runnable; ++ } ++ ++ @Override ++ public void run() { ++ Runnable task = queued; ++ queued = null; ++ if (task != null) { ++ task.run(); ++ } ++ } ++ }; ++ // CraftBukkit end ++ + public PlayerChunkMap(WorldServer worldserver, File file, DataFixer datafixer, DefinedStructureManager definedstructuremanager, Executor executor, IAsyncTaskHandler iasynctaskhandler, ILightAccess ilightaccess, ChunkGenerator chunkgenerator, WorldLoadListener worldloadlistener, Supplier supplier, int i) { + super(new File(worldserver.getWorldProvider().getDimensionManager().a(file), "region"), datafixer); + this.visibleChunks = this.updatingChunks.clone(); +@@ -184,9 +210,12 @@ return completablefuture1.thenApply((list1) -> { List list2 = Lists.newArrayList(); @@ -32,7 +64,7 @@ final Either either = (Either) iterator.next(); Optional optional = either.left(); -@@ -284,7 +288,7 @@ +@@ -284,7 +313,7 @@ PlayerChunkMap.LOGGER.info("ThreadedAnvilChunkStorage ({}): All chunks are saved", this.x.getName()); } else { this.visibleChunks.values().stream().filter(PlayerChunk::hasBeenLoaded).forEach((playerchunk) -> { @@ -41,7 +73,7 @@ if (ichunkaccess instanceof ProtoChunkExtension || ichunkaccess instanceof Chunk) { this.saveChunk(ichunkaccess); -@@ -295,7 +299,6 @@ +@@ -295,7 +324,6 @@ } } @@ -49,7 +81,7 @@ protected void unloadChunks(BooleanSupplier booleansupplier) { GameProfilerFiller gameprofilerfiller = this.world.getMethodProfiler(); -@@ -334,7 +337,7 @@ +@@ -334,7 +362,7 @@ private void a(long i, PlayerChunk playerchunk) { CompletableFuture completablefuture = playerchunk.getChunkSave(); @@ -58,7 +90,7 @@ CompletableFuture completablefuture1 = playerchunk.getChunkSave(); if (completablefuture1 != completablefuture) { -@@ -483,7 +486,7 @@ +@@ -483,7 +511,7 @@ return CompletableFuture.completedFuture(Either.right(playerchunk_failure)); }); }, (runnable) -> { @@ -67,7 +99,7 @@ }); } -@@ -564,7 +567,7 @@ +@@ -564,7 +592,7 @@ long i = playerchunk.i().pair(); playerchunk.getClass(); @@ -76,7 +108,7 @@ }); } -@@ -581,7 +584,7 @@ +@@ -581,7 +609,7 @@ return Either.left(chunk); }); }, (runnable) -> { @@ -85,7 +117,7 @@ }); completablefuture1.thenAcceptAsync((either) -> { -@@ -595,7 +598,7 @@ +@@ -595,7 +623,7 @@ return Either.left(chunk); }); }, (runnable) -> { @@ -94,7 +126,7 @@ }); return completablefuture1; } -@@ -609,7 +612,7 @@ +@@ -609,7 +637,7 @@ return chunk; }); }, (runnable) -> { @@ -103,7 +135,7 @@ }); } -@@ -724,7 +727,7 @@ +@@ -724,7 +752,7 @@ private NBTTagCompound readChunkData(ChunkCoordIntPair chunkcoordintpair) throws IOException { NBTTagCompound nbttagcompound = this.read(chunkcoordintpair); @@ -112,7 +144,7 @@ } boolean isOutsideOfRange(ChunkCoordIntPair chunkcoordintpair) { -@@ -1056,7 +1059,7 @@ +@@ -1056,7 +1084,7 @@ public final Set trackedPlayers = Sets.newHashSet(); public EntityTracker(Entity entity, int i, int j, boolean flag) { @@ -121,7 +153,7 @@ this.tracker = entity; this.trackingDistance = i; this.e = SectionPosition.a(entity); -@@ -1109,7 +1112,7 @@ +@@ -1109,7 +1137,7 @@ public void updatePlayer(EntityPlayer entityplayer) { if (entityplayer != this.tracker) { @@ -130,7 +162,7 @@ int i = Math.min(this.trackingDistance, (PlayerChunkMap.this.viewDistance - 1) * 16); boolean flag = vec3d.x >= (double) (-i) && vec3d.x <= (double) i && vec3d.z >= (double) (-i) && vec3d.z <= (double) i && this.tracker.a(entityplayer); -@@ -1125,6 +1128,17 @@ +@@ -1125,6 +1153,17 @@ } }