[Bleeding] Fix corruption due to thread safety issues. Fixes BUKKIT-3333

The 'tag' NBTTagCompound field of the ItemStack assumes that it is OK to
save a reference to an NBT supplied via load() and assumes it is OK to
supply a reference to the internal field during a save(). Neither is true,
as Chunk NBT structures are required to be read-only once created (due to
being written asynchronously off the server thread AND due to the potential
to be passed to a new Chunk if the same chunk is reloaded before the
writing of the NBT is completed by the File I/O thread). Keeping a live
reference to the NBT copy passed in, or to the NBT value passed back
during saving, creates serious thread safety issues which can result in
corrupted data being written to the world data files.

The specific issue here was uncovered by the recent change to use
setName("") on the ItemStack.tag object. When a chunk is being loaded
again before its save is completed, this results in name of the field
in the NBT being set to "". This causes it to be saved as "" instead
of "tag" resulting in it not being properly reloaded in the future which
results in the itemstack losing all of its metadata.

1 files changed, 3 insertions, 3 deletions

diff --git a/src/main/java/net/minecraft/server/ItemStack.java b/src/main/java/net/minecraft/server/ItemStack.java
index 453ca4ac..7c1bbec3 100644
--- a/src/main/java/net/minecraft/server/ItemStack.java
+++ b/src/main/java/net/minecraft/server/ItemStack.java
@@ -95,7 +95,7 @@ public final class ItemStack {
         nbttagcompound.setByte("Count", (byte) this.count);
         nbttagcompound.setShort("Damage", (short) this.damage);
         if (this.tag != null) {
-            nbttagcompound.set("tag", this.tag);
+            nbttagcompound.set("tag", this.tag.clone()); // CraftBukkit - make defensive copy, data is going to another thread
         }
 
         return nbttagcompound;
@@ -106,8 +106,8 @@ public final class ItemStack {
         this.count = nbttagcompound.getByte("Count");
         this.damage = nbttagcompound.getShort("Damage");
         if (nbttagcompound.hasKey("tag")) {
-            // CraftBukkit - clear name from compound
-            this.tag = (NBTTagCompound) nbttagcompound.getCompound("tag").setName("");
+            // CraftBukkit - clear name from compound and make defensive copy as this data may be coming from the save thread
+            this.tag = (NBTTagCompound) nbttagcompound.getCompound("tag").clone().setName("");
         }
     }