Fix XML serialiser for SpecConverter It was not thread-safe. The XmlMementoSerializer uses a single instance of SpecConverter. If multiple threads tried to deserialise different objects at the same time, theyâd overwrite the âinstanceâ value and cause serious problems deserialising.
Now use thread-local storage for the âinstanceâ cache. Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/b5dae5b7 Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/b5dae5b7 Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/b5dae5b7 Branch: refs/heads/master Commit: b5dae5b7ad5b4d0919e00da1f02b5a54a1e4d3e1 Parents: 89b3a66 Author: Aled Sage <aled.s...@gmail.com> Authored: Tue Mar 22 12:47:36 2016 +0000 Committer: Aled Sage <aled.s...@gmail.com> Committed: Tue Mar 22 12:47:36 2016 +0000 ---------------------------------------------------------------------- .../core/mgmt/persist/XmlMementoSerializer.java | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/b5dae5b7/core/src/main/java/org/apache/brooklyn/core/mgmt/persist/XmlMementoSerializer.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/persist/XmlMementoSerializer.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/persist/XmlMementoSerializer.java index 588f8fd..0df2340 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/persist/XmlMementoSerializer.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/persist/XmlMementoSerializer.java @@ -393,6 +393,12 @@ public class XmlMementoSerializer<T> extends XmlSerializer<T> implements Memento } } + // Would prefer this as a field of SpecConverter, but can't do that because the class is not static. + // Must use thread-local storage because a single instance of the SpecConverter is used by this + // XmlMementoSerializer. In BrooklynMementoPersisterToObjectStore.visitMemento, it uses a thread-pool + // for concurrently deserializing multiple objects. + private static final ThreadLocal<Object> SpecConverterLocalInstance = new ThreadLocal<Object>(); + /** When reading/writing specs, it checks whether there is a catalog item id set and uses it to load */ public class SpecConverter extends ReflectionConverter { SpecConverter() { @@ -454,25 +460,25 @@ public class XmlMementoSerializer<T> extends XmlSerializer<T> implements Memento result.catalogItemId(catalogItemId); return result; } finally { - instance = null; + SpecConverterLocalInstance.remove(); if (customLoaderSet) { popXstreamCustomClassLoader(); } } } - Object instance; - @Override protected Object instantiateNewInstance(HierarchicalStreamReader reader, UnmarshallingContext context) { // the super calls getAttribute which requires that we have not yet done moveDown, // so we do this earlier and cache it for when we call super.unmarshal + Object instance = SpecConverterLocalInstance.get(); if (instance==null) throw new IllegalStateException("Instance should be created and cached"); return instance; } protected void instantiateNewInstanceSettingCache(HierarchicalStreamReader reader, UnmarshallingContext context) { - instance = super.instantiateNewInstance(reader, context); + Object instance = super.instantiateNewInstance(reader, context); + SpecConverterLocalInstance.set(instance); } }
