From 373ed1ddd5749ac413566a57ee0418d083133f20 Mon Sep 17 00:00:00 2001 From: CraftBukkit/Spigot Date: Sun, 14 Feb 2021 09:24:23 +1100 Subject: [PATCH] SPIGOT-5228: Entities that are removed during chunk unloads are not properly removed from the chunk. This could lead to dead entities accumulating in memory over time if the chunk never gets fully unloaded (as it is the case for chunks around the spawn region). The issue is that Minecraft processes the removal of these entities during the next tick, when the chunk has already switched to state INACCESSIBLE and can no longer be retrieved as usual. For the purpose of removing dead entities from their still loaded but no longer accessible chunk, this adds and uses a new method with which a chunk can be accessed without checking its current state first. By: blablubbabc --- .../nms-patches/ChunkProviderServer.patch | 26 ++++++++++++------- paper-server/nms-patches/PlayerChunk.patch | 20 ++++++++------ paper-server/nms-patches/WorldServer.patch | 9 +++++++ 3 files changed, 38 insertions(+), 17 deletions(-) diff --git a/paper-server/nms-patches/ChunkProviderServer.patch b/paper-server/nms-patches/ChunkProviderServer.patch index d945a3d4d7..b7f02ad873 100644 --- a/paper-server/nms-patches/ChunkProviderServer.patch +++ b/paper-server/nms-patches/ChunkProviderServer.patch @@ -1,6 +1,6 @@ --- a/net/minecraft/server/ChunkProviderServer.java +++ b/net/minecraft/server/ChunkProviderServer.java -@@ -54,6 +54,16 @@ +@@ -54,6 +54,24 @@ this.clearCache(); } @@ -12,12 +12,20 @@ + } + return chunk.getFullChunk() != null; + } ++ ++ public Chunk getChunkUnchecked(int chunkX, int chunkZ) { ++ PlayerChunk chunk = this.playerChunkMap.getUpdatingChunk(ChunkCoordIntPair.pair(chunkX, chunkZ)); ++ if (chunk == null) { ++ return null; ++ } ++ return chunk.getFullChunkUnchecked(); ++ } + // CraftBukkit end + @Override public LightEngineThreaded getLightEngine() { return this.lightEngine; -@@ -98,7 +108,7 @@ +@@ -98,7 +116,7 @@ for (int l = 0; l < 4; ++l) { if (k == this.cachePos[l] && chunkstatus == this.cacheStatus[l]) { ichunkaccess = this.cacheChunk[l]; @@ -26,7 +34,7 @@ return ichunkaccess; } } -@@ -144,12 +154,12 @@ +@@ -144,12 +162,12 @@ if (playerchunk == null) { return null; } else { @@ -41,7 +49,7 @@ if (ichunkaccess1 != null) { this.a(k, ichunkaccess1, ChunkStatus.FULL); -@@ -176,7 +186,15 @@ +@@ -176,7 +194,15 @@ int l = 33 + ChunkStatus.a(chunkstatus); PlayerChunk playerchunk = this.getChunk(k); @@ -58,7 +66,7 @@ this.chunkMapDistance.a(TicketType.UNKNOWN, chunkcoordintpair, l, chunkcoordintpair); if (this.a(playerchunk, l)) { GameProfilerFiller gameprofilerfiller = this.world.getMethodProfiler(); -@@ -195,7 +213,7 @@ +@@ -195,7 +221,7 @@ } private boolean a(@Nullable PlayerChunk playerchunk, int i) { @@ -67,7 +75,7 @@ } public boolean isLoaded(int i, int j) { -@@ -257,19 +275,19 @@ +@@ -257,19 +283,19 @@ public boolean a(Entity entity) { long i = ChunkCoordIntPair.pair(MathHelper.floor(entity.locX()) >> 4, MathHelper.floor(entity.locZ()) >> 4); @@ -90,7 +98,7 @@ } private boolean a(long i, Function>> function) { -@@ -291,11 +309,31 @@ +@@ -291,11 +317,31 @@ @Override public void close() throws IOException { @@ -123,7 +131,7 @@ public void tick(BooleanSupplier booleansupplier) { this.world.getMethodProfiler().enter("purge"); this.chunkMapDistance.purgeTickets(); -@@ -315,12 +353,12 @@ +@@ -315,12 +361,12 @@ this.lastTickTime = i; WorldData worlddata = this.world.getWorldData(); boolean flag = this.world.isDebugWorld(); @@ -138,7 +146,7 @@ this.world.getMethodProfiler().enter("naturalSpawnCount"); int l = this.chunkMapDistance.b(); -@@ -507,12 +545,18 @@ +@@ -507,12 +553,18 @@ @Override protected boolean executeNext() { diff --git a/paper-server/nms-patches/PlayerChunk.patch b/paper-server/nms-patches/PlayerChunk.patch index d35f4afb43..004473544d 100644 --- a/paper-server/nms-patches/PlayerChunk.patch +++ b/paper-server/nms-patches/PlayerChunk.patch @@ -9,23 +9,27 @@ this.dirtyBlocks = new ShortSet[16]; this.location = chunkcoordintpair; this.lightEngine = lightengine; -@@ -56,6 +56,15 @@ +@@ -56,6 +56,19 @@ this.a(i); } + // CraftBukkit start + public Chunk getFullChunk() { + if (!getChunkState(this.oldTicketLevel).isAtLeast(PlayerChunk.State.BORDER)) return null; // note: using oldTicketLevel for isLoaded checks ++ return this.getFullChunkUnchecked(); ++ } ++ ++ public Chunk getFullChunkUnchecked() { + CompletableFuture> statusFuture = this.getStatusFutureUnchecked(ChunkStatus.FULL); + Either either = (Either) statusFuture.getNow(null); -+ return either == null ? null : (Chunk) either.left().orElse(null); ++ return (either == null) ? null : (Chunk) either.left().orElse(null); + } + // CraftBukkit end + public CompletableFuture> getStatusFutureUnchecked(ChunkStatus chunkstatus) { CompletableFuture> completablefuture = (CompletableFuture) this.statusFutures.get(chunkstatus.c()); -@@ -81,9 +90,9 @@ +@@ -81,9 +94,9 @@ @Nullable public Chunk getChunk() { CompletableFuture> completablefuture = this.a(); @@ -37,7 +41,7 @@ } @Nullable -@@ -114,6 +123,7 @@ +@@ -114,6 +127,7 @@ if (chunk != null) { byte b0 = (byte) SectionPosition.a(blockposition.getY()); @@ -45,7 +49,7 @@ if (this.dirtyBlocks[b0] == null) { this.p = true; this.dirtyBlocks[b0] = new ShortArraySet(); -@@ -216,7 +226,7 @@ +@@ -216,7 +230,7 @@ CompletableFuture> completablefuture = (CompletableFuture) this.statusFutures.get(i); if (completablefuture != null) { @@ -54,7 +58,7 @@ if (either == null || either.left().isPresent()) { return completablefuture; -@@ -271,6 +281,30 @@ +@@ -271,6 +285,30 @@ boolean flag1 = this.ticketLevel <= PlayerChunkMap.GOLDEN_TICKET; PlayerChunk.State playerchunk_state = getChunkState(this.oldTicketLevel); PlayerChunk.State playerchunk_state1 = getChunkState(this.ticketLevel); @@ -85,7 +89,7 @@ CompletableFuture completablefuture; if (flag) { -@@ -302,7 +336,7 @@ +@@ -302,7 +340,7 @@ if (flag2 && !flag3) { completablefuture = this.fullChunkFuture; this.fullChunkFuture = PlayerChunk.UNLOADED_CHUNK_FUTURE; @@ -94,7 +98,7 @@ playerchunkmap.getClass(); return either1.ifLeft(playerchunkmap::a); })); -@@ -340,6 +374,26 @@ +@@ -340,6 +378,26 @@ this.u.a(this.location, this::k, this.ticketLevel, this::d); this.oldTicketLevel = this.ticketLevel; diff --git a/paper-server/nms-patches/WorldServer.patch b/paper-server/nms-patches/WorldServer.patch index 64a8877e65..33f4a40308 100644 --- a/paper-server/nms-patches/WorldServer.patch +++ b/paper-server/nms-patches/WorldServer.patch @@ -448,6 +448,15 @@ } } +@@ -909,7 +1070,7 @@ + } + + private void removeEntityFromChunk(Entity entity) { +- IChunkAccess ichunkaccess = this.getChunkAt(entity.chunkX, entity.chunkZ, ChunkStatus.FULL, false); ++ IChunkAccess ichunkaccess = chunkProvider.getChunkUnchecked(entity.chunkX, entity.chunkZ); // CraftBukkit - SPIGOT-5228: getChunkAt won't find the entity's chunk if it has already been unloaded (i.e. if it switched to state INACCESSIBLE). + + if (ichunkaccess instanceof Chunk) { + ((Chunk) ichunkaccess).b(entity); @@ -923,10 +1084,33 @@ this.everyoneSleeping(); }