Skip to content

Commit 87dbda2

Browse files
committed
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
1 parent 69be1c8 commit 87dbda2

File tree

2 files changed

+58
-32
lines changed

2 files changed

+58
-32
lines changed

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/BindConverter.java

Lines changed: 26 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -185,18 +185,10 @@ public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor t
185185
private static class TypeConverterConversionService extends GenericConversionService {
186186

187187
TypeConverterConversionService(Consumer<PropertyEditorRegistry> initializer) {
188-
addConverter(new TypeConverterConverter(createTypeConverter(initializer)));
188+
addConverter(new TypeConverterConverter(initializer));
189189
ApplicationConversionService.addDelimitedStringConverters(this);
190190
}
191191

192-
private SimpleTypeConverter createTypeConverter(Consumer<PropertyEditorRegistry> initializer) {
193-
SimpleTypeConverter typeConverter = new SimpleTypeConverter();
194-
if (initializer != null) {
195-
initializer.accept(typeConverter);
196-
}
197-
return typeConverter;
198-
}
199-
200192
@Override
201193
public boolean canConvert(TypeDescriptor sourceType, TypeDescriptor targetType) {
202194
// Prefer conversion service to handle things like String to char[].
@@ -213,10 +205,15 @@ public boolean canConvert(TypeDescriptor sourceType, TypeDescriptor targetType)
213205
*/
214206
private static class TypeConverterConverter implements ConditionalGenericConverter {
215207

216-
private final SimpleTypeConverter typeConverter;
208+
private final Consumer<PropertyEditorRegistry> initializer;
209+
210+
// SimpleTypeConverter is not thread-safe to use for conversion but we can use it
211+
// in a thread-safe way to check if conversion is possible.
212+
private final SimpleTypeConverter matchesOnlyTypeConverter;
217213

218-
TypeConverterConverter(SimpleTypeConverter typeConverter) {
219-
this.typeConverter = typeConverter;
214+
TypeConverterConverter(Consumer<PropertyEditorRegistry> initializer) {
215+
this.initializer = initializer;
216+
this.matchesOnlyTypeConverter = createTypeConverter();
220217
}
221218

222219
@Override
@@ -226,32 +223,32 @@ public Set<ConvertiblePair> getConvertibleTypes() {
226223

227224
@Override
228225
public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) {
229-
return getPropertyEditor(targetType.getType()) != null;
230-
}
231-
232-
@Override
233-
public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {
234-
SimpleTypeConverter typeConverter = this.typeConverter;
235-
return typeConverter.convertIfNecessary(source, targetType.getType());
236-
}
237-
238-
private PropertyEditor getPropertyEditor(Class<?> type) {
226+
Class<?> type = targetType.getType();
239227
if (type == null || type == Object.class || Collection.class.isAssignableFrom(type)
240228
|| Map.class.isAssignableFrom(type)) {
241-
return null;
229+
return false;
242230
}
243-
SimpleTypeConverter typeConverter = this.typeConverter;
244-
PropertyEditor editor = typeConverter.getDefaultEditor(type);
231+
PropertyEditor editor = this.matchesOnlyTypeConverter.getDefaultEditor(type);
245232
if (editor == null) {
246-
editor = typeConverter.findCustomEditor(type, null);
233+
editor = this.matchesOnlyTypeConverter.findCustomEditor(type, null);
247234
}
248235
if (editor == null && String.class != type) {
249236
editor = BeanUtils.findEditorByConvention(type);
250237
}
251-
if (editor == null || EXCLUDED_EDITORS.contains(editor.getClass())) {
252-
return null;
238+
return (editor != null && !EXCLUDED_EDITORS.contains(editor.getClass()));
239+
}
240+
241+
@Override
242+
public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {
243+
return createTypeConverter().convertIfNecessary(source, targetType.getType());
244+
}
245+
246+
private SimpleTypeConverter createTypeConverter() {
247+
SimpleTypeConverter typeConverter = new SimpleTypeConverter();
248+
if (this.initializer != null) {
249+
this.initializer.accept(typeConverter);
253250
}
254-
return editor;
251+
return typeConverter;
255252
}
256253

257254
}

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/BindConverterTests.java

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
import java.beans.PropertyEditorSupport;
2020
import java.io.File;
2121
import java.time.Duration;
22+
import java.util.ArrayList;
23+
import java.util.Collections;
2224
import java.util.List;
2325
import java.util.function.Consumer;
2426

@@ -192,6 +194,30 @@ void convertWhenConversionFailsThrowsConversionFailedExceptionRatherThanConverte
192194
.withRootCauseInstanceOf(ClassNotFoundException.class);
193195
}
194196

197+
@Test
198+
void convertWhenUsingTypeConverterConversionServiceFromMultipleThreads() {
199+
BindConverter bindConverter = getPropertyEditorOnlyBindConverter(this::registerSampleTypeEditor);
200+
ResolvableType type = ResolvableType.forClass(SampleType.class);
201+
List<Thread> threads = new ArrayList<>();
202+
List<SampleType> results = Collections.synchronizedList(new ArrayList<>());
203+
for (int i = 0; i < 40; i++) {
204+
threads.add(new Thread(() -> {
205+
for (int j = 0; j < 20; j++) {
206+
results.add(bindConverter.convert("test", type));
207+
}
208+
}));
209+
}
210+
threads.forEach(Thread::start);
211+
for (Thread thread : threads) {
212+
try {
213+
thread.join();
214+
}
215+
catch (InterruptedException ex) {
216+
}
217+
}
218+
assertThat(results).isNotEmpty().doesNotContainNull();
219+
}
220+
195221
private BindConverter getPropertyEditorOnlyBindConverter(
196222
Consumer<PropertyEditorRegistry> propertyEditorInitializer) {
197223
return BindConverter.get(new ThrowingConversionService(), propertyEditorInitializer);
@@ -221,9 +247,12 @@ static class SampleTypePropertyEditor extends PropertyEditorSupport {
221247

222248
@Override
223249
public void setAsText(String text) throws IllegalArgumentException {
224-
SampleType value = new SampleType();
225-
value.text = text;
226-
setValue(value);
250+
setValue(null);
251+
if (text != null) {
252+
SampleType value = new SampleType();
253+
value.text = text;
254+
setValue(value);
255+
}
227256
}
228257

229258
}

0 commit comments

Comments
 (0)