Skip to content

Commit

Permalink
Fix regressions in SimpleThreadScope and SimpleTransactionScope
Browse files Browse the repository at this point in the history
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
  • Loading branch information
sbrannen committed Aug 22, 2020
1 parent d939016 commit 148dc95
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 46 deletions.
@@ -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");
* you may not use this file except in compliance with the License.
Expand All @@ -16,8 +16,8 @@

package org.springframework.context.support;

import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
Expand Down Expand Up @@ -59,7 +59,7 @@ public class SimpleThreadScope implements Scope {
new NamedThreadLocal<Map<String, Object>>("SimpleThreadScope") {
@Override
protected Map<String, Object> initialValue() {
return new HashMap<>();
return new ConcurrentHashMap<>();
}
};

Expand Down
@@ -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");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -30,14 +30,14 @@
* @author Arjen Poutsma
* @author Sam Brannen
*/
public class SimpleThreadScopeTests {
class SimpleThreadScopeTests {

private final ApplicationContext applicationContext =
new ClassPathXmlApplicationContext("simpleThreadScopeTests.xml", getClass());


@Test
public void getFromScope() throws Exception {
void getFromScope() throws Exception {
String name = "threadScopedObject";
TestBean bean = this.applicationContext.getBean(name, TestBean.class);
assertThat(bean).isNotNull();
Expand All @@ -47,7 +47,7 @@ public void getFromScope() throws Exception {
}

@Test
public void getMultipleInstances() throws Exception {
void getMultipleInstances() throws Exception {
// Arrange
TestBean[] beans = new TestBean[2];
Thread thread1 = new Thread(() -> beans[0] = applicationContext.getBean("threadScopedObject", TestBean.class));
Expand Down
@@ -1,51 +1,21 @@
<?xml version="1.0" encoding="UTF-8"?>
<beans xmlns="http://www.springframework.org/schema/beans"
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-2.5.xsd">
<beans xmlns="http://www.springframework.org/schema/beans" 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">

<bean class="org.springframework.beans.factory.config.CustomScopeConfigurer">
<property name="scopes">
<map>
<entry key="thread">
<bean class="org.springframework.context.support.SimpleThreadScope"/>
<bean class="org.springframework.context.support.SimpleThreadScope" />
</entry>
</map>
</property>
</bean>

<bean id="threadScopedObject" class="org.springframework.beans.testfixture.beans.TestBean" scope="thread"/>

<!--
<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 id="threadScopedObject" class="org.springframework.beans.testfixture.beans.TestBean" scope="thread">
<property name="spouse" ref="threadScopedObject2" />
</bean>

<bean id="requestScopedObjectCircle2" class="org.springframework.beans.testfixture.beans.TestBean" scope="request">
<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>
<bean id="threadScopedObject2" class="org.springframework.beans.testfixture.beans.TestBean" scope="thread" />

-->
</beans>
@@ -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 @@ -16,9 +16,9 @@

package org.springframework.transaction.support;

import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

import org.springframework.beans.factory.ObjectFactory;
import org.springframework.beans.factory.config.Scope;
Expand Down Expand Up @@ -92,7 +92,7 @@ public String getConversationId() {
*/
static class ScopedObjectsHolder {

final Map<String, Object> scopedInstances = new HashMap<>();
final Map<String, Object> scopedInstances = new ConcurrentHashMap<>();

final Map<String, Runnable> destructionCallbacks = new LinkedHashMap<>();
}
Expand Down

0 comments on commit 148dc95

Please sign in to comment.