Consistently use singleton lock for FactoryBean processing

Closes gh-33972

Author: jhoeller

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

@@ -991,54 +991,60 @@ protected Object getEarlyBeanReference(String beanName, RootBeanDefinition mbd,
 	 */
 	@Nullable
 	private FactoryBean<?> getSingletonFactoryBeanForTypeCheck(String beanName, RootBeanDefinition mbd) {
-		BeanWrapper bw = this.factoryBeanInstanceCache.get(beanName);
-		if (bw != null) {
-			return (FactoryBean<?>) bw.getWrappedInstance();
-		}
-		Object beanInstance = getSingleton(beanName, false);
-		if (beanInstance instanceof FactoryBean<?> factoryBean) {
-			return factoryBean;
-		}
-		if (isSingletonCurrentlyInCreation(beanName) ||
-				(mbd.getFactoryBeanName() != null && isSingletonCurrentlyInCreation(mbd.getFactoryBeanName()))) {
-			return null;
-		}
-
-		Object instance;
+		this.singletonLock.lock();
 		try {
-			// Mark this bean as currently in creation, even if just partially.
-			beforeSingletonCreation(beanName);
-			// Give BeanPostProcessors a chance to return a proxy instead of the target bean instance.
-			instance = resolveBeforeInstantiation(beanName, mbd);
-			if (instance == null) {
-				bw = createBeanInstance(beanName, mbd, null);
-				instance = bw.getWrappedInstance();
-				this.factoryBeanInstanceCache.put(beanName, bw);
+			BeanWrapper bw = this.factoryBeanInstanceCache.get(beanName);
+			if (bw != null) {
+				return (FactoryBean<?>) bw.getWrappedInstance();
 			}
-		}
-		catch (UnsatisfiedDependencyException ex) {
-			// Don't swallow, probably misconfiguration...
-			throw ex;
-		}
-		catch (BeanCreationException ex) {
-			// Don't swallow a linkage error since it contains a full stacktrace on
-			// first occurrence... and just a plain NoClassDefFoundError afterwards.
-			if (ex.contains(LinkageError.class)) {
+			Object beanInstance = getSingleton(beanName, false);
+			if (beanInstance instanceof FactoryBean<?> factoryBean) {
+				return factoryBean;
+			}
+			if (isSingletonCurrentlyInCreation(beanName) ||
+					(mbd.getFactoryBeanName() != null && isSingletonCurrentlyInCreation(mbd.getFactoryBeanName()))) {
+				return null;
+			}
+
+			Object instance;
+			try {
+				// Mark this bean as currently in creation, even if just partially.
+				beforeSingletonCreation(beanName);
+				// Give BeanPostProcessors a chance to return a proxy instead of the target bean instance.
+				instance = resolveBeforeInstantiation(beanName, mbd);
+				if (instance == null) {
+					bw = createBeanInstance(beanName, mbd, null);
+					instance = bw.getWrappedInstance();
+					this.factoryBeanInstanceCache.put(beanName, bw);
+				}
+			}
+			catch (UnsatisfiedDependencyException ex) {
+				// Don't swallow, probably misconfiguration...
 				throw ex;
 			}
-			// Instantiation failure, maybe too early...
-			if (logger.isDebugEnabled()) {
-				logger.debug("Bean creation exception on singleton FactoryBean type check: " + ex);
+			catch (BeanCreationException ex) {
+				// Don't swallow a linkage error since it contains a full stacktrace on
+				// first occurrence... and just a plain NoClassDefFoundError afterwards.
+				if (ex.contains(LinkageError.class)) {
+					throw ex;
+				}
+				// Instantiation failure, maybe too early...
+				if (logger.isDebugEnabled()) {
+					logger.debug("Bean creation exception on singleton FactoryBean type check: " + ex);
+				}
+				onSuppressedException(ex);
+				return null;
 			}
-			onSuppressedException(ex);
-			return null;
+			finally {
+				// Finished partial creation of this bean.
+				afterSingletonCreation(beanName);
+			}
+
+			return getFactoryBean(beanName, instance);
 		}
 		finally {
-			// Finished partial creation of this bean.
-			afterSingletonCreation(beanName);
+			this.singletonLock.unlock();
 		}
-
-		return getFactoryBean(beanName, instance);
 	}
 
 	/**

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

@@ -76,6 +76,9 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements
 	private static final int SUPPRESSED_EXCEPTIONS_LIMIT = 100;
 
 
+	/** Common lock for singleton creation. */
+	final Lock singletonLock = new ReentrantLock();
+
 	/** Cache of singleton objects: bean name to bean instance. */
 	private final Map<String, Object> singletonObjects = new ConcurrentHashMap<>(256);
 
@@ -91,8 +94,6 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements
 	/** Set of registered singletons, containing the bean names in registration order. */
 	private final Set<String> registeredSingletons = Collections.synchronizedSet(new LinkedHashSet<>(256));
 
-	private final Lock singletonLock = new ReentrantLock();
-
 	/** Names of beans that are currently in creation. */
 	private final Set<String> singletonsCurrentlyInCreation = ConcurrentHashMap.newKeySet(16);
 

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

@@ -118,39 +118,45 @@ protected Object getCachedObjectForFactoryBean(String beanName) {
 	 */
 	protected Object getObjectFromFactoryBean(FactoryBean<?> factory, String beanName, boolean shouldPostProcess) {
 		if (factory.isSingleton() && containsSingleton(beanName)) {
-			Object object = this.factoryBeanObjectCache.get(beanName);
-			if (object == null) {
-				object = doGetObjectFromFactoryBean(factory, beanName);
-				// Only post-process and store if not put there already during getObject() call above
-				// (for example, because of circular reference processing triggered by custom getBean calls)
-				Object alreadyThere = this.factoryBeanObjectCache.get(beanName);
-				if (alreadyThere != null) {
-					object = alreadyThere;
-				}
-				else {
-					if (shouldPostProcess) {
-						if (isSingletonCurrentlyInCreation(beanName)) {
-							// Temporarily return non-post-processed object, not storing it yet
-							return object;
-						}
-						beforeSingletonCreation(beanName);
-						try {
-							object = postProcessObjectFromFactoryBean(object, beanName);
-						}
-						catch (Throwable ex) {
-							throw new BeanCreationException(beanName,
-									"Post-processing of FactoryBean's singleton object failed", ex);
+			this.singletonLock.lock();
+			try {
+				Object object = this.factoryBeanObjectCache.get(beanName);
+				if (object == null) {
+					object = doGetObjectFromFactoryBean(factory, beanName);
+					// Only post-process and store if not put there already during getObject() call above
+					// (for example, because of circular reference processing triggered by custom getBean calls)
+					Object alreadyThere = this.factoryBeanObjectCache.get(beanName);
+					if (alreadyThere != null) {
+						object = alreadyThere;
+					}
+					else {
+						if (shouldPostProcess) {
+							if (isSingletonCurrentlyInCreation(beanName)) {
+								// Temporarily return non-post-processed object, not storing it yet
+								return object;
+							}
+							beforeSingletonCreation(beanName);
+							try {
+								object = postProcessObjectFromFactoryBean(object, beanName);
+							}
+							catch (Throwable ex) {
+								throw new BeanCreationException(beanName,
+										"Post-processing of FactoryBean's singleton object failed", ex);
+							}
+							finally {
+								afterSingletonCreation(beanName);
+							}
 						}
-						finally {
-							afterSingletonCreation(beanName);
+						if (containsSingleton(beanName)) {
+							this.factoryBeanObjectCache.put(beanName, object);
 						}
 					}
-					if (containsSingleton(beanName)) {
-						this.factoryBeanObjectCache.put(beanName, object);
-					}
 				}
+				return object;
+			}
+			finally {
+				this.singletonLock.unlock();
 			}
-			return object;
 		}
 		else {
 			Object object = doGetObjectFromFactoryBean(factory, beanName);