From da3b503596a1ca291ed4b5922b573f6d1bdea67d Mon Sep 17 00:00:00 2001 From: Shane Freeder Date: Sat, 18 May 2019 06:01:44 +0100 Subject: [PATCH] Restore some entity duplication fix patches --- ...isPrimaryThread-and-MinecraftServer-.patch | 10 +- ...h-entity-loss-due-to-unloaded-chunks.patch | 45 ++++ .../0395-Duplicate-UUID-Resolve-Option.patch | 245 ++++++++++++++++++ 3 files changed, 295 insertions(+), 5 deletions(-) create mode 100644 Spigot-Server-Patches/0394-Fix-issues-with-entity-loss-due-to-unloaded-chunks.patch create mode 100644 Spigot-Server-Patches/0395-Duplicate-UUID-Resolve-Option.patch diff --git a/Spigot-Server-Patches/0392-Fix-CraftServer-isPrimaryThread-and-MinecraftServer-.patch b/Spigot-Server-Patches/0392-Fix-CraftServer-isPrimaryThread-and-MinecraftServer-.patch index a18c51274..ec74e12e8 100644 --- a/Spigot-Server-Patches/0392-Fix-CraftServer-isPrimaryThread-and-MinecraftServer-.patch +++ b/Spigot-Server-Patches/0392-Fix-CraftServer-isPrimaryThread-and-MinecraftServer-.patch @@ -1,4 +1,4 @@ -From 32cdfb9500efe574a54465daca2204f8fe78d4a2 Mon Sep 17 00:00:00 2001 +From 3dabcd7c52beadaadbc2858a38b0252c2b8dca51 Mon Sep 17 00:00:00 2001 From: Spottedleaf Date: Mon, 13 May 2019 21:10:59 -0700 Subject: [PATCH] Fix CraftServer#isPrimaryThread and MinecraftServer @@ -11,12 +11,12 @@ of behavior for this exact reason. md_5 also placed code in PlayerConnectionUtils that would have solved https://bugs.mojang.com/browse/MC-142590, making the change to MinecraftServer#isMainThread irrelevant. -By reverting his change to MinecraftServer#isMainThread packet -handling that should have been handled synchronously will be handled +By reverting his change to MinecraftServer#isMainThread packet +handling that should have been handled synchronously will be handled synchronously when the server gets shut down. diff --git a/src/main/java/net/minecraft/server/MinecraftServer.java b/src/main/java/net/minecraft/server/MinecraftServer.java -index 70f730996c..73ac45bf7a 100644 +index 70f730996..73ac45bf7 100644 --- a/src/main/java/net/minecraft/server/MinecraftServer.java +++ b/src/main/java/net/minecraft/server/MinecraftServer.java @@ -1945,7 +1945,7 @@ public abstract class MinecraftServer extends IAsyncTaskHandlerReentrant +Date: Fri, 28 Sep 2018 21:49:53 -0400 +Subject: [PATCH] Fix issues with entity loss due to unloaded chunks + +Vanilla has risk of losing entities by causing them to be +removed from all chunks if they try to move into an unloaded chunk. + +This pretty much means high chance this entity will be lost in this +scenario. + +There is another case that adding an entity to the world can fail if +the chunk isn't loaded. + +Lots of the server is designed around addEntity never expecting to fail +for these reasons, nor is it really logical. + +This change ensures the chunks are always loaded when entities are +added to the world, or a valid entity moves between chunks. + +diff --git a/src/main/java/net/minecraft/server/WorldServer.java b/src/main/java/net/minecraft/server/WorldServer.java +index 1a01e5e69..be3ce3f13 100644 +--- a/src/main/java/net/minecraft/server/WorldServer.java ++++ b/src/main/java/net/minecraft/server/WorldServer.java +@@ -635,7 +635,7 @@ public class WorldServer extends World { + this.getChunkAt(entity.chunkX, entity.chunkZ).a(entity, entity.chunkY); + } + +- if (!entity.bT() && !this.isChunkLoaded(i, k)) { ++ if (!entity.valid && !entity.bT() && !this.isChunkLoaded(i, k)) { // Paper - always load chunks to register valid entities location + entity.inChunk = false; + } else { + this.getChunkAt(i, k).a(entity); +@@ -950,7 +950,7 @@ public class WorldServer extends World { + return false; + } + // CraftBukkit end +- IChunkAccess ichunkaccess = this.getChunkAt(MathHelper.floor(entity.locX / 16.0D), MathHelper.floor(entity.locZ / 16.0D), ChunkStatus.FULL, entity.attachedToPlayer); ++ IChunkAccess ichunkaccess = this.getChunkAt(MathHelper.floor(entity.locX / 16.0D), MathHelper.floor(entity.locZ / 16.0D), ChunkStatus.FULL, true); // Paper - always load chunks for entity adds + + if (!(ichunkaccess instanceof Chunk)) { + return false; +-- +2.21.0 + diff --git a/Spigot-Server-Patches/0395-Duplicate-UUID-Resolve-Option.patch b/Spigot-Server-Patches/0395-Duplicate-UUID-Resolve-Option.patch new file mode 100644 index 000000000..f412af5a4 --- /dev/null +++ b/Spigot-Server-Patches/0395-Duplicate-UUID-Resolve-Option.patch @@ -0,0 +1,245 @@ +From 7c17dfd40b62a1e3e8f0cc2e0d0a7f46513e109f Mon Sep 17 00:00:00 2001 +From: Aikar +Date: Sat, 21 Jul 2018 14:27:34 -0400 +Subject: [PATCH] Duplicate UUID Resolve Option + +Due to a bug in https://github.com/PaperMC/Paper/commit/2e29af3df05ec0a383f48be549d1c03200756d24 +which was added all the way back in March of 2016, it was unknown (potentially not at the time) +that an entity might actually change the seed of the random object. + +At some point, EntitySquid did start setting the seed. Due to this shared random, this caused +every entity to use a Random object with a predictable seed. + +This has caused entities to potentially generate with the same UUID.... + +Over the years, servers have had entities disappear, but no sign of trouble +because CraftBukkit removed the log lines indicating that something was wrong. + +We have fixed the root issue causing duplicate UUID's, however we now have chunk +files full of entities that have the same UUID as another entity! + +When these chunks load, the 2nd entity will not be added to the world correctly. + +If that chunk loads in a different order in the future, then it will reverse and the +missing one is now the one added to the world and not the other. This results in very +inconsistent entity behavior. + +This change allows you to recover any duplicate entity by generating a new UUID for it. +This also lets you delete them instead if you don't want to risk having new entities added to +the world that you previously did not see. + +But for those who are ok with leaving this inconsistent behavior, you may use WARN or NOTHING options. + +It is recommended you regenerate the entities, as these were legit entities, and deserve your love. + +diff --git a/src/main/java/com/destroystokyo/paper/PaperWorldConfig.java b/src/main/java/com/destroystokyo/paper/PaperWorldConfig.java +index ef882b897..385b3ac0c 100644 +--- a/src/main/java/com/destroystokyo/paper/PaperWorldConfig.java ++++ b/src/main/java/com/destroystokyo/paper/PaperWorldConfig.java +@@ -449,4 +449,43 @@ public class PaperWorldConfig { + log("Using vanilla redstone algorithm."); + } + } ++ ++ public enum DuplicateUUIDMode { ++ SAFE_REGEN, DELETE, NOTHING, WARN ++ } ++ public DuplicateUUIDMode duplicateUUIDMode = DuplicateUUIDMode.SAFE_REGEN; ++ public int duplicateUUIDDeleteRange = 32; ++ private void repairDuplicateUUID() { ++ String desiredMode = getString("duplicate-uuid-resolver", "saferegen").toLowerCase().trim(); ++ duplicateUUIDDeleteRange = getInt("duplicate-uuid-saferegen-delete-range", duplicateUUIDDeleteRange); ++ switch (desiredMode.toLowerCase()) { ++ case "regen": ++ case "regenerate": ++ case "saferegen": ++ case "saferegenerate": ++ duplicateUUIDMode = DuplicateUUIDMode.SAFE_REGEN; ++ log("Duplicate UUID Resolve: Regenerate New UUID if distant (Delete likely duplicates within " + duplicateUUIDDeleteRange + " blocks)"); ++ break; ++ case "remove": ++ case "delete": ++ duplicateUUIDMode = DuplicateUUIDMode.DELETE; ++ log("Duplicate UUID Resolve: Delete Entity"); ++ break; ++ case "silent": ++ case "nothing": ++ duplicateUUIDMode = DuplicateUUIDMode.NOTHING; ++ logError("Duplicate UUID Resolve: Do Nothing (no logs) - Warning, may lose indication of bad things happening"); ++ break; ++ case "log": ++ case "warn": ++ duplicateUUIDMode = DuplicateUUIDMode.WARN; ++ log("Duplicate UUID Resolve: Warn (do nothing but log it happened, may be spammy)"); ++ break; ++ default: ++ duplicateUUIDMode = DuplicateUUIDMode.WARN; ++ logError("Warning: Invalid duplicate-uuid-resolver config " + desiredMode + " - must be one of: regen, delete, nothing, warn"); ++ log("Duplicate UUID Resolve: Warn (do nothing but log it happened, may be spammy)"); ++ break; ++ } ++ } + } +diff --git a/src/main/java/net/minecraft/server/Chunk.java b/src/main/java/net/minecraft/server/Chunk.java +index 4d96e4fb1..26cf42a45 100644 +--- a/src/main/java/net/minecraft/server/Chunk.java ++++ b/src/main/java/net/minecraft/server/Chunk.java +@@ -392,6 +392,7 @@ public class Chunk implements IChunkAccess { + if (i != this.loc.x || j != this.loc.z) { + Chunk.LOGGER.warn("Wrong location! ({}, {}) should be ({}, {}), {}", i, j, this.loc.x, this.loc.z, entity); + entity.dead = true; ++ return; // Paper + } + + int k = MathHelper.floor(entity.locY / 16.0D); +diff --git a/src/main/java/net/minecraft/server/Entity.java b/src/main/java/net/minecraft/server/Entity.java +index d02e48a86..3b1555dd2 100644 +--- a/src/main/java/net/minecraft/server/Entity.java ++++ b/src/main/java/net/minecraft/server/Entity.java +@@ -2669,6 +2669,7 @@ public abstract class Entity implements INamableTileEntity, ICommandListener, Ke + }); + } + ++ public void setUUID(UUID uuid) { a(uuid); } // Paper - OBFHELPER + public void a(UUID uuid) { + this.uniqueID = uuid; + this.ap = this.uniqueID.toString(); +diff --git a/src/main/java/net/minecraft/server/PlayerChunkMap.java b/src/main/java/net/minecraft/server/PlayerChunkMap.java +index 1cf516192..292fa782d 100644 +--- a/src/main/java/net/minecraft/server/PlayerChunkMap.java ++++ b/src/main/java/net/minecraft/server/PlayerChunkMap.java +@@ -1,5 +1,6 @@ + package net.minecraft.server; + ++import com.destroystokyo.paper.PaperWorldConfig; // Paper + import com.google.common.collect.ImmutableList; + import co.aikar.timings.Timing; + import com.google.common.collect.ComparisonChain; +@@ -19,11 +20,14 @@ import it.unimi.dsi.fastutil.objects.ObjectIterator; + import java.io.File; + import java.io.IOException; + import java.util.ArrayList; ++import java.util.HashMap; // Paper + import java.util.Iterator; + import java.util.List; ++import java.util.Map; // Paper + import java.util.Objects; + import java.util.Optional; + import java.util.Set; ++import java.util.UUID; // Paper + import java.util.concurrent.CompletableFuture; + import java.util.concurrent.Executor; + import java.util.concurrent.atomic.AtomicInteger; +@@ -502,12 +506,49 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d { + + for (int j = 0; j < i; ++j) { + List entityslice = aentityslice[j]; // Spigot +- Iterator iterator = entityslice.iterator(); + +- while (iterator.hasNext()) { +- Entity entity = (Entity) iterator.next(); ++ // Paper start ++ PaperWorldConfig.DuplicateUUIDMode mode = world.paperConfig.duplicateUUIDMode; ++ if (mode == PaperWorldConfig.DuplicateUUIDMode.WARN || mode == PaperWorldConfig.DuplicateUUIDMode.DELETE || mode == PaperWorldConfig.DuplicateUUIDMode.SAFE_REGEN) { ++ Map thisChunk = new HashMap<>(); ++ for (Iterator iterator = ((List) entityslice).iterator(); iterator.hasNext(); ) { ++ Entity entity = iterator.next(); ++ if (entity.dead || entity.valid) continue; ++ Entity other = ((WorldServer) world).getEntity(entity.uniqueID); ++ if (other == null || other.dead) { ++ other = thisChunk.get(entity.uniqueID); ++ } ++ ++ if (mode == PaperWorldConfig.DuplicateUUIDMode.SAFE_REGEN && other != null && !other.dead ++ && java.util.Objects.equals(other.getSaveID(), entity.getSaveID()) ++ && entity.getBukkitEntity().getLocation().distance(other.getBukkitEntity().getLocation()) < world.paperConfig.duplicateUUIDDeleteRange ++ ) { ++ if (World.DEBUG_ENTITIES) LOGGER.warn("[DUPE-UUID] Duplicate UUID found used by " + other + ", deleted entity " + entity + " because it was near the duplicate and likely an actual duplicate. See https://github.com/PaperMC/Paper/issues/1223 for discussion on what this is about."); ++ entity.die(); ++ iterator.remove(); ++ continue; ++ } ++ if (other != null && !other.dead) { ++ switch (mode) { ++ case SAFE_REGEN: { ++ entity.setUUID(UUID.randomUUID()); ++ if (World.DEBUG_ENTITIES) LOGGER.warn("[DUPE-UUID] Duplicate UUID found used by " + other + ", regenerated UUID for " + entity + ". See https://github.com/PaperMC/Paper/issues/1223 for discussion on what this is about."); ++ break; ++ } ++ case DELETE: { ++ if (World.DEBUG_ENTITIES) LOGGER.warn("[DUPE-UUID] Duplicate UUID found used by " + other + ", deleted entity " + entity + ". See https://github.com/PaperMC/Paper/issues/1223 for discussion on what this is about."); ++ entity.die(); ++ iterator.remove(); ++ break; ++ } ++ default: ++ if (World.DEBUG_ENTITIES) LOGGER.warn("[DUPE-UUID] Duplicate UUID found used by " + other + ", doing nothing to " + entity + ". See https://github.com/PaperMC/Paper/issues/1223 for discussion on what this is about."); ++ break; ++ } ++ } ++ + +- if (!(entity instanceof EntityHuman) && !this.world.addEntityChunk(entity)) { ++ if (!(entity instanceof EntityHuman) && (entity.dead || !this.world.addEntityChunk(entity))) { // Paper + if (list == null) { + list = Lists.newArrayList(new Entity[] { entity}); + } else { +@@ -515,6 +556,7 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d { + } + } + } ++ } // Paper + } + + if (list != null) { +diff --git a/src/main/java/net/minecraft/server/WorldServer.java b/src/main/java/net/minecraft/server/WorldServer.java +index be3ce3f13..3ed12672e 100644 +--- a/src/main/java/net/minecraft/server/WorldServer.java ++++ b/src/main/java/net/minecraft/server/WorldServer.java +@@ -2,6 +2,8 @@ package net.minecraft.server; + + import co.aikar.timings.TimingHistory; + import co.aikar.timings.Timings; ++ ++import com.destroystokyo.paper.PaperWorldConfig; + import com.google.common.collect.Lists; + import com.google.common.collect.Maps; + import com.google.common.collect.Queues; +@@ -977,8 +979,23 @@ public class WorldServer extends World { + if (entity1 == null) { + return false; + } else { +- WorldServer.LOGGER.error("Keeping entity {} that already exists with UUID {}", EntityTypes.getName(entity1.getEntityType()), entity.getUniqueID().toString()); // CraftBukkit // paper +- WorldServer.LOGGER.error("Deleting duplicate entity {}", entity); // CraftBukkit // paper ++ // Paper start ++ if (entity1.dead) { ++ unregisterEntity(entity1); // remove the existing entity ++ return false; ++ } ++ ++ if (DEBUG_ENTITIES && entity.world.paperConfig.duplicateUUIDMode != PaperWorldConfig.DuplicateUUIDMode.NOTHING) { ++ WorldServer.LOGGER.error("Keeping entity {} that already exists with UUID {}", EntityTypes.getName(entity1.getEntityType()), entity.getUniqueID().toString()); // CraftBukkit // paper ++ WorldServer.LOGGER.error("Deleting duplicate entity {}", entity); // CraftBukkit // paper ++ ++ if (entity1.addedToWorldStack != null) { ++ entity1.addedToWorldStack.printStackTrace(); ++ } ++ ++ getAddToWorldStackTrace(entity).printStackTrace(); ++ } ++ // Paper end + return true; + } + } +@@ -1109,7 +1126,7 @@ public class WorldServer extends World { + } + + Entity old = this.entitiesByUUID.put(entity.getUniqueID(), entity); +- if (old != null && old.getId() != entity.getId() && old.valid) { ++ if (old != null && old.getId() != entity.getId() && old.valid && entity.world.paperConfig.duplicateUUIDMode != com.destroystokyo.paper.PaperWorldConfig.DuplicateUUIDMode.NOTHING) { // Paper + Logger logger = LogManager.getLogger(); + logger.error("Overwrote an existing entity " + old + " with " + entity); + if (DEBUG_ENTITIES) { +-- +2.21.0 +