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
This commit is contained in:
Sam Brannen
2020-05-06 18:46:43 +02:00
committed by Sam Brannen
parent 42f60fe490
commit f474595dee
2 changed files with 34 additions and 13 deletions

View File

@@ -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) {
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;
}

View File

@@ -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.Collections;
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 class XStreamMarshallerTests {
@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