Fix some issues from Folia test

Player spawn position changing caused any player to log in while
on a boat to trip a thread check. To resolve this, simply reposition
any mounted entities if they are more than 5 blocks away from the player.

In general, this may happen to other entities that are loaded from
chunks as well. In these cases, we can delete the entity if it itself
is not saved in the correct chunk, and we can reposition the mounted
entities if they are not in the correct chunk. For tile entities,
we can simply remove them if they are not in the chunk.

These changes broadly should make loading player/chunk data more
resiliant to bad logic run by plugins.

Also, the Folia test revealed that the chunk generate rate limiter
also affected chunk loading, which was not intended and has been
resolved by exempting already FULL status chunks from the limit.
This commit is contained in:
Spottedleaf 2023-06-25 14:01:20 -07:00
parent 0558e7d923
commit 34039e3709
4 changed files with 300 additions and 0 deletions

View File

@ -0,0 +1,50 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Spottedleaf <Spottedleaf@users.noreply.github.com>
Date: Sun, 18 Jun 2023 22:56:19 -0700
Subject: [PATCH] fixup! Rewrite chunk system
diff --git a/src/main/java/io/papermc/paper/chunk/system/RegionizedPlayerChunkLoader.java b/src/main/java/io/papermc/paper/chunk/system/RegionizedPlayerChunkLoader.java
index 7b664f32868028758d0c6ee1eaa82efcd83661d2..c5df121d6194a97b20dc390698991b9c72dba538 100644
--- a/src/main/java/io/papermc/paper/chunk/system/RegionizedPlayerChunkLoader.java
+++ b/src/main/java/io/papermc/paper/chunk/system/RegionizedPlayerChunkLoader.java
@@ -788,20 +788,34 @@ public class RegionizedPlayerChunkLoader {
// try to push more chunk generations
final long maxGens = Math.max(0L, Math.min(MAX_RATE, Math.min(this.genQueue.size(), this.getMaxChunkGenerates())));
final int maxGensThisTick = (int)this.chunkGenerateTicketLimiter.takeAllocation(time, genRate, maxGens);
- for (int i = 0; i < maxGensThisTick; ++i) {
- final long chunk = this.genQueue.dequeueLong();
- final byte prev = this.chunkTicketStage.put(chunk, CHUNK_TICKET_STAGE_GENERATING);
+ int ratedGensThisTick = 0;
+ while (!this.genQueue.isEmpty()) {
+ final long chunkKey = this.genQueue.firstLong();
+ final int chunkX = CoordinateUtils.getChunkX(chunkKey);
+ final int chunkZ = CoordinateUtils.getChunkZ(chunkKey);
+ final ChunkAccess chunk = this.world.chunkSource.getChunkAtImmediately(chunkX, chunkZ);
+ if (chunk.getStatus() != ChunkStatus.FULL) {
+ // only rate limit actual generations
+ if ((ratedGensThisTick + 1) > maxGensThisTick) {
+ break;
+ }
+ ++ratedGensThisTick;
+ }
+
+ this.genQueue.dequeueLong();
+
+ final byte prev = this.chunkTicketStage.put(chunkKey, CHUNK_TICKET_STAGE_GENERATING);
if (prev != CHUNK_TICKET_STAGE_LOADED) {
throw new IllegalStateException("Previous state should be " + CHUNK_TICKET_STAGE_LOADED + ", not " + prev);
}
this.pushDelayedTicketOp(
ChunkHolderManager.TicketOperation.addAndRemove(
- chunk,
+ chunkKey,
REGION_PLAYER_TICKET, GENERATED_TICKET_LEVEL, this.idBoxed,
REGION_PLAYER_TICKET, LOADED_TICKET_LEVEL, this.idBoxed
)
);
- this.generatingQueue.enqueue(chunk);
+ this.generatingQueue.enqueue(chunkKey);
}
// try to pull ticking chunks

View File

@ -0,0 +1,50 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Spottedleaf <Spottedleaf@users.noreply.github.com>
Date: Sun, 18 Jun 2023 23:04:46 -0700
Subject: [PATCH] Do not read tile entities in chunks that are positioned
outside of the chunk
The tile entities are not accessible and so should not be loaded.
This can happen as a result of users moving regionfiles around,
which would cause a crash on Folia but would appear to function
fine on Paper.
diff --git a/src/main/java/net/minecraft/world/level/chunk/storage/ChunkSerializer.java b/src/main/java/net/minecraft/world/level/chunk/storage/ChunkSerializer.java
index bc938c2a4cb30f3151b600ab88ca5c4e9734f326..035b06d00aadc67eb247efd7b25aea92ba0532a2 100644
--- a/src/main/java/net/minecraft/world/level/chunk/storage/ChunkSerializer.java
+++ b/src/main/java/net/minecraft/world/level/chunk/storage/ChunkSerializer.java
@@ -381,6 +381,13 @@ public class ChunkSerializer {
for (int k1 = 0; k1 < nbttaglist3.size(); ++k1) {
CompoundTag nbttagcompound4 = nbttaglist3.getCompound(k1);
+ // Paper start - do not read tile entities positioned outside the chunk
+ BlockPos blockposition = BlockEntity.getPosFromTag(nbttagcompound4);
+ if ((blockposition.getX() >> 4) != chunkPos.x || (blockposition.getZ() >> 4) != chunkPos.z) {
+ LOGGER.warn("Tile entity serialized in chunk " + chunkPos + " in world '" + world.getWorld().getName() + "' positioned at " + blockposition + " is located outside of the chunk");
+ continue;
+ }
+ // Paper end - do not read tile entities positioned outside the chunk
((ChunkAccess) object1).setBlockEntityNbt(nbttagcompound4);
}
@@ -679,10 +686,19 @@ public class ChunkSerializer {
CompoundTag nbttagcompound1 = nbttaglist1.getCompound(i);
boolean flag = nbttagcompound1.getBoolean("keepPacked");
+ // Paper start - do not read tile entities positioned outside the chunk
+ BlockPos blockposition = BlockEntity.getPosFromTag(nbttagcompound1); // moved up
+ ChunkPos chunkPos = chunk.getPos();
+ if ((blockposition.getX() >> 4) != chunkPos.x || (blockposition.getZ() >> 4) != chunkPos.z) {
+ LOGGER.warn("Tile entity serialized in chunk " + chunkPos + " in world '" + world.getWorld().getName() + "' positioned at " + blockposition + " is located outside of the chunk");
+ continue;
+ }
+ // Paper end - do not read tile entities positioned outside the chunk
+
if (flag) {
chunk.setBlockEntityNbt(nbttagcompound1);
} else {
- BlockPos blockposition = BlockEntity.getPosFromTag(nbttagcompound1);
+ // Paper - do not read tile entities positioned outside the chunk - move up
BlockEntity tileentity = BlockEntity.loadStatic(blockposition, chunk.getBlockState(blockposition), nbttagcompound1);
if (tileentity != null) {

View File

@ -0,0 +1,174 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Spottedleaf <Spottedleaf@users.noreply.github.com>
Date: Sun, 18 Jun 2023 23:25:29 -0700
Subject: [PATCH] fixup! Rewrite chunk system
diff --git a/src/main/java/io/papermc/paper/chunk/system/entity/EntityLookup.java b/src/main/java/io/papermc/paper/chunk/system/entity/EntityLookup.java
index b9095f559472dd92375ea719886913f606f0374c..fdbd2e442d983ab17ba0d805a12c59acbe8dd2ec 100644
--- a/src/main/java/io/papermc/paper/chunk/system/entity/EntityLookup.java
+++ b/src/main/java/io/papermc/paper/chunk/system/entity/EntityLookup.java
@@ -17,6 +17,7 @@ import net.minecraft.util.AbortableIterationConsumer;
import net.minecraft.util.Mth;
import net.minecraft.world.entity.Entity;
import net.minecraft.world.entity.EntityType;
+import net.minecraft.world.level.ChunkPos;
import net.minecraft.world.level.entity.EntityInLevelCallback;
import net.minecraft.world.level.entity.EntityTypeTest;
import net.minecraft.world.level.entity.LevelCallback;
@@ -24,6 +25,7 @@ import net.minecraft.server.level.FullChunkStatus;
import net.minecraft.world.level.entity.LevelEntityGetter;
import net.minecraft.world.level.entity.Visibility;
import net.minecraft.world.phys.AABB;
+import net.minecraft.world.phys.Vec3;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.slf4j.Logger;
@@ -314,22 +316,55 @@ public final class EntityLookup implements LevelEntityGetter<Entity> {
this.getChunk(x, z).updateStatus(newStatus, this);
}
- public void addLegacyChunkEntities(final List<Entity> entities) {
- for (int i = 0, len = entities.size(); i < len; ++i) {
- this.addEntity(entities.get(i), true);
- }
+ public void addLegacyChunkEntities(final List<Entity> entities, final ChunkPos forChunk) {
+ this.addEntityChunk(entities, forChunk, true);
}
- public void addEntityChunkEntities(final List<Entity> entities) {
- for (int i = 0, len = entities.size(); i < len; ++i) {
- this.addEntity(entities.get(i), true);
+ public void addEntityChunkEntities(final List<Entity> entities, final ChunkPos forChunk) {
+ this.addEntityChunk(entities, forChunk, true);
+ }
+
+ public void addWorldGenChunkEntities(final List<Entity> entities, final ChunkPos forChunk) {
+ this.addEntityChunk(entities, forChunk, false);
+ }
+
+ private void addRecursivelySafe(final Entity root, final boolean fromDisk) {
+ if (!this.addEntity(root, fromDisk)) {
+ // possible we are a passenger, and so should dismount from any valid entity in the world
+ root.stopRiding(true);
+ return;
+ }
+ for (final Entity passenger : root.getPassengers()) {
+ this.addRecursivelySafe(passenger, fromDisk);
}
}
- public void addWorldGenChunkEntities(final List<Entity> entities) {
+ private void addEntityChunk(final List<Entity> entities, final ChunkPos forChunk, final boolean fromDisk) {
for (int i = 0, len = entities.size(); i < len; ++i) {
- this.addEntity(entities.get(i), false);
- }
+ final Entity entity = entities.get(i);
+ if (entity.isPassenger()) {
+ continue;
+ }
+
+ if (!entity.chunkPosition().equals(forChunk)) {
+ LOGGER.warn("Root entity " + entity + " is outside of serialized chunk " + forChunk);
+ // can't set removed here, as we may not own the chunk position
+ // skip the entity
+ continue;
+ }
+
+ final Vec3 rootPosition = entity.position();
+
+ // always adjust positions before adding passengers in case plugins access the entity, and so that
+ // they are added to the right entity chunk
+ for (final Entity passenger : entity.getIndirectPassengers()) {
+ if (!passenger.chunkPosition().equals(forChunk)) {
+ passenger.setPosRaw(rootPosition.x, rootPosition.y, rootPosition.z, true);
+ }
+ }
+
+ this.addRecursivelySafe(entity, fromDisk);
+ }
}
public boolean addNewEntity(final Entity entity) {
diff --git a/src/main/java/io/papermc/paper/chunk/system/scheduling/ChunkFullTask.java b/src/main/java/io/papermc/paper/chunk/system/scheduling/ChunkFullTask.java
index 0b78d1eb90500e0123b7281d722805dc65d551d0..8a48238fac0949cfddcdd9ccf179d16befe9c4d4 100644
--- a/src/main/java/io/papermc/paper/chunk/system/scheduling/ChunkFullTask.java
+++ b/src/main/java/io/papermc/paper/chunk/system/scheduling/ChunkFullTask.java
@@ -83,7 +83,7 @@ public final class ChunkFullTask extends ChunkProgressionTask implements Runnabl
final ServerLevel world = this.world;
final ProtoChunk protoChunk = (ProtoChunk)this.fromChunk;
chunk = new LevelChunk(this.world, protoChunk, (final LevelChunk unused) -> {
- ChunkMap.postLoadProtoChunk(world, protoChunk.getEntities());
+ ChunkMap.postLoadProtoChunk(world, protoChunk.getEntities(), protoChunk.getPos()); // Paper - rewrite chunk system
});
}
diff --git a/src/main/java/io/papermc/paper/chunk/system/scheduling/NewChunkHolder.java b/src/main/java/io/papermc/paper/chunk/system/scheduling/NewChunkHolder.java
index f1c68d9850ece7532a8607db955eaa4fc3a4bf05..08075b8895f816420c2a940bf551dfada3c0cd9e 100644
--- a/src/main/java/io/papermc/paper/chunk/system/scheduling/NewChunkHolder.java
+++ b/src/main/java/io/papermc/paper/chunk/system/scheduling/NewChunkHolder.java
@@ -115,7 +115,7 @@ public final class NewChunkHolder {
if (entityChunk != null) {
final List<Entity> entities = EntityStorage.readEntities(this.world, entityChunk);
- this.world.getEntityLookup().addEntityChunkEntities(entities);
+ this.world.getEntityLookup().addEntityChunkEntities(entities, new ChunkPos(this.chunkX, this.chunkZ));
}
}
diff --git a/src/main/java/net/minecraft/server/level/ChunkMap.java b/src/main/java/net/minecraft/server/level/ChunkMap.java
index 916bfdfb13d8f8093e1908a7c35344b83d0ee0ac..25fe439c8d1e88a86e85ac9a4761425d98ee6c4f 100644
--- a/src/main/java/net/minecraft/server/level/ChunkMap.java
+++ b/src/main/java/net/minecraft/server/level/ChunkMap.java
@@ -597,7 +597,7 @@ public class ChunkMap extends ChunkStorage implements ChunkHolder.PlayerProvider
return chunkstatus1;
}
- public static void postLoadProtoChunk(ServerLevel world, List<CompoundTag> nbt) { // Paper - public
+ public static void postLoadProtoChunk(ServerLevel world, List<CompoundTag> nbt, ChunkPos position) { // Paper - public and add chunk position parameter
if (!nbt.isEmpty()) {
// CraftBukkit start - these are spawned serialized (DefinedStructure) and we don't call an add event below at the moment due to ordering complexities
world.addWorldGenChunkEntities(EntityType.loadEntitiesRecursive(nbt, world).filter((entity) -> {
@@ -613,7 +613,7 @@ public class ChunkMap extends ChunkStorage implements ChunkHolder.PlayerProvider
}
checkDupeUUID(world, entity); // Paper
return !needsRemoval;
- }));
+ }), position); // Paper - rewrite chunk system
// CraftBukkit end
}
diff --git a/src/main/java/net/minecraft/server/level/ServerLevel.java b/src/main/java/net/minecraft/server/level/ServerLevel.java
index f95f1d1bc08b3c8f331a4f760e3218238f74ff67..042ca6b3faae5249210567f2c26dff404974e1ff 100644
--- a/src/main/java/net/minecraft/server/level/ServerLevel.java
+++ b/src/main/java/net/minecraft/server/level/ServerLevel.java
@@ -2638,12 +2638,12 @@ public class ServerLevel extends Level implements WorldGenLevel {
return this.entityLookup; // Paper - rewrite chunk system
}
- public void addLegacyChunkEntities(Stream<Entity> entities) {
- this.entityLookup.addLegacyChunkEntities(entities.toList()); // Paper - rewrite chunk system
+ public void addLegacyChunkEntities(Stream<Entity> entities, ChunkPos forChunk) { // Paper - rewrite chunk system
+ this.entityLookup.addLegacyChunkEntities(entities.toList(), forChunk); // Paper - rewrite chunk system
}
- public void addWorldGenChunkEntities(Stream<Entity> entities) {
- this.entityLookup.addWorldGenChunkEntities(entities.toList()); // Paper - rewrite chunk system
+ public void addWorldGenChunkEntities(Stream<Entity> entities, ChunkPos forChunk) { // Paper - rewrite chunk system
+ this.entityLookup.addWorldGenChunkEntities(entities.toList(), forChunk); // Paper - rewrite chunk system
}
public void startTickingChunk(LevelChunk chunk) {
diff --git a/src/main/java/net/minecraft/world/level/chunk/storage/ChunkSerializer.java b/src/main/java/net/minecraft/world/level/chunk/storage/ChunkSerializer.java
index 035b06d00aadc67eb247efd7b25aea92ba0532a2..97533c4a31ab6137072b79dc4377fd602fef9ea8 100644
--- a/src/main/java/net/minecraft/world/level/chunk/storage/ChunkSerializer.java
+++ b/src/main/java/net/minecraft/world/level/chunk/storage/ChunkSerializer.java
@@ -678,7 +678,7 @@ public class ChunkSerializer {
return nbttaglist == null && nbttaglist1 == null ? null : (chunk) -> {
if (nbttaglist != null) {
- world.addLegacyChunkEntities(EntityType.loadEntitiesRecursive(nbttaglist, world));
+ world.addLegacyChunkEntities(EntityType.loadEntitiesRecursive(nbttaglist, world), chunk.getPos()); // Paper - rewrite chunk system
}
if (nbttaglist1 != null) {

View File

@ -0,0 +1,26 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Spottedleaf <Spottedleaf@users.noreply.github.com>
Date: Sun, 25 Jun 2023 13:57:30 -0700
Subject: [PATCH] Sync vehicle position to player position on player data load
This allows the player to be re-positioned before logging into
the world without causing thread checks to trip on Folia.
diff --git a/src/main/java/net/minecraft/server/players/PlayerList.java b/src/main/java/net/minecraft/server/players/PlayerList.java
index d4620ddfcd06a037f647ab0a8938797405e001b2..654ab7fc8caa9af7dc665391a197b41599c2bc8f 100644
--- a/src/main/java/net/minecraft/server/players/PlayerList.java
+++ b/src/main/java/net/minecraft/server/players/PlayerList.java
@@ -498,7 +498,13 @@ public abstract class PlayerList {
CompoundTag nbttagcompound1 = nbttagcompound.getCompound("RootVehicle");
// CraftBukkit start
ServerLevel finalWorldServer = worldserver1;
+ Vec3 playerPos = player.position(); // Paper - force sync root vehicle to player position
Entity entity = EntityType.loadEntityRecursive(nbttagcompound1.getCompound("Entity"), finalWorldServer, (entity1) -> {
+ // Paper start - force sync root vehicle to player position
+ if (entity1.distanceToSqr(player) > (5.0 * 5.0)) {
+ entity1.setPosRaw(playerPos.x, playerPos.y, playerPos.z, true);
+ }
+ // Paper end - force sync root vehicle to player position
return !finalWorldServer.addWithUUID(entity1, org.bukkit.event.entity.CreatureSpawnEvent.SpawnReason.MOUNT) ? null : entity1; // Paper
// CraftBukkit end
});