From 8d5587fe4f938c51b68dffa3370840b2bda19d54 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Sat, 24 Mar 2018 16:30:51 +0100 Subject: [PATCH] Consistent thread-safe iteration in DefaultSingletonBeanRegistry Issue: SPR-16620 --- .../support/DefaultSingletonBeanRegistry.java | 52 +++++++++++-------- .../support/DefaultLifecycleProcessor.java | 2 +- 2 files changed, 31 insertions(+), 23 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultSingletonBeanRegistry.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultSingletonBeanRegistry.java index b66e24b0ad..8f3b6f8782 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultSingletonBeanRegistry.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultSingletonBeanRegistry.java @@ -384,16 +384,12 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements * @see #registerDependentBean */ public void registerContainedBean(String containedBeanName, String containingBeanName) { - // A quick check for an existing entry upfront, avoiding synchronization... - Set containedBeans = this.containedBeanMap.get(containingBeanName); - if (containedBeans != null && containedBeans.contains(containedBeanName)) { - return; - } - - // No entry yet -> fully synchronized manipulation of the containedBeans Set synchronized (this.containedBeanMap) { - containedBeans = this.containedBeanMap.computeIfAbsent(containingBeanName, k -> new LinkedHashSet<>(8)); - containedBeans.add(containedBeanName); + Set containedBeans = + this.containedBeanMap.computeIfAbsent(containingBeanName, k -> new LinkedHashSet<>(8)); + if (!containedBeans.add(containedBeanName)) { + return; + } } registerDependentBean(containedBeanName, containingBeanName); } @@ -405,18 +401,16 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements * @param dependentBeanName the name of the dependent bean */ public void registerDependentBean(String beanName, String dependentBeanName) { - // A quick check for an existing entry upfront, avoiding synchronization... String canonicalName = canonicalName(beanName); - Set dependentBeans = this.dependentBeanMap.get(canonicalName); - if (dependentBeans != null && dependentBeans.contains(dependentBeanName)) { - return; - } - // No entry yet -> fully synchronized manipulation of the dependentBeans Set synchronized (this.dependentBeanMap) { - dependentBeans = this.dependentBeanMap.computeIfAbsent(canonicalName, k -> new LinkedHashSet<>(8)); - dependentBeans.add(dependentBeanName); + Set dependentBeans = + this.dependentBeanMap.computeIfAbsent(canonicalName, k -> new LinkedHashSet<>(8)); + if (!dependentBeans.add(dependentBeanName)) { + return; + } } + synchronized (this.dependenciesForBeanMap) { Set dependenciesForBean = this.dependenciesForBeanMap.computeIfAbsent(dependentBeanName, k -> new LinkedHashSet<>(8)); @@ -432,7 +426,9 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements * @since 4.0 */ protected boolean isDependent(String beanName, String dependentBeanName) { - return isDependent(beanName, dependentBeanName, null); + synchronized (this.dependentBeanMap) { + return isDependent(beanName, dependentBeanName, null); + } } private boolean isDependent(String beanName, String dependentBeanName, @Nullable Set alreadySeen) { @@ -477,7 +473,9 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements if (dependentBeans == null) { return new String[0]; } - return StringUtils.toStringArray(dependentBeans); + synchronized (this.dependentBeanMap) { + return StringUtils.toStringArray(dependentBeans); + } } /** @@ -491,7 +489,9 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements if (dependenciesForBean == null) { return new String[0]; } - return StringUtils.toStringArray(dependenciesForBean); + synchronized (this.dependenciesForBeanMap) { + return StringUtils.toStringArray(dependenciesForBean); + } } public void destroySingletons() { @@ -557,7 +557,11 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements */ protected void destroyBean(String beanName, @Nullable DisposableBean bean) { // Trigger destruction of dependent beans first... - Set dependencies = this.dependentBeanMap.remove(beanName); + Set dependencies; + synchronized (this.dependentBeanMap) { + // Within full synchronization in order to guarantee a disconnected Set + dependencies = this.dependentBeanMap.remove(beanName); + } if (dependencies != null) { if (logger.isDebugEnabled()) { logger.debug("Retrieved dependent beans for bean '" + beanName + "': " + dependencies); @@ -578,7 +582,11 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements } // Trigger destruction of contained beans... - Set containedBeans = this.containedBeanMap.remove(beanName); + Set containedBeans; + synchronized (this.containedBeanMap) { + // Within full synchronization in order to guarantee a disconnected Set + containedBeans = this.containedBeanMap.remove(beanName); + } if (containedBeans != null) { for (String containedBeanName : containedBeans) { destroySingleton(containedBeanName); diff --git a/spring-context/src/main/java/org/springframework/context/support/DefaultLifecycleProcessor.java b/spring-context/src/main/java/org/springframework/context/support/DefaultLifecycleProcessor.java index 38713f4e24..bfb7fa8bbf 100644 --- a/spring-context/src/main/java/org/springframework/context/support/DefaultLifecycleProcessor.java +++ b/spring-context/src/main/java/org/springframework/context/support/DefaultLifecycleProcessor.java @@ -167,7 +167,7 @@ public class DefaultLifecycleProcessor implements LifecycleProcessor, BeanFactor */ private void doStart(Map lifecycleBeans, String beanName, boolean autoStartupOnly) { Lifecycle bean = lifecycleBeans.remove(beanName); - if (bean != null && !this.equals(bean)) { + if (bean != null && bean != this) { String[] dependenciesForBean = getBeanFactory().getDependenciesForBean(beanName); for (String dependency : dependenciesForBean) { doStart(lifecycleBeans, dependency, autoStartupOnly); -- GitLab