Fix regressions in SimpleThreadScope and SimpleTransactionScope

PR gh-25038 introduced regressions in SimpleThreadScope and
SimpleTransactionScope in Spring Framework 5.2.7. Specifically, if a
thread-scoped or transaction-scoped bean has a dependency on another
thread-scoped or transaction-scoped bean, respectively, a
ConcurrentModificationException will be thrown on Java 11 or higher.

The reason is that Java 11 introduced a check for concurrent
modification in java.util.HashMap's computeIfAbsent() implementation,
and such a modification can occur when a thread-scoped bean is being
created in order to satisfy a dependency of another thread-scoped bean
that is currently being created.

This commit fixes these regressions by switching from HashMap to
ConcurrentHashMap for the instance maps in SimpleThreadScope and
SimpleTransactionScope.

Closes gh-25618
This commit is contained in:
Sam Brannen 2020-08-22 23:19:58 +02:00
parent d939016a09
commit 148dc95eb1
4 changed files with 16 additions and 46 deletions

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2019 the original author or authors. * Copyright 2002-2020 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -16,8 +16,8 @@
package org.springframework.context.support; package org.springframework.context.support;
import java.util.HashMap;
import java.util.Map; import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import org.apache.commons.logging.Log; import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory; import org.apache.commons.logging.LogFactory;
@ -59,7 +59,7 @@ public class SimpleThreadScope implements Scope {
new NamedThreadLocal<Map<String, Object>>("SimpleThreadScope") { new NamedThreadLocal<Map<String, Object>>("SimpleThreadScope") {
@Override @Override
protected Map<String, Object> initialValue() { protected Map<String, Object> initialValue() {
return new HashMap<>(); return new ConcurrentHashMap<>();
} }
}; };

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2019 the original author or authors. * Copyright 2002-2020 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -30,14 +30,14 @@ import static org.assertj.core.api.Assertions.assertThat;
* @author Arjen Poutsma * @author Arjen Poutsma
* @author Sam Brannen * @author Sam Brannen
*/ */
public class SimpleThreadScopeTests { class SimpleThreadScopeTests {
private final ApplicationContext applicationContext = private final ApplicationContext applicationContext =
new ClassPathXmlApplicationContext("simpleThreadScopeTests.xml", getClass()); new ClassPathXmlApplicationContext("simpleThreadScopeTests.xml", getClass());
@Test @Test
public void getFromScope() throws Exception { void getFromScope() throws Exception {
String name = "threadScopedObject"; String name = "threadScopedObject";
TestBean bean = this.applicationContext.getBean(name, TestBean.class); TestBean bean = this.applicationContext.getBean(name, TestBean.class);
assertThat(bean).isNotNull(); assertThat(bean).isNotNull();
@ -47,7 +47,7 @@ public class SimpleThreadScopeTests {
} }
@Test @Test
public void getMultipleInstances() throws Exception { void getMultipleInstances() throws Exception {
// Arrange // Arrange
TestBean[] beans = new TestBean[2]; TestBean[] beans = new TestBean[2];
Thread thread1 = new Thread(() -> beans[0] = applicationContext.getBean("threadScopedObject", TestBean.class)); Thread thread1 = new Thread(() -> beans[0] = applicationContext.getBean("threadScopedObject", TestBean.class));

View File

@ -1,51 +1,21 @@
<?xml version="1.0" encoding="UTF-8"?> <?xml version="1.0" encoding="UTF-8"?>
<beans xmlns="http://www.springframework.org/schema/beans" <beans xmlns="http://www.springframework.org/schema/beans" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.springframework.org/schema/beans https://www.springframework.org/schema/beans/spring-beans.xsd">
xsi:schemaLocation="http://www.springframework.org/schema/beans
https://www.springframework.org/schema/beans/spring-beans-2.5.xsd">
<bean class="org.springframework.beans.factory.config.CustomScopeConfigurer"> <bean class="org.springframework.beans.factory.config.CustomScopeConfigurer">
<property name="scopes"> <property name="scopes">
<map> <map>
<entry key="thread"> <entry key="thread">
<bean class="org.springframework.context.support.SimpleThreadScope"/> <bean class="org.springframework.context.support.SimpleThreadScope" />
</entry> </entry>
</map> </map>
</property> </property>
</bean> </bean>
<bean id="threadScopedObject" class="org.springframework.beans.testfixture.beans.TestBean" scope="thread"/> <bean id="threadScopedObject" class="org.springframework.beans.testfixture.beans.TestBean" scope="thread">
<property name="spouse" ref="threadScopedObject2" />
<!--
<bean id="requestScopedDisposableObject" class="org.springframework.beans.testfixture.beans.DerivedTestBean" scope="request"/>
<bean id="requestScopedFactoryBean" class="org.springframework.beans.testfixture.beans.factory.DummyFactory" scope="request"/>
<bean id="requestScopedObjectCircle1" class="org.springframework.beans.testfixture.beans.TestBean" scope="request">
<property name="spouse" ref="requestScopedObjectCircle2"/>
</bean> </bean>
<bean id="requestScopedObjectCircle2" class="org.springframework.beans.testfixture.beans.TestBean" scope="request"> <bean id="threadScopedObject2" class="org.springframework.beans.testfixture.beans.TestBean" scope="thread" />
<property name="spouse" ref="requestScopedObjectCircle1"/>
</bean>
<bean id="requestScopedOuterBean" class="org.springframework.beans.testfixture.beans.DerivedTestBean" scope="request">
<property name="name" value="outer"/>
<property name="spouse">
<bean class="org.springframework.beans.testfixture.beans.DerivedTestBean">
<property name="name" value="inner"/>
</bean>
</property>
</bean>
<bean id="singletonOuterBean" class="org.springframework.beans.testfixture.beans.DerivedTestBean" lazy-init="true">
<property name="name" value="outer"/>
<property name="spouse">
<bean class="org.springframework.beans.testfixture.beans.DerivedTestBean" scope="request">
<property name="name" value="inner"/>
</bean>
</property>
</bean>
-->
</beans> </beans>

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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -16,9 +16,9 @@
package org.springframework.transaction.support; package org.springframework.transaction.support;
import java.util.HashMap;
import java.util.LinkedHashMap; import java.util.LinkedHashMap;
import java.util.Map; import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import org.springframework.beans.factory.ObjectFactory; import org.springframework.beans.factory.ObjectFactory;
import org.springframework.beans.factory.config.Scope; import org.springframework.beans.factory.config.Scope;
@ -92,7 +92,7 @@ public class SimpleTransactionScope implements Scope {
*/ */
static class ScopedObjectsHolder { static class ScopedObjectsHolder {
final Map<String, Object> scopedInstances = new HashMap<>(); final Map<String, Object> scopedInstances = new ConcurrentHashMap<>();
final Map<String, Runnable> destructionCallbacks = new LinkedHashMap<>(); final Map<String, Runnable> destructionCallbacks = new LinkedHashMap<>();
} }