From b54ba0d75b2dc8ea2b627e875dd88cc9405c81fa Mon Sep 17 00:00:00 2001 From: Spottedleaf Date: Mon, 20 Jan 2025 04:56:56 -0800 Subject: [PATCH] Remove nearbyPlayers cache on ChunkData We cannot guarantee that the current region owns the chunk associated with the ChunkData. As a result, more than 1 region may write to the field. Additionally, we did not include any logic to adjust the field during a region merge or split - which would leave invalid data in the field. As a result, the nearbyPlayers data retrieved from the ChunkData was possibly invalid which may have lead to entity tracker desync problems. Fixes https://github.com/PaperMC/Folia/issues/317 --- .../features/0006-Region-profiler.patch | 27 ++++++++++--------- .../common/misc/NearbyPlayers.java.patch | 23 ++++++++++++++++ .../level/chunk/ChunkData.java.patch | 11 ++++++++ .../server/level/ChunkMap.java.patch | 12 ++++++++- .../server/level/ServerChunkCache.java.patch | 11 ++++++++ 5 files changed, 70 insertions(+), 14 deletions(-) create mode 100644 folia-server/minecraft-patches/sources/ca/spottedleaf/moonrise/common/misc/NearbyPlayers.java.patch create mode 100644 folia-server/minecraft-patches/sources/ca/spottedleaf/moonrise/patches/chunk_system/level/chunk/ChunkData.java.patch diff --git a/folia-server/minecraft-patches/features/0006-Region-profiler.patch b/folia-server/minecraft-patches/features/0006-Region-profiler.patch index 3a366fe..a2283e8 100644 --- a/folia-server/minecraft-patches/features/0006-Region-profiler.patch +++ b/folia-server/minecraft-patches/features/0006-Region-profiler.patch @@ -1050,7 +1050,7 @@ index 601ed36413bbbf9c17e530b42906986e441237fd..6ae7d646c1173c4835fa235c7cee21ec private boolean saveChunk(final ChunkAccess chunk, final boolean unloading) { diff --git a/io/papermc/paper/threadedregions/TickRegionScheduler.java b/io/papermc/paper/threadedregions/TickRegionScheduler.java -index 4471285a4358e51da9912ed791a824527f1a2e8e..a18da3f3f245031f0547efe9b52a1f2a219ef04a 100644 +index 18f57216522a8e22ea6c217c05588f8be13f5c88..2fd27993b445cd8c3b517a91746c3f303a35238d 100644 --- a/io/papermc/paper/threadedregions/TickRegionScheduler.java +++ b/io/papermc/paper/threadedregions/TickRegionScheduler.java @@ -67,8 +67,13 @@ public final class TickRegionScheduler { @@ -1544,7 +1544,7 @@ index 9c9de462eb7187d6cc3562c796e3bcf69fb20783..faf72dd6dff74296c73cb058aaabd1f9 //this.playerList.tick(); // Folia - region threading if (SharedConstants.IS_RUNNING_IN_IDE && this.tickRateManager.runsNormally()) { diff --git a/net/minecraft/server/level/ChunkMap.java b/net/minecraft/server/level/ChunkMap.java -index 06cc9d69220b09667db30a5c1e161333d2563b23..65b9a49e84d9bde75c249651e974d57bc1a43277 100644 +index 17d124acb7149f6c19f9295ad173108d5070e55b..4bdb05948e9102304364f3681ce353f1cf2a0aee 100644 --- a/net/minecraft/server/level/ChunkMap.java +++ b/net/minecraft/server/level/ChunkMap.java @@ -410,12 +410,17 @@ public class ChunkMap extends ChunkStorage implements ChunkHolder.PlayerProvider @@ -1565,7 +1565,7 @@ index 06cc9d69220b09667db30a5c1e161333d2563b23..65b9a49e84d9bde75c249651e974d57b } profilerFiller.pop(); -@@ -937,12 +942,17 @@ public class ChunkMap extends ChunkStorage implements ChunkHolder.PlayerProvider +@@ -937,13 +942,18 @@ public class ChunkMap extends ChunkStorage implements ChunkHolder.PlayerProvider // Paper start - optimise entity tracker private void newTrackerTick() { @@ -1576,6 +1576,7 @@ index 06cc9d69220b09667db30a5c1e161333d2563b23..65b9a49e84d9bde75c249651e974d57b + // Folia end - profiler final io.papermc.paper.threadedregions.RegionizedWorldData worldData = this.level.getCurrentWorldData(); // Folia - region threading final ca.spottedleaf.moonrise.patches.chunk_system.level.entity.server.ServerEntityLookup entityLookup = (ca.spottedleaf.moonrise.patches.chunk_system.level.entity.server.ServerEntityLookup)((ca.spottedleaf.moonrise.patches.chunk_system.level.ChunkSystemServerLevel)this.level).moonrise$getEntityLookup();; + final ca.spottedleaf.moonrise.common.misc.NearbyPlayers nearbyPlayers = this.level.moonrise$getNearbyPlayers(); // Folia - region threading final ca.spottedleaf.moonrise.common.list.ReferenceList trackerEntities = worldData.trackerEntities; // Folia - region threading final Entity[] trackerEntitiesRaw = trackerEntities.getRawDataUnchecked(); @@ -1584,7 +1585,7 @@ index 06cc9d69220b09667db30a5c1e161333d2563b23..65b9a49e84d9bde75c249651e974d57b final Entity entity = trackerEntitiesRaw[i]; final ChunkMap.TrackedEntity tracker = ((ca.spottedleaf.moonrise.patches.entity_tracker.EntityTrackerEntity)entity).moonrise$getTrackedEntity(); if (tracker == null) { -@@ -954,6 +964,8 @@ public class ChunkMap extends ChunkStorage implements ChunkHolder.PlayerProvider +@@ -955,6 +965,8 @@ public class ChunkMap extends ChunkStorage implements ChunkHolder.PlayerProvider tracker.serverEntity.sendChanges(); } } @@ -1594,10 +1595,10 @@ index 06cc9d69220b09667db30a5c1e161333d2563b23..65b9a49e84d9bde75c249651e974d57b // Paper end - optimise entity tracker diff --git a/net/minecraft/server/level/ServerChunkCache.java b/net/minecraft/server/level/ServerChunkCache.java -index c340d537749c49d83f50f6cec84ac75e1ace2bbd..089e694753e3c1b61902255442b26a3a6cbdf9af 100644 +index 548f5f0382c81ca86d238bfd7f94008bbd6e41bc..ac06b8a4813716a8d136be5731cbd96113976a7e 100644 --- a/net/minecraft/server/level/ServerChunkCache.java +++ b/net/minecraft/server/level/ServerChunkCache.java -@@ -483,17 +483,24 @@ public class ServerChunkCache extends ChunkSource implements ca.spottedleaf.moon +@@ -481,17 +481,24 @@ public class ServerChunkCache extends ChunkSource implements ca.spottedleaf.moon @Override public void tick(BooleanSupplier hasTimeLeft, boolean tickChunks) { @@ -1622,7 +1623,7 @@ index c340d537749c49d83f50f6cec84ac75e1ace2bbd..089e694753e3c1b61902255442b26a3a this.chunkMap.tick(); } -@@ -505,6 +512,7 @@ public class ServerChunkCache extends ChunkSource implements ca.spottedleaf.moon +@@ -503,6 +510,7 @@ public class ServerChunkCache extends ChunkSource implements ca.spottedleaf.moon private void tickChunks() { io.papermc.paper.threadedregions.RegionizedWorldData regionizedWorldData = this.level.getCurrentWorldData(); // Folia - region threading @@ -1630,7 +1631,7 @@ index c340d537749c49d83f50f6cec84ac75e1ace2bbd..089e694753e3c1b61902255442b26a3a //long gameTime = this.level.getGameTime(); // Folia - region threading long l = 1L; // Folia - region threading //this.lastInhabitedUpdate = gameTime; // Folia - region threading -@@ -516,7 +524,9 @@ public class ServerChunkCache extends ChunkSource implements ca.spottedleaf.moon +@@ -514,7 +522,9 @@ public class ServerChunkCache extends ChunkSource implements ca.spottedleaf.moon try { profilerFiller.push("filteringTickingChunks"); @@ -1640,7 +1641,7 @@ index c340d537749c49d83f50f6cec84ac75e1ace2bbd..089e694753e3c1b61902255442b26a3a profilerFiller.popPush("shuffleChunks"); // Paper start - chunk tick iteration optimisation this.shuffleRandom.setSeed(this.level.random.nextLong()); -@@ -529,7 +539,9 @@ public class ServerChunkCache extends ChunkSource implements ca.spottedleaf.moon +@@ -527,7 +537,9 @@ public class ServerChunkCache extends ChunkSource implements ca.spottedleaf.moon } } @@ -1650,7 +1651,7 @@ index c340d537749c49d83f50f6cec84ac75e1ace2bbd..089e694753e3c1b61902255442b26a3a profilerFiller.pop(); } } -@@ -574,10 +586,12 @@ public class ServerChunkCache extends ChunkSource implements ca.spottedleaf.moon +@@ -572,10 +584,12 @@ public class ServerChunkCache extends ChunkSource implements ca.spottedleaf.moon private void tickChunks(ProfilerFiller profiler, long timeInhabited, List chunks) { io.papermc.paper.threadedregions.RegionizedWorldData regionizedWorldData = this.level.getCurrentWorldData(); // Folia - region threading @@ -1663,7 +1664,7 @@ index c340d537749c49d83f50f6cec84ac75e1ace2bbd..089e694753e3c1b61902255442b26a3a if ((this.spawnFriendlies || this.spawnEnemies) && this.level.paperConfig().entities.spawning.perPlayerMobSpawns) { // don't count mobs when animals and monsters are disabled // re-set mob counts for (ServerPlayer player : this.level.getLocalPlayers()) { // Folia - region threading -@@ -597,6 +611,7 @@ public class ServerChunkCache extends ChunkSource implements ca.spottedleaf.moon +@@ -595,6 +609,7 @@ public class ServerChunkCache extends ChunkSource implements ca.spottedleaf.moon } else { spawnState = NaturalSpawner.createState(naturalSpawnChunkCount, regionizedWorldData.getLoadedEntities(), this::getFullChunk, !this.level.paperConfig().entities.spawning.perPlayerMobSpawns ? new LocalMobCapCalculator(this.chunkMap) : null, false); // Folia - region threading - note: function only cares about loaded entities, doesn't need all } @@ -1671,7 +1672,7 @@ index c340d537749c49d83f50f6cec84ac75e1ace2bbd..089e694753e3c1b61902255442b26a3a // Paper end - Optional per player mob spawns regionizedWorldData.lastSpawnState = spawnState; // Folia - region threading profiler.popPush("spawnAndTick"); -@@ -618,21 +633,31 @@ public class ServerChunkCache extends ChunkSource implements ca.spottedleaf.moon +@@ -616,21 +631,31 @@ public class ServerChunkCache extends ChunkSource implements ca.spottedleaf.moon filteredSpawningCategories = List.of(); } @@ -1704,7 +1705,7 @@ index c340d537749c49d83f50f6cec84ac75e1ace2bbd..089e694753e3c1b61902255442b26a3a } diff --git a/net/minecraft/server/level/ServerLevel.java b/net/minecraft/server/level/ServerLevel.java -index 49a385261deef774575dfd7a5b259d8ed31ed91a..0cc5607080f79f9e9b65606a3e16fd4961368b02 100644 +index ce310e2dc1bb46e17143bffc5e6cec7d43a0389d..d34ad333b6ea3855a24a58fcd80ccf19b2bbf41c 100644 --- a/net/minecraft/server/level/ServerLevel.java +++ b/net/minecraft/server/level/ServerLevel.java @@ -729,6 +729,7 @@ public class ServerLevel extends Level implements ServerEntityGetter, WorldGenLe diff --git a/folia-server/minecraft-patches/sources/ca/spottedleaf/moonrise/common/misc/NearbyPlayers.java.patch b/folia-server/minecraft-patches/sources/ca/spottedleaf/moonrise/common/misc/NearbyPlayers.java.patch new file mode 100644 index 0000000..0d26973 --- /dev/null +++ b/folia-server/minecraft-patches/sources/ca/spottedleaf/moonrise/common/misc/NearbyPlayers.java.patch @@ -0,0 +1,23 @@ +--- a/ca/spottedleaf/moonrise/common/misc/NearbyPlayers.java ++++ b/ca/spottedleaf/moonrise/common/misc/NearbyPlayers.java +@@ -244,7 +_,7 @@ + created.addPlayer(parameter, type); + type.addTo(parameter, NearbyPlayers.this.world, chunkX, chunkZ); + +- ((ChunkSystemLevel)NearbyPlayers.this.world).moonrise$requestChunkData(chunkKey).nearbyPlayers = created; ++ //((ChunkSystemLevel)NearbyPlayers.this.world).moonrise$requestChunkData(chunkKey).nearbyPlayers = created; // Folia - region threading + } + } + +@@ -263,10 +_,7 @@ + + if (chunk.isEmpty()) { + NearbyPlayers.this.byChunk.remove(chunkKey); +- final ChunkData chunkData = ((ChunkSystemLevel)NearbyPlayers.this.world).moonrise$releaseChunkData(chunkKey); +- if (chunkData != null) { +- chunkData.nearbyPlayers = null; +- } ++ // Folia - region threading + } + } + } diff --git a/folia-server/minecraft-patches/sources/ca/spottedleaf/moonrise/patches/chunk_system/level/chunk/ChunkData.java.patch b/folia-server/minecraft-patches/sources/ca/spottedleaf/moonrise/patches/chunk_system/level/chunk/ChunkData.java.patch new file mode 100644 index 0000000..976d861 --- /dev/null +++ b/folia-server/minecraft-patches/sources/ca/spottedleaf/moonrise/patches/chunk_system/level/chunk/ChunkData.java.patch @@ -0,0 +1,11 @@ +--- a/ca/spottedleaf/moonrise/patches/chunk_system/level/chunk/ChunkData.java ++++ b/ca/spottedleaf/moonrise/patches/chunk_system/level/chunk/ChunkData.java +@@ -5,7 +_,7 @@ + public final class ChunkData { + + private int referenceCount = 0; +- public NearbyPlayers.TrackedChunk nearbyPlayers; // Moonrise - nearby players ++ //public NearbyPlayers.TrackedChunk nearbyPlayers; // Moonrise - nearby players // Folia - region threading + + public ChunkData() { + diff --git a/folia-server/minecraft-patches/sources/net/minecraft/server/level/ChunkMap.java.patch b/folia-server/minecraft-patches/sources/net/minecraft/server/level/ChunkMap.java.patch index 467aa07..1379895 100644 --- a/folia-server/minecraft-patches/sources/net/minecraft/server/level/ChunkMap.java.patch +++ b/folia-server/minecraft-patches/sources/net/minecraft/server/level/ChunkMap.java.patch @@ -140,18 +140,28 @@ if (trackedEntity1 != null) { trackedEntity1.broadcastRemoved(); } -@@ -938,9 +_,10 @@ +@@ -938,9 +_,11 @@ // Paper start - optimise entity tracker private void newTrackerTick() { + final io.papermc.paper.threadedregions.RegionizedWorldData worldData = this.level.getCurrentWorldData(); // Folia - region threading final ca.spottedleaf.moonrise.patches.chunk_system.level.entity.server.ServerEntityLookup entityLookup = (ca.spottedleaf.moonrise.patches.chunk_system.level.entity.server.ServerEntityLookup)((ca.spottedleaf.moonrise.patches.chunk_system.level.ChunkSystemServerLevel)this.level).moonrise$getEntityLookup();; ++ final ca.spottedleaf.moonrise.common.misc.NearbyPlayers nearbyPlayers = this.level.moonrise$getNearbyPlayers(); // Folia - region threading - final ca.spottedleaf.moonrise.common.list.ReferenceList trackerEntities = entityLookup.trackerEntities; + final ca.spottedleaf.moonrise.common.list.ReferenceList trackerEntities = worldData.trackerEntities; // Folia - region threading final Entity[] trackerEntitiesRaw = trackerEntities.getRawDataUnchecked(); for (int i = 0, len = trackerEntities.size(); i < len; ++i) { final Entity entity = trackerEntitiesRaw[i]; +@@ -948,7 +_,7 @@ + if (tracker == null) { + continue; + } +- ((ca.spottedleaf.moonrise.patches.entity_tracker.EntityTrackerTrackedEntity)tracker).moonrise$tick(((ca.spottedleaf.moonrise.patches.chunk_system.entity.ChunkSystemEntity)entity).moonrise$getChunkData().nearbyPlayers); ++ ((ca.spottedleaf.moonrise.patches.entity_tracker.EntityTrackerTrackedEntity)tracker).moonrise$tick(nearbyPlayers.getChunk(entity.chunkPosition())); // Folia - region threading + if (((ca.spottedleaf.moonrise.patches.entity_tracker.EntityTrackerTrackedEntity)tracker).moonrise$hasPlayers() + || ((ca.spottedleaf.moonrise.patches.chunk_system.entity.ChunkSystemEntity)entity).moonrise$getChunkStatus().isOrAfter(FullChunkStatus.ENTITY_TICKING)) { + tracker.serverEntity.sendChanges(); @@ -966,44 +_,18 @@ // Paper end - optimise entity tracker // Paper - rewrite chunk system diff --git a/folia-server/minecraft-patches/sources/net/minecraft/server/level/ServerChunkCache.java.patch b/folia-server/minecraft-patches/sources/net/minecraft/server/level/ServerChunkCache.java.patch index 450a7a8..07a9bd4 100644 --- a/folia-server/minecraft-patches/sources/net/minecraft/server/level/ServerChunkCache.java.patch +++ b/folia-server/minecraft-patches/sources/net/minecraft/server/level/ServerChunkCache.java.patch @@ -33,6 +33,17 @@ final ca.spottedleaf.moonrise.patches.chunk_system.scheduling.ChunkTaskScheduler chunkTaskScheduler = ((ca.spottedleaf.moonrise.patches.chunk_system.level.ChunkSystemServerLevel)this.level).moonrise$getChunkTaskScheduler(); final CompletableFuture completable = new CompletableFuture<>(); chunkTaskScheduler.scheduleChunkLoad( +@@ -154,9 +_,7 @@ + // Paper start - chunk tick iteration optimisations + private final ca.spottedleaf.moonrise.common.util.SimpleThreadUnsafeRandom shuffleRandom = new ca.spottedleaf.moonrise.common.util.SimpleThreadUnsafeRandom(0L); + private boolean isChunkNearPlayer(final ChunkMap chunkMap, final ChunkPos chunkPos, final LevelChunk levelChunk) { +- final ca.spottedleaf.moonrise.patches.chunk_system.level.chunk.ChunkData chunkData = ((ca.spottedleaf.moonrise.patches.chunk_system.level.chunk.ChunkSystemChunkHolder)((ca.spottedleaf.moonrise.patches.chunk_system.level.chunk.ChunkSystemLevelChunk)levelChunk).moonrise$getChunkAndHolder().holder()) +- .moonrise$getRealChunkHolder().holderData; +- final ca.spottedleaf.moonrise.common.misc.NearbyPlayers.TrackedChunk nearbyPlayers = chunkData.nearbyPlayers; ++ final ca.spottedleaf.moonrise.common.misc.NearbyPlayers.TrackedChunk nearbyPlayers = this.level.moonrise$getNearbyPlayers().getChunk(chunkPos); // Folia - region threading + if (nearbyPlayers == null) { + return false; + } @@ -355,6 +_,7 @@ }