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

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 @@ import org.springframework.util.xml.StaxUtils;
  * @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 @@ public class XStreamMarshaller extends AbstractMarshaller implements BeanClassLo
 	 * <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) {
+		XStream xs = this.xstream;
-			this.xstream = buildXStream();
+		if (xs == null) {
+			synchronized (this) {
+				xs = this.xstream;
+				if (xs == null) {
+					xs = buildXStream();
+					this.xstream = xs;
 				}
-		return this.xstream;
+			}
+		}
+		return xs;
 	}
 
 

spring-oxm/src/test/java/org/springframework/oxm/xstream/XStreamMarshallerTests.java

@@ -21,10 +21,10 @@ import java.io.Reader;
 import java.io.StringReader;
 import java.io.StringWriter;
 import java.io.Writer;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.stream.IntStream;
 
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
@@ -185,13 +185,23 @@ class XStreamMarshallerTests {
 	@Test
 	void converters() throws Exception {
 		marshaller.setConverters(new EncodedByteArrayConverter());
-		byte[] buf = new byte[]{0x1, 0x2};
+		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(XmlContent.from(writer)).isSimilarTo("<byte-array>AQI=</byte-array>");
 				Reader reader = new StringReader(writer.toString());
 				byte[] bufResult = (byte[]) marshaller.unmarshal(new StreamSource(reader));
-		assertThat(Arrays.equals(buf, bufResult)).as("Invalid result").isTrue();
+				assertThat(bufResult).as("Invalid result").isEqualTo(buf);
+			}
+			catch (Exception ex) {
+				throw new RuntimeException(ex);
+			}
+		});
 	}
 
 	@Test