From 8d8e218b52274055d7b1854f3363c8c8c4bd71f2 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 23 Mar 2018 17:34:33 +0100 Subject: [PATCH] Consistent FactoryBean cache visibility in concurrent creation scenarios Issue: SPR-16625 Issue: SPR-16627 --- .../AbstractAutowireCapableBeanFactory.java | 23 +++++++++++++--- .../support/DefaultSingletonBeanRegistry.java | 27 +++++++++---------- .../support/FactoryBeanRegistrySupport.java | 16 ++++++----- 3 files changed, 42 insertions(+), 24 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java index 41589161ab..feeb34d188 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java @@ -927,12 +927,16 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac if (bw != null) { return (FactoryBean) bw.getWrappedInstance(); } + Object beanInstance = getSingleton(beanName, false); + if (beanInstance instanceof FactoryBean) { + return (FactoryBean) beanInstance; + } if (isSingletonCurrentlyInCreation(beanName) || (mbd.getFactoryBeanName() != null && isSingletonCurrentlyInCreation(mbd.getFactoryBeanName()))) { return null; } - Object instance = null; + Object instance; try { // Mark this bean as currently in creation, even if just partially. beforeSingletonCreation(beanName); @@ -1845,8 +1849,21 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac */ @Override protected void removeSingleton(String beanName) { - super.removeSingleton(beanName); - this.factoryBeanInstanceCache.remove(beanName); + synchronized (getSingletonMutex()) { + super.removeSingleton(beanName); + this.factoryBeanInstanceCache.remove(beanName); + } + } + + /** + * Overridden to clear FactoryBean instance cache as well. + */ + @Override + protected void clearSingletonCache() { + synchronized (getSingletonMutex()) { + super.clearSingletonCache(); + this.factoryBeanInstanceCache.clear(); + } } 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 ee0296cfa6..b66e24b0ad 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 @@ -392,11 +392,7 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements // 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 = this.containedBeanMap.computeIfAbsent(containingBeanName, k -> new LinkedHashSet<>(8)); containedBeans.add(containedBeanName); } registerDependentBean(containedBeanName, containingBeanName); @@ -418,19 +414,12 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements // 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 = this.dependentBeanMap.computeIfAbsent(canonicalName, k -> new LinkedHashSet<>(8)); dependentBeans.add(dependentBeanName); } synchronized (this.dependenciesForBeanMap) { - Set dependenciesForBean = this.dependenciesForBeanMap.get(dependentBeanName); - if (dependenciesForBean == null) { - dependenciesForBean = new LinkedHashSet<>(8); - this.dependenciesForBeanMap.put(dependentBeanName, dependenciesForBean); - } + Set dependenciesForBean = + this.dependenciesForBeanMap.computeIfAbsent(dependentBeanName, k -> new LinkedHashSet<>(8)); dependenciesForBean.add(canonicalName); } } @@ -525,6 +514,14 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements this.dependentBeanMap.clear(); this.dependenciesForBeanMap.clear(); + clearSingletonCache(); + } + + /** + * Clear all cached singleton instances in this registry. + * @since 4.3.15 + */ + protected void clearSingletonCache() { synchronized (this.singletonObjects) { this.singletonObjects.clear(); this.singletonFactories.clear(); diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/FactoryBeanRegistrySupport.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/FactoryBeanRegistrySupport.java index e12080fffd..0d316be9c4 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/FactoryBeanRegistrySupport.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/FactoryBeanRegistrySupport.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 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. @@ -214,17 +214,21 @@ public abstract class FactoryBeanRegistrySupport extends DefaultSingletonBeanReg */ @Override protected void removeSingleton(String beanName) { - super.removeSingleton(beanName); - this.factoryBeanObjectCache.remove(beanName); + synchronized (getSingletonMutex()) { + super.removeSingleton(beanName); + this.factoryBeanObjectCache.remove(beanName); + } } /** * Overridden to clear the FactoryBean object cache as well. */ @Override - public void destroySingletons() { - super.destroySingletons(); - this.factoryBeanObjectCache.clear(); + protected void clearSingletonCache() { + synchronized (getSingletonMutex()) { + super.clearSingletonCache(); + this.factoryBeanObjectCache.clear(); + } } /**