OvercastNetwork / SportBukkit

CraftBukkit and Bukkit modifications that improve stability and add new features
99 stars 84 forks source link

Threadsafe metadata #136

Closed jedediah closed 9 years ago

jedediah commented 9 years ago

As far as I can tell, the way the vanilla server serializes entity metadata is not threadsafe at all. WatchableObjects are stored in the packet, and because they are mutable, can change value before or during serialization. DataWatcher uses a lock to synchronize access to the container that holds the WatchableObjects, but the WOs themselves have no synchronization.

Also, one of the types that WO can wrap is ItemStack which can also be mutated while the IO thread is serializing it. We have seen a ConcurrentModificationException on our servers that seems to be a symptom of this.

This patch clones all WatchableObjects when the packets containing them are created, as well as any ItemStacks they contain. This should ensure that what is serialized is always a snapshot of the metadata when the packet was instantiated on the main thread.

jedediah commented 9 years ago

Ok, this seems to be working fine now