From 148dc95eb110c4d97829ee3d60c78fb683b9db79 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Sat, 22 Aug 2020 23:19:58 +0200 Subject: [PATCH] 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 --- .../context/support/SimpleThreadScope.java | 6 +-- .../support/SimpleThreadScopeTests.java | 8 ++-- .../support/simpleThreadScopeTests.xml | 42 +++---------------- .../support/SimpleTransactionScope.java | 6 +-- 4 files changed, 16 insertions(+), 46 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/context/support/SimpleThreadScope.java b/spring-context/src/main/java/org/springframework/context/support/SimpleThreadScope.java index 7b469df0ccce..900282e45873 100644 --- a/spring-context/src/main/java/org/springframework/context/support/SimpleThreadScope.java +++ b/spring-context/src/main/java/org/springframework/context/support/SimpleThreadScope.java @@ -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. @@ -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; @@ -59,7 +59,7 @@ public class SimpleThreadScope implements Scope { new NamedThreadLocal>("SimpleThreadScope") { @Override protected Map initialValue() { - return new HashMap<>(); + return new ConcurrentHashMap<>(); } }; diff --git a/spring-context/src/test/java/org/springframework/context/support/SimpleThreadScopeTests.java b/spring-context/src/test/java/org/springframework/context/support/SimpleThreadScopeTests.java index 95f4e0d98b30..93916a671f97 100644 --- a/spring-context/src/test/java/org/springframework/context/support/SimpleThreadScopeTests.java +++ b/spring-context/src/test/java/org/springframework/context/support/SimpleThreadScopeTests.java @@ -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. @@ -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(); @@ -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)); diff --git a/spring-context/src/test/resources/org/springframework/context/support/simpleThreadScopeTests.xml b/spring-context/src/test/resources/org/springframework/context/support/simpleThreadScopeTests.xml index a8200730794d..2a7527aed983 100644 --- a/spring-context/src/test/resources/org/springframework/context/support/simpleThreadScopeTests.xml +++ b/spring-context/src/test/resources/org/springframework/context/support/simpleThreadScopeTests.xml @@ -1,51 +1,21 @@ - + - + - - - diff --git a/spring-tx/src/main/java/org/springframework/transaction/support/SimpleTransactionScope.java b/spring-tx/src/main/java/org/springframework/transaction/support/SimpleTransactionScope.java index 38c93de5b35f..272ac19524a3 100644 --- a/spring-tx/src/main/java/org/springframework/transaction/support/SimpleTransactionScope.java +++ b/spring-tx/src/main/java/org/springframework/transaction/support/SimpleTransactionScope.java @@ -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. @@ -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; @@ -92,7 +92,7 @@ public String getConversationId() { */ static class ScopedObjectsHolder { - final Map scopedInstances = new HashMap<>(); + final Map scopedInstances = new ConcurrentHashMap<>(); final Map destructionCallbacks = new LinkedHashMap<>(); }