From 713eb3481566bd1232acedc490ae9cf268b66146 Mon Sep 17 00:00:00 2001 From: lmagyar Date: Wed, 21 Mar 2018 00:54:04 +0100 Subject: [PATCH] Refactor concurrency within org.springframework.beans.factory.support.DefaultSingletonBeanRegistry Issue: SPR-16620 --- .../support/DefaultSingletonBeanRegistry.java | 90 ++++++++++--------- 1 file changed, 49 insertions(+), 41 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 ee0296cfa656..9dea03139742 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 @@ -155,11 +155,14 @@ protected void addSingleton(String beanName, Object singletonObject) { */ protected void addSingletonFactory(String beanName, ObjectFactory singletonFactory) { Assert.notNull(singletonFactory, "Singleton factory must not be null"); - synchronized (this.singletonObjects) { - if (!this.singletonObjects.containsKey(beanName)) { - this.singletonFactories.put(beanName, singletonFactory); - this.earlySingletonObjects.remove(beanName); - this.registeredSingletons.add(beanName); + + if (!this.singletonObjects.containsKey(beanName)) { + synchronized (this.singletonObjects) { + if (!this.singletonObjects.containsKey(beanName)) { + this.singletonFactories.put(beanName, singletonFactory); + this.earlySingletonObjects.remove(beanName); + this.registeredSingletons.add(beanName); + } } } } @@ -390,15 +393,13 @@ public void registerContainedBean(String containedBeanName, String containingBea return; } - // No entry yet -> fully synchronized manipulation of the containedBeans Set - synchronized (this.containedBeanMap) { - containedBeans = this.containedBeanMap.get(containingBeanName); - if (containedBeans == null) { - containedBeans = new LinkedHashSet<>(8); - this.containedBeanMap.put(containingBeanName, containedBeans); - } - containedBeans.add(containedBeanName); + if(containedBeans == null) { + containedBeans = containedBeanMap.computeIfAbsent( + containingBeanName, (key) -> Collections.synchronizedSet(new LinkedHashSet(8)) + ); } + + containedBeans.add(containedBeanName); registerDependentBean(containedBeanName, containingBeanName); } @@ -416,23 +417,20 @@ public void registerDependentBean(String beanName, String dependentBeanName) { return; } - // No entry yet -> fully synchronized manipulation of the dependentBeans Set - synchronized (this.dependentBeanMap) { - dependentBeans = this.dependentBeanMap.get(canonicalName); - if (dependentBeans == null) { - dependentBeans = new LinkedHashSet<>(8); - this.dependentBeanMap.put(canonicalName, dependentBeans); - } - dependentBeans.add(dependentBeanName); - } - synchronized (this.dependenciesForBeanMap) { - Set dependenciesForBean = this.dependenciesForBeanMap.get(dependentBeanName); - if (dependenciesForBean == null) { - dependenciesForBean = new LinkedHashSet<>(8); - this.dependenciesForBeanMap.put(dependentBeanName, dependenciesForBean); + if(dependentBeans == null) { + //need synchronization to get consistent view on entrySet + synchronized (this.dependentBeanMap) { + dependentBeans = this.dependentBeanMap.computeIfAbsent(canonicalName, + (key) -> Collections.synchronizedSet(new LinkedHashSet<>(8)) + ); } - dependenciesForBean.add(canonicalName); } + dependentBeans.add(dependentBeanName); + + Set dependenciesForBean = this.dependenciesForBeanMap.computeIfAbsent(dependentBeanName, + (key) -> Collections.synchronizedSet(new LinkedHashSet<>(8)) + ); + dependenciesForBean.add(canonicalName); } /** @@ -458,13 +456,15 @@ private boolean isDependent(String beanName, String dependentBeanName, @Nullable if (dependentBeans.contains(dependentBeanName)) { return true; } - for (String transitiveDependency : dependentBeans) { - if (alreadySeen == null) { - alreadySeen = new HashSet<>(); - } - alreadySeen.add(beanName); - if (isDependent(transitiveDependency, dependentBeanName, alreadySeen)) { - return true; + synchronized (dependentBeans) { + for (String transitiveDependency : dependentBeans) { + if (alreadySeen == null) { + alreadySeen = new HashSet<>(); + } + alreadySeen.add(beanName); + if (isDependent(transitiveDependency, dependentBeanName, alreadySeen)) { + return true; + } } } return false; @@ -488,7 +488,9 @@ public String[] getDependentBeans(String beanName) { if (dependentBeans == null) { return new String[0]; } - return StringUtils.toStringArray(dependentBeans); + synchronized (dependentBeans) { + return StringUtils.toStringArray(dependentBeans); + } } /** @@ -502,7 +504,9 @@ public String[] getDependenciesForBean(String beanName) { if (dependenciesForBean == null) { return new String[0]; } - return StringUtils.toStringArray(dependenciesForBean); + synchronized (dependenciesForBean) { + return StringUtils.toStringArray(dependenciesForBean); + } } public void destroySingletons() { @@ -565,8 +569,10 @@ protected void destroyBean(String beanName, @Nullable DisposableBean bean) { if (logger.isDebugEnabled()) { logger.debug("Retrieved dependent beans for bean '" + beanName + "': " + dependencies); } - for (String dependentBeanName : dependencies) { - destroySingleton(dependentBeanName); + synchronized (dependencies) { + for (String dependentBeanName : dependencies) { + destroySingleton(dependentBeanName); + } } } @@ -583,8 +589,10 @@ protected void destroyBean(String beanName, @Nullable DisposableBean bean) { // Trigger destruction of contained beans... Set containedBeans = this.containedBeanMap.remove(beanName); if (containedBeans != null) { - for (String containedBeanName : containedBeans) { - destroySingleton(containedBeanName); + synchronized (containedBeans) { + for (String containedBeanName : containedBeans) { + destroySingleton(containedBeanName); + } } }