Ensure TypeConverterConverter is thread safe

Update `TypeConverterConverter` do that a new `SimpleTypeConverter` is
obtained for each `convert` operation. Prior to this commit the same
`SimpleTypeConverter` could be accessed concurrently from multiple
threads which is not allowed.

Fixes gh-27829
pull/28061/head
Phillip Webb 3 years ago
parent 69be1c872e
commit 87dbda2339

@ -185,18 +185,10 @@ final class BindConverter {
private static class TypeConverterConversionService extends GenericConversionService { private static class TypeConverterConversionService extends GenericConversionService {
TypeConverterConversionService(Consumer<PropertyEditorRegistry> initializer) { TypeConverterConversionService(Consumer<PropertyEditorRegistry> initializer) {
addConverter(new TypeConverterConverter(createTypeConverter(initializer))); addConverter(new TypeConverterConverter(initializer));
ApplicationConversionService.addDelimitedStringConverters(this); ApplicationConversionService.addDelimitedStringConverters(this);
} }
private SimpleTypeConverter createTypeConverter(Consumer<PropertyEditorRegistry> initializer) {
SimpleTypeConverter typeConverter = new SimpleTypeConverter();
if (initializer != null) {
initializer.accept(typeConverter);
}
return typeConverter;
}
@Override @Override
public boolean canConvert(TypeDescriptor sourceType, TypeDescriptor targetType) { public boolean canConvert(TypeDescriptor sourceType, TypeDescriptor targetType) {
// Prefer conversion service to handle things like String to char[]. // Prefer conversion service to handle things like String to char[].
@ -213,10 +205,15 @@ final class BindConverter {
*/ */
private static class TypeConverterConverter implements ConditionalGenericConverter { private static class TypeConverterConverter implements ConditionalGenericConverter {
private final SimpleTypeConverter typeConverter; private final Consumer<PropertyEditorRegistry> initializer;
// SimpleTypeConverter is not thread-safe to use for conversion but we can use it
// in a thread-safe way to check if conversion is possible.
private final SimpleTypeConverter matchesOnlyTypeConverter;
TypeConverterConverter(SimpleTypeConverter typeConverter) { TypeConverterConverter(Consumer<PropertyEditorRegistry> initializer) {
this.typeConverter = typeConverter; this.initializer = initializer;
this.matchesOnlyTypeConverter = createTypeConverter();
} }
@Override @Override
@ -226,32 +223,32 @@ final class BindConverter {
@Override @Override
public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) { public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) {
return getPropertyEditor(targetType.getType()) != null; Class<?> type = targetType.getType();
}
@Override
public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {
SimpleTypeConverter typeConverter = this.typeConverter;
return typeConverter.convertIfNecessary(source, targetType.getType());
}
private PropertyEditor getPropertyEditor(Class<?> type) {
if (type == null || type == Object.class || Collection.class.isAssignableFrom(type) if (type == null || type == Object.class || Collection.class.isAssignableFrom(type)
|| Map.class.isAssignableFrom(type)) { || Map.class.isAssignableFrom(type)) {
return null; return false;
} }
SimpleTypeConverter typeConverter = this.typeConverter; PropertyEditor editor = this.matchesOnlyTypeConverter.getDefaultEditor(type);
PropertyEditor editor = typeConverter.getDefaultEditor(type);
if (editor == null) { if (editor == null) {
editor = typeConverter.findCustomEditor(type, null); editor = this.matchesOnlyTypeConverter.findCustomEditor(type, null);
} }
if (editor == null && String.class != type) { if (editor == null && String.class != type) {
editor = BeanUtils.findEditorByConvention(type); editor = BeanUtils.findEditorByConvention(type);
} }
if (editor == null || EXCLUDED_EDITORS.contains(editor.getClass())) { return (editor != null && !EXCLUDED_EDITORS.contains(editor.getClass()));
return null;
} }
return editor;
@Override
public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {
return createTypeConverter().convertIfNecessary(source, targetType.getType());
}
private SimpleTypeConverter createTypeConverter() {
SimpleTypeConverter typeConverter = new SimpleTypeConverter();
if (this.initializer != null) {
this.initializer.accept(typeConverter);
}
return typeConverter;
} }
} }

@ -19,6 +19,8 @@ package org.springframework.boot.context.properties.bind;
import java.beans.PropertyEditorSupport; import java.beans.PropertyEditorSupport;
import java.io.File; import java.io.File;
import java.time.Duration; import java.time.Duration;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List; import java.util.List;
import java.util.function.Consumer; import java.util.function.Consumer;
@ -192,6 +194,30 @@ class BindConverterTests {
.withRootCauseInstanceOf(ClassNotFoundException.class); .withRootCauseInstanceOf(ClassNotFoundException.class);
} }
@Test
void convertWhenUsingTypeConverterConversionServiceFromMultipleThreads() {
BindConverter bindConverter = getPropertyEditorOnlyBindConverter(this::registerSampleTypeEditor);
ResolvableType type = ResolvableType.forClass(SampleType.class);
List<Thread> threads = new ArrayList<>();
List<SampleType> results = Collections.synchronizedList(new ArrayList<>());
for (int i = 0; i < 40; i++) {
threads.add(new Thread(() -> {
for (int j = 0; j < 20; j++) {
results.add(bindConverter.convert("test", type));
}
}));
}
threads.forEach(Thread::start);
for (Thread thread : threads) {
try {
thread.join();
}
catch (InterruptedException ex) {
}
}
assertThat(results).isNotEmpty().doesNotContainNull();
}
private BindConverter getPropertyEditorOnlyBindConverter( private BindConverter getPropertyEditorOnlyBindConverter(
Consumer<PropertyEditorRegistry> propertyEditorInitializer) { Consumer<PropertyEditorRegistry> propertyEditorInitializer) {
return BindConverter.get(new ThrowingConversionService(), propertyEditorInitializer); return BindConverter.get(new ThrowingConversionService(), propertyEditorInitializer);
@ -221,10 +247,13 @@ class BindConverterTests {
@Override @Override
public void setAsText(String text) throws IllegalArgumentException { public void setAsText(String text) throws IllegalArgumentException {
setValue(null);
if (text != null) {
SampleType value = new SampleType(); SampleType value = new SampleType();
value.text = text; value.text = text;
setValue(value); setValue(value);
} }
}
} }

Loading…
Cancel
Save