From 1756f837013a9fe4384ff75b364ae5705f831e01 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Tue, 18 Sep 2018 21:25:36 +0200 Subject: [PATCH] Defensively expect concurrent registration of BeanPostProcessors Declaring beanPostProcessors (and also embeddedValueResolvers) as CopyOnWriteArrayList prevents ConcurrentModificationExceptions in case of concurrent registration/access attempts. Issue: SPR-17286 --- .../factory/support/AbstractBeanFactory.java | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java index 2208b8c4f14..390e6cbd549 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java @@ -29,11 +29,11 @@ import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.LinkedHashSet; -import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CopyOnWriteArrayList; import org.springframework.beans.BeanUtils; import org.springframework.beans.BeanWrapper; @@ -145,16 +145,16 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp private TypeConverter typeConverter; /** String resolvers to apply e.g. to annotation attribute values. */ - private final List embeddedValueResolvers = new LinkedList<>(); + private final List embeddedValueResolvers = new CopyOnWriteArrayList<>(); /** BeanPostProcessors to apply in createBean. */ - private final List beanPostProcessors = new ArrayList<>(); + private final List beanPostProcessors = new CopyOnWriteArrayList<>(); /** Indicates whether any InstantiationAwareBeanPostProcessors have been registered. */ - private boolean hasInstantiationAwareBeanPostProcessors; + private volatile boolean hasInstantiationAwareBeanPostProcessors; /** Indicates whether any DestructionAwareBeanPostProcessors have been registered. */ - private boolean hasDestructionAwareBeanPostProcessors; + private volatile boolean hasDestructionAwareBeanPostProcessors; /** Map from scope identifier String to corresponding Scope. */ private final Map scopes = new LinkedHashMap<>(8); @@ -850,14 +850,17 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp @Override public void addBeanPostProcessor(BeanPostProcessor beanPostProcessor) { Assert.notNull(beanPostProcessor, "BeanPostProcessor must not be null"); + // Remove from old position, if any this.beanPostProcessors.remove(beanPostProcessor); - this.beanPostProcessors.add(beanPostProcessor); + // Track whether it is instantiation/destruction aware if (beanPostProcessor instanceof InstantiationAwareBeanPostProcessor) { this.hasInstantiationAwareBeanPostProcessors = true; } if (beanPostProcessor instanceof DestructionAwareBeanPostProcessor) { this.hasDestructionAwareBeanPostProcessors = true; } + // Add to end of list + this.beanPostProcessors.add(beanPostProcessor); } @Override @@ -988,7 +991,6 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp @Override public BeanDefinition getMergedBeanDefinition(String name) throws BeansException { String beanName = transformedBeanName(name); - // Efficiently check whether bean definition exists in this factory. if (!containsBeanDefinition(beanName) && getParentBeanFactory() instanceof ConfigurableBeanFactory) { return ((ConfigurableBeanFactory) getParentBeanFactory()).getMergedBeanDefinition(beanName); @@ -1000,18 +1002,15 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp @Override public boolean isFactoryBean(String name) throws NoSuchBeanDefinitionException { String beanName = transformedBeanName(name); - Object beanInstance = getSingleton(beanName, false); if (beanInstance != null) { return (beanInstance instanceof FactoryBean); } - // No singleton instance found -> check bean definition. if (!containsBeanDefinition(beanName) && getParentBeanFactory() instanceof ConfigurableBeanFactory) { // No bean definition found in this factory -> delegate to parent. return ((ConfigurableBeanFactory) getParentBeanFactory()).isFactoryBean(name); } - return isFactoryBean(beanName, getMergedLocalBeanDefinition(beanName)); }