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 <lukas@wirsindwir.de>
This commit is contained in:
CraftBukkit/Spigot 2021-02-14 09:24:23 +11:00
parent 98e0c5c715
commit 373ed1ddd5
3 changed files with 38 additions and 17 deletions

View file

@ -1,6 +1,6 @@
--- a/net/minecraft/server/ChunkProviderServer.java --- a/net/minecraft/server/ChunkProviderServer.java
+++ b/net/minecraft/server/ChunkProviderServer.java +++ b/net/minecraft/server/ChunkProviderServer.java
@@ -54,6 +54,16 @@ @@ -54,6 +54,24 @@
this.clearCache(); this.clearCache();
} }
@ -12,12 +12,20 @@
+ } + }
+ return chunk.getFullChunk() != null; + 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 + // CraftBukkit end
+ +
@Override @Override
public LightEngineThreaded getLightEngine() { public LightEngineThreaded getLightEngine() {
return this.lightEngine; return this.lightEngine;
@@ -98,7 +108,7 @@ @@ -98,7 +116,7 @@
for (int l = 0; l < 4; ++l) { for (int l = 0; l < 4; ++l) {
if (k == this.cachePos[l] && chunkstatus == this.cacheStatus[l]) { if (k == this.cachePos[l] && chunkstatus == this.cacheStatus[l]) {
ichunkaccess = this.cacheChunk[l]; ichunkaccess = this.cacheChunk[l];
@ -26,7 +34,7 @@
return ichunkaccess; return ichunkaccess;
} }
} }
@@ -144,12 +154,12 @@ @@ -144,12 +162,12 @@
if (playerchunk == null) { if (playerchunk == null) {
return null; return null;
} else { } else {
@ -41,7 +49,7 @@
if (ichunkaccess1 != null) { if (ichunkaccess1 != null) {
this.a(k, ichunkaccess1, ChunkStatus.FULL); this.a(k, ichunkaccess1, ChunkStatus.FULL);
@@ -176,7 +186,15 @@ @@ -176,7 +194,15 @@
int l = 33 + ChunkStatus.a(chunkstatus); int l = 33 + ChunkStatus.a(chunkstatus);
PlayerChunk playerchunk = this.getChunk(k); PlayerChunk playerchunk = this.getChunk(k);
@ -58,7 +66,7 @@
this.chunkMapDistance.a(TicketType.UNKNOWN, chunkcoordintpair, l, chunkcoordintpair); this.chunkMapDistance.a(TicketType.UNKNOWN, chunkcoordintpair, l, chunkcoordintpair);
if (this.a(playerchunk, l)) { if (this.a(playerchunk, l)) {
GameProfilerFiller gameprofilerfiller = this.world.getMethodProfiler(); GameProfilerFiller gameprofilerfiller = this.world.getMethodProfiler();
@@ -195,7 +213,7 @@ @@ -195,7 +221,7 @@
} }
private boolean a(@Nullable PlayerChunk playerchunk, int i) { private boolean a(@Nullable PlayerChunk playerchunk, int i) {
@ -67,7 +75,7 @@
} }
public boolean isLoaded(int i, int j) { public boolean isLoaded(int i, int j) {
@@ -257,19 +275,19 @@ @@ -257,19 +283,19 @@
public boolean a(Entity entity) { public boolean a(Entity entity) {
long i = ChunkCoordIntPair.pair(MathHelper.floor(entity.locX()) >> 4, MathHelper.floor(entity.locZ()) >> 4); long i = ChunkCoordIntPair.pair(MathHelper.floor(entity.locX()) >> 4, MathHelper.floor(entity.locZ()) >> 4);
@ -90,7 +98,7 @@
} }
private boolean a(long i, Function<PlayerChunk, CompletableFuture<Either<Chunk, PlayerChunk.Failure>>> function) { private boolean a(long i, Function<PlayerChunk, CompletableFuture<Either<Chunk, PlayerChunk.Failure>>> function) {
@@ -291,11 +309,31 @@ @@ -291,11 +317,31 @@
@Override @Override
public void close() throws IOException { public void close() throws IOException {
@ -123,7 +131,7 @@
public void tick(BooleanSupplier booleansupplier) { public void tick(BooleanSupplier booleansupplier) {
this.world.getMethodProfiler().enter("purge"); this.world.getMethodProfiler().enter("purge");
this.chunkMapDistance.purgeTickets(); this.chunkMapDistance.purgeTickets();
@@ -315,12 +353,12 @@ @@ -315,12 +361,12 @@
this.lastTickTime = i; this.lastTickTime = i;
WorldData worlddata = this.world.getWorldData(); WorldData worlddata = this.world.getWorldData();
boolean flag = this.world.isDebugWorld(); boolean flag = this.world.isDebugWorld();
@ -138,7 +146,7 @@
this.world.getMethodProfiler().enter("naturalSpawnCount"); this.world.getMethodProfiler().enter("naturalSpawnCount");
int l = this.chunkMapDistance.b(); int l = this.chunkMapDistance.b();
@@ -507,12 +545,18 @@ @@ -507,12 +553,18 @@
@Override @Override
protected boolean executeNext() { protected boolean executeNext() {

View file

@ -9,23 +9,27 @@
this.dirtyBlocks = new ShortSet[16]; this.dirtyBlocks = new ShortSet[16];
this.location = chunkcoordintpair; this.location = chunkcoordintpair;
this.lightEngine = lightengine; this.lightEngine = lightengine;
@@ -56,6 +56,15 @@ @@ -56,6 +56,19 @@
this.a(i); this.a(i);
} }
+ // CraftBukkit start + // CraftBukkit start
+ public Chunk getFullChunk() { + public Chunk getFullChunk() {
+ if (!getChunkState(this.oldTicketLevel).isAtLeast(PlayerChunk.State.BORDER)) return null; // note: using oldTicketLevel for isLoaded checks + if (!getChunkState(this.oldTicketLevel).isAtLeast(PlayerChunk.State.BORDER)) return null; // note: using oldTicketLevel for isLoaded checks
+ return this.getFullChunkUnchecked();
+ }
+
+ public Chunk getFullChunkUnchecked() {
+ CompletableFuture<Either<IChunkAccess, PlayerChunk.Failure>> statusFuture = this.getStatusFutureUnchecked(ChunkStatus.FULL); + CompletableFuture<Either<IChunkAccess, PlayerChunk.Failure>> statusFuture = this.getStatusFutureUnchecked(ChunkStatus.FULL);
+ Either<IChunkAccess, PlayerChunk.Failure> either = (Either<IChunkAccess, PlayerChunk.Failure>) statusFuture.getNow(null); + Either<IChunkAccess, PlayerChunk.Failure> either = (Either<IChunkAccess, PlayerChunk.Failure>) statusFuture.getNow(null);
+ return either == null ? null : (Chunk) either.left().orElse(null); + return (either == null) ? null : (Chunk) either.left().orElse(null);
+ } + }
+ // CraftBukkit end + // CraftBukkit end
+ +
public CompletableFuture<Either<IChunkAccess, PlayerChunk.Failure>> getStatusFutureUnchecked(ChunkStatus chunkstatus) { public CompletableFuture<Either<IChunkAccess, PlayerChunk.Failure>> getStatusFutureUnchecked(ChunkStatus chunkstatus) {
CompletableFuture<Either<IChunkAccess, PlayerChunk.Failure>> completablefuture = (CompletableFuture) this.statusFutures.get(chunkstatus.c()); CompletableFuture<Either<IChunkAccess, PlayerChunk.Failure>> completablefuture = (CompletableFuture) this.statusFutures.get(chunkstatus.c());
@@ -81,9 +90,9 @@ @@ -81,9 +94,9 @@
@Nullable @Nullable
public Chunk getChunk() { public Chunk getChunk() {
CompletableFuture<Either<Chunk, PlayerChunk.Failure>> completablefuture = this.a(); CompletableFuture<Either<Chunk, PlayerChunk.Failure>> completablefuture = this.a();
@ -37,7 +41,7 @@
} }
@Nullable @Nullable
@@ -114,6 +123,7 @@ @@ -114,6 +127,7 @@
if (chunk != null) { if (chunk != null) {
byte b0 = (byte) SectionPosition.a(blockposition.getY()); byte b0 = (byte) SectionPosition.a(blockposition.getY());
@ -45,7 +49,7 @@
if (this.dirtyBlocks[b0] == null) { if (this.dirtyBlocks[b0] == null) {
this.p = true; this.p = true;
this.dirtyBlocks[b0] = new ShortArraySet(); this.dirtyBlocks[b0] = new ShortArraySet();
@@ -216,7 +226,7 @@ @@ -216,7 +230,7 @@
CompletableFuture<Either<IChunkAccess, PlayerChunk.Failure>> completablefuture = (CompletableFuture) this.statusFutures.get(i); CompletableFuture<Either<IChunkAccess, PlayerChunk.Failure>> completablefuture = (CompletableFuture) this.statusFutures.get(i);
if (completablefuture != null) { if (completablefuture != null) {
@ -54,7 +58,7 @@
if (either == null || either.left().isPresent()) { if (either == null || either.left().isPresent()) {
return completablefuture; return completablefuture;
@@ -271,6 +281,30 @@ @@ -271,6 +285,30 @@
boolean flag1 = this.ticketLevel <= PlayerChunkMap.GOLDEN_TICKET; boolean flag1 = this.ticketLevel <= PlayerChunkMap.GOLDEN_TICKET;
PlayerChunk.State playerchunk_state = getChunkState(this.oldTicketLevel); PlayerChunk.State playerchunk_state = getChunkState(this.oldTicketLevel);
PlayerChunk.State playerchunk_state1 = getChunkState(this.ticketLevel); PlayerChunk.State playerchunk_state1 = getChunkState(this.ticketLevel);
@ -85,7 +89,7 @@
CompletableFuture completablefuture; CompletableFuture completablefuture;
if (flag) { if (flag) {
@@ -302,7 +336,7 @@ @@ -302,7 +340,7 @@
if (flag2 && !flag3) { if (flag2 && !flag3) {
completablefuture = this.fullChunkFuture; completablefuture = this.fullChunkFuture;
this.fullChunkFuture = PlayerChunk.UNLOADED_CHUNK_FUTURE; this.fullChunkFuture = PlayerChunk.UNLOADED_CHUNK_FUTURE;
@ -94,7 +98,7 @@
playerchunkmap.getClass(); playerchunkmap.getClass();
return either1.ifLeft(playerchunkmap::a); 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.u.a(this.location, this::k, this.ticketLevel, this::d);
this.oldTicketLevel = this.ticketLevel; this.oldTicketLevel = this.ticketLevel;

View file

@ -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 @@ @@ -923,10 +1084,33 @@
this.everyoneSleeping(); this.everyoneSleeping();
} }