From 34039e37099e9eefae8d617fa905656125cec2e5 Mon Sep 17 00:00:00 2001 From: Spottedleaf Date: Sun, 25 Jun 2023 14:01:20 -0700 Subject: [PATCH] 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. --- .../0020-fixup-Rewrite-chunk-system.patch | 50 +++++ ...-entities-in-chunks-that-are-positio.patch | 50 +++++ .../0022-fixup-Rewrite-chunk-system.patch | 174 ++++++++++++++++++ ...ition-to-player-position-on-player-d.patch | 26 +++ 4 files changed, 300 insertions(+) create mode 100644 patches/server/0020-fixup-Rewrite-chunk-system.patch create mode 100644 patches/server/0021-Do-not-read-tile-entities-in-chunks-that-are-positio.patch create mode 100644 patches/server/0022-fixup-Rewrite-chunk-system.patch create mode 100644 patches/server/0023-Sync-vehicle-position-to-player-position-on-player-d.patch diff --git a/patches/server/0020-fixup-Rewrite-chunk-system.patch b/patches/server/0020-fixup-Rewrite-chunk-system.patch new file mode 100644 index 0000000..ad68818 --- /dev/null +++ b/patches/server/0020-fixup-Rewrite-chunk-system.patch @@ -0,0 +1,50 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Spottedleaf +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 diff --git a/patches/server/0021-Do-not-read-tile-entities-in-chunks-that-are-positio.patch b/patches/server/0021-Do-not-read-tile-entities-in-chunks-that-are-positio.patch new file mode 100644 index 0000000..9ae89de --- /dev/null +++ b/patches/server/0021-Do-not-read-tile-entities-in-chunks-that-are-positio.patch @@ -0,0 +1,50 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Spottedleaf +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) { diff --git a/patches/server/0022-fixup-Rewrite-chunk-system.patch b/patches/server/0022-fixup-Rewrite-chunk-system.patch new file mode 100644 index 0000000..0482390 --- /dev/null +++ b/patches/server/0022-fixup-Rewrite-chunk-system.patch @@ -0,0 +1,174 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Spottedleaf +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 { + this.getChunk(x, z).updateStatus(newStatus, this); + } + +- public void addLegacyChunkEntities(final List entities) { +- for (int i = 0, len = entities.size(); i < len; ++i) { +- this.addEntity(entities.get(i), true); +- } ++ public void addLegacyChunkEntities(final List entities, final ChunkPos forChunk) { ++ this.addEntityChunk(entities, forChunk, true); + } + +- public void addEntityChunkEntities(final List entities) { +- for (int i = 0, len = entities.size(); i < len; ++i) { +- this.addEntity(entities.get(i), true); ++ public void addEntityChunkEntities(final List entities, final ChunkPos forChunk) { ++ this.addEntityChunk(entities, forChunk, true); ++ } ++ ++ public void addWorldGenChunkEntities(final List 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 entities) { ++ private void addEntityChunk(final List 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 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 nbt) { // Paper - public ++ public static void postLoadProtoChunk(ServerLevel world, List 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 entities) { +- this.entityLookup.addLegacyChunkEntities(entities.toList()); // Paper - rewrite chunk system ++ public void addLegacyChunkEntities(Stream entities, ChunkPos forChunk) { // Paper - rewrite chunk system ++ this.entityLookup.addLegacyChunkEntities(entities.toList(), forChunk); // Paper - rewrite chunk system + } + +- public void addWorldGenChunkEntities(Stream entities) { +- this.entityLookup.addWorldGenChunkEntities(entities.toList()); // Paper - rewrite chunk system ++ public void addWorldGenChunkEntities(Stream 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) { diff --git a/patches/server/0023-Sync-vehicle-position-to-player-position-on-player-d.patch b/patches/server/0023-Sync-vehicle-position-to-player-position-on-player-d.patch new file mode 100644 index 0000000..f99c337 --- /dev/null +++ b/patches/server/0023-Sync-vehicle-position-to-player-position-on-player-d.patch @@ -0,0 +1,26 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Spottedleaf +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 + });