Skip to content

Commit

Permalink
Fix concurrency issues in XStreamMarshaller
Browse files Browse the repository at this point in the history
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
  • Loading branch information
sbrannen committed May 7, 2020
1 parent 42f60fe commit f474595
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 13 deletions.
@@ -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.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -187,7 +188,7 @@ public class XStreamMarshaller extends AbstractMarshaller implements BeanClassLo
private ClassLoader beanClassLoader = new CompositeClassLoader();

@Nullable
private XStream xstream;
private volatile XStream xstream;


/**
Expand Down Expand Up @@ -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;
}


Expand Down
@@ -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.
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit f474595

Please sign in to comment.