From 9c055a0fadf44bc6f6fab5c1e26e49107d61e77b Mon Sep 17 00:00:00 2001
From: Sam Brannen <sam@sambrannen.com>
Date: Wed, 6 May 2020 18:46:43 +0200
Subject: [PATCH] Fix concurrency issues in XStreamMarshaller

Prior to this commit, if an instance of XStreamMarshaller was used
concurrently from multiple threads without having first invoked the
afterPropertiesSet() method, the private `xstream` field could be
initialized multiple times resulting in a ConcurrentModificationException
in XStream's internal DefaultConverterLookup.

This commit fixes these concurrency issues by making the `xstream` field
volatile and by implementing a double-checked locking algorithm in
getXStream() to avoid concurrent instance creation.

Closes gh-25017
---
 .../oxm/xstream/XStreamMarshaller.java        | 20 ++++++++++----
 .../oxm/xstream/XStreamMarshallerTests.java   | 27 +++++++++++++------
 2 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/spring-oxm/src/main/java/org/springframework/oxm/xstream/XStreamMarshaller.java b/spring-oxm/src/main/java/org/springframework/oxm/xstream/XStreamMarshaller.java
index df1100e471cf..596b587fa292 100644
--- a/spring-oxm/src/main/java/org/springframework/oxm/xstream/XStreamMarshaller.java
+++ b/spring-oxm/src/main/java/org/springframework/oxm/xstream/XStreamMarshaller.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2020 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -113,6 +113,7 @@
  * @author Peter Meijer
  * @author Arjen Poutsma
  * @author Juergen Hoeller
+ * @author Sam Brannen
  * @since 3.0
  */
 public class XStreamMarshaller extends AbstractMarshaller implements BeanClassLoaderAware, InitializingBean {
@@ -187,7 +188,7 @@ public class XStreamMarshaller extends AbstractMarshaller implements BeanClassLo
 	private ClassLoader beanClassLoader = new CompositeClassLoader();
 
 	@Nullable
-	private XStream xstream;
+	private volatile XStream xstream;
 
 
 	/**
@@ -616,12 +617,21 @@ protected void customizeXStream(XStream xstream) {
 	 * <p><b>NOTE: This method has been marked as final as of Spring 4.0.</b>
 	 * It can be used to access the fully configured XStream for marshalling
 	 * but not configuration purposes anymore.
+	 * <p>As of Spring Framework 5.2.7, creation of the {@link XStream} instance
+	 * returned by this method is thread safe.
 	 */
 	public final XStream getXStream() {
-		if (this.xstream == null) {
-			this.xstream = buildXStream();
+		XStream xs = this.xstream;
+		if (xs == null) {
+			synchronized (this) {
+				xs = this.xstream;
+				if (xs == null) {
+					xs = buildXStream();
+					this.xstream = xs;
+				}
+			}
 		}
-		return this.xstream;
+		return xs;
 	}
 
 
diff --git a/spring-oxm/src/test/java/org/springframework/oxm/xstream/XStreamMarshallerTests.java b/spring-oxm/src/test/java/org/springframework/oxm/xstream/XStreamMarshallerTests.java
index 6cd086fe33da..73e528bc5d18 100644
--- a/spring-oxm/src/test/java/org/springframework/oxm/xstream/XStreamMarshallerTests.java
+++ b/spring-oxm/src/test/java/org/springframework/oxm/xstream/XStreamMarshallerTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2020 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -26,6 +26,7 @@
 import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.stream.IntStream;
 
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
@@ -188,13 +189,23 @@ public void marshalStaxResultXMLEventWriter() throws Exception {
 	@Test
 	public void converters() throws Exception {
 		marshaller.setConverters(new EncodedByteArrayConverter());
-		byte[] buf = new byte[]{0x1, 0x2};
-		Writer writer = new StringWriter();
-		marshaller.marshal(buf, new StreamResult(writer));
-		assertThat(writer.toString(), isSimilarTo("<byte-array>AQI=</byte-array>"));
-		Reader reader = new StringReader(writer.toString());
-		byte[] bufResult = (byte[]) marshaller.unmarshal(new StreamSource(reader));
-		assertTrue("Invalid result", Arrays.equals(buf, bufResult));
+		byte[] buf = {0x1, 0x2};
+
+		// Execute multiple times concurrently to ensure there are no concurrency issues.
+		// See https://github.com/spring-projects/spring-framework/issues/25017
+		IntStream.rangeClosed(1, 100).parallel().forEach(n -> {
+			try {
+				Writer writer = new StringWriter();
+				marshaller.marshal(buf, new StreamResult(writer));
+				assertThat(writer.toString(), isSimilarTo("<byte-array>AQI=</byte-array>"));
+				Reader reader = new StringReader(writer.toString());
+				byte[] bufResult = (byte[]) marshaller.unmarshal(new StreamSource(reader));
+				assertTrue("Invalid result", Arrays.equals(buf, bufResult));
+			}
+			catch (Exception ex) {
+				throw new RuntimeException(ex);
+			}
+		});
 	}
 
 	@Test