DefaultListableBeanFactory switches to thread-safe copying for names collections if necessary

Issue: SPR-13493
Issue: SPR-13123
Issue: SPR-12503

Author: jhoeller

spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java

@@ -1535,6 +1535,16 @@ protected boolean removeSingletonIfCreatedForTypeCheckOnly(String beanName) {
 		}
 	}
 
+	/**
+	 * Check whether this factory's bean creation phase already started,
+	 * i.e. whether any bean has been marked as created in the meantime.
+	 * @since 4.2.2
+	 * @see #markBeanAsCreated
+	 */
+	protected boolean hasBeanCreationStarted() {
+		return !this.alreadyCreated.isEmpty();
+	}
+
 	/**
 	 * Get the object for the given bean instance, either the bean
 	 * instance itself or its created object in case of a FactoryBean.

spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java

@@ -155,7 +155,7 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto
 	private AutowireCandidateResolver autowireCandidateResolver = new SimpleAutowireCandidateResolver();
 
 	/** Map from dependency type to corresponding autowired value */
-	private final Map<Class<?>, Object> resolvableDependencies = new HashMap<Class<?>, Object>(16);
+	private final Map<Class<?>, Object> resolvableDependencies = new ConcurrentHashMap<Class<?>, Object>(16);
 
 	/** Map of bean definition objects, keyed by bean name */
 	private final Map<String, BeanDefinition> beanDefinitionMap = new ConcurrentHashMap<String, BeanDefinition>(64);
@@ -167,16 +167,16 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto
 	private final Map<Class<?>, String[]> singletonBeanNamesByType = new ConcurrentHashMap<Class<?>, String[]>(64);
 
 	/** List of bean definition names, in registration order */
-	private final List<String> beanDefinitionNames = new ArrayList<String>(64);
+	private volatile List<String> beanDefinitionNames = new ArrayList<String>(64);
 
 	/** List of names of manually registered singletons, in registration order */
-	private final Set<String> manualSingletonNames = new LinkedHashSet<String>(16);
-
-	/** Whether bean definition metadata may be cached for all beans */
-	private boolean configurationFrozen = false;
+	private volatile Set<String> manualSingletonNames = new LinkedHashSet<String>(16);
 
 	/** Cached array of bean definition names in case of frozen configuration */
-	private String[] frozenBeanDefinitionNames;
+	private volatile String[] frozenBeanDefinitionNames;
+
+	/** Whether bean definition metadata may be cached for all beans */
+	private volatile boolean configurationFrozen = false;
 
 
 	/**
@@ -848,13 +848,32 @@ else if (!beanDefinition.equals(oldBeanDefinition)) {
 							"] with [" + beanDefinition + "]");
 				}
 			}
+			this.beanDefinitionMap.put(beanName, beanDefinition);
 		}
 		else {
-			this.beanDefinitionNames.add(beanName);
-			this.manualSingletonNames.remove(beanName);
+			if (hasBeanCreationStarted()) {
+				// Cannot modify startup-time collection elements anymore (for stable iteration)
+				synchronized (this.beanDefinitionMap) {
+					this.beanDefinitionMap.put(beanName, beanDefinition);
+					List<String> updatedDefinitions = new ArrayList<String>(this.beanDefinitionNames.size() + 1);
+					updatedDefinitions.addAll(this.beanDefinitionNames);
+					updatedDefinitions.add(beanName);
+					this.beanDefinitionNames = updatedDefinitions;
+					if (this.manualSingletonNames.contains(beanName)) {
+						Set<String> updatedSingletons = new LinkedHashSet<String>(this.manualSingletonNames);
+						updatedSingletons.remove(beanName);
+						this.manualSingletonNames = updatedSingletons;
+					}
+				}
+			}
+			else {
+				// Still in startup registration phase
+				this.beanDefinitionMap.put(beanName, beanDefinition);
+				this.beanDefinitionNames.add(beanName);
+				this.manualSingletonNames.remove(beanName);
+			}
 			this.frozenBeanDefinitionNames = null;
 		}
-		this.beanDefinitionMap.put(beanName, beanDefinition);
 
 		if (oldBeanDefinition != null || containsSingleton(beanName)) {
 			resetBeanDefinition(beanName);
@@ -872,7 +891,19 @@ public void removeBeanDefinition(String beanName) throws NoSuchBeanDefinitionExc
 			}
 			throw new NoSuchBeanDefinitionException(beanName);
 		}
-		this.beanDefinitionNames.remove(beanName);
+
+		if (hasBeanCreationStarted()) {
+			// Cannot modify startup-time collection elements anymore (for stable iteration)
+			synchronized (this.beanDefinitionMap) {
+				List<String> updatedDefinitions = new ArrayList<String>(this.beanDefinitionNames);
+				updatedDefinitions.remove(beanName);
+				this.beanDefinitionNames = updatedDefinitions;
+			}
+		}
+		else {
+			// Still in startup registration phase
+			this.beanDefinitionNames.remove(beanName);
+		}
 		this.frozenBeanDefinitionNames = null;
 
 		resetBeanDefinition(beanName);
@@ -914,9 +945,25 @@ protected boolean allowAliasOverriding() {
 	@Override
 	public void registerSingleton(String beanName, Object singletonObject) throws IllegalStateException {
 		super.registerSingleton(beanName, singletonObject);
-		if (!this.beanDefinitionMap.containsKey(beanName)) {
-			this.manualSingletonNames.add(beanName);
+
+		if (hasBeanCreationStarted()) {
+			// Cannot modify startup-time collection elements anymore (for stable iteration)
+			synchronized (this.beanDefinitionMap) {
+				if (!this.beanDefinitionMap.containsKey(beanName)) {
+					Set<String> updatedSingletons = new LinkedHashSet<String>(this.manualSingletonNames.size() + 1);
+					updatedSingletons.addAll(this.manualSingletonNames);
+					updatedSingletons.add(beanName);
+					this.manualSingletonNames = updatedSingletons;
+				}
+			}
 		}
+		else {
+			// Still in startup registration phase
+			if (!this.beanDefinitionMap.containsKey(beanName)) {
+				this.manualSingletonNames.add(beanName);
+			}
+		}
+
 		clearByTypeCache();
 	}
 

spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java

@@ -1224,7 +1224,7 @@ public void testExpressionInStringArray() {
 
 		RootBeanDefinition rbd = new RootBeanDefinition(PropertiesFactoryBean.class);
 		MutablePropertyValues pvs = new MutablePropertyValues();
-		pvs.add("locations", new String[] {"#{foo}"});
+		pvs.add("locations", new String[]{"#{foo}"});
 		rbd.setPropertyValues(pvs);
 		bf.registerBeanDefinition("myProperties", rbd);
 		Properties properties = (Properties) bf.getBean("myProperties");
@@ -2462,6 +2462,7 @@ public void testBeanPostProcessorWithWrappedObjectAndDisposableBean() {
 			public Object postProcessBeforeInitialization(Object bean, String beanName) {
 				return new TestBean();
 			}
+
 			@Override
 			public Object postProcessAfterInitialization(Object bean, String beanName) {
 				return bean;
@@ -2750,10 +2751,6 @@ public void resolveEmbeddedValue() throws Exception {
 		verify(r3, never()).resolveStringValue(isNull(String.class));
 	}
 
-
-	static class A { }
-	static class B { }
-
 	/**
 	 * Test that by-type bean lookup caching is working effectively by searching for a
 	 * bean of type B 10K times within a container having 1K additional beans of type A.
@@ -2764,23 +2761,52 @@ static class B { }
 	 * under the 1000 ms timeout, usually ~= 300ms. With caching removed and on the same
 	 * hardware the method will take ~13000 ms. See SPR-6870.
 	 */
-	@Test(timeout=1000)
+	@Test(timeout = 1000)
 	public void testByTypeLookupIsFastEnough() {
 		Assume.group(TestGroup.PERFORMANCE);
 		DefaultListableBeanFactory bf = new DefaultListableBeanFactory();
 
 		for (int i = 0; i < 1000; i++) {
-			bf.registerBeanDefinition("a"+i, new RootBeanDefinition(A.class));
+			bf.registerBeanDefinition("a" + i, new RootBeanDefinition(A.class));
 		}
 		bf.registerBeanDefinition("b", new RootBeanDefinition(B.class));
 
 		bf.freezeConfiguration();
 
-		for (int i=0; i<10000; i++) {
+		for (int i = 0; i < 10000; i++) {
 			bf.getBean(B.class);
 		}
 	}
 
+	@Test(timeout = 1000)
+	public void testRegistrationOfManyBeanDefinitionsIsFastEnough() {
+		// Assume.group(TestGroup.PERFORMANCE);
+		DefaultListableBeanFactory bf = new DefaultListableBeanFactory();
+		bf.registerBeanDefinition("b", new RootBeanDefinition(B.class));
+		// bf.getBean("b");
+
+		for (int i = 0; i < 100000; i++) {
+			bf.registerBeanDefinition("a" + i, new RootBeanDefinition(A.class));
+		}
+	}
+
+	@Test(timeout = 1000)
+	public void testRegistrationOfManySingletonsIsFastEnough() {
+		// Assume.group(TestGroup.PERFORMANCE);
+		DefaultListableBeanFactory bf = new DefaultListableBeanFactory();
+		bf.registerBeanDefinition("b", new RootBeanDefinition(B.class));
+		// bf.getBean("b");
+
+		for (int i = 0; i < 100000; i++) {
+			bf.registerSingleton("a" + i, new A());
+		}
+	}
+
+
+	static class A { }
+
+	static class B { }
+
 
 	public static class NoDependencies {