diff --git a/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/BeanFactoryAspectInstanceFactory.java b/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/BeanFactoryAspectInstanceFactory.java index 3bdbb9c16a..bef8a37b71 100644 --- a/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/BeanFactoryAspectInstanceFactory.java +++ b/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/BeanFactoryAspectInstanceFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2024 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. @@ -109,13 +109,8 @@ public class BeanFactoryAspectInstanceFactory implements MetadataAwareAspectInst // Rely on singleton semantics provided by the factory -> no local lock. return null; } - else if (this.beanFactory instanceof ConfigurableBeanFactory cbf) { - // No singleton guarantees from the factory -> let's lock locally but - // reuse the factory's singleton lock, just in case a lazy dependency - // of our advice bean happens to trigger the singleton lock implicitly... - return cbf.getSingletonMutex(); - } else { + // No singleton guarantees from the factory -> let's lock locally. return this; } } diff --git a/spring-aop/src/main/java/org/springframework/aop/support/AbstractBeanFactoryPointcutAdvisor.java b/spring-aop/src/main/java/org/springframework/aop/support/AbstractBeanFactoryPointcutAdvisor.java index f9efdc469a..e6a10c621b 100644 --- a/spring-aop/src/main/java/org/springframework/aop/support/AbstractBeanFactoryPointcutAdvisor.java +++ b/spring-aop/src/main/java/org/springframework/aop/support/AbstractBeanFactoryPointcutAdvisor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2024 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. @@ -23,7 +23,6 @@ import org.aopalliance.aop.Advice; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryAware; -import org.springframework.beans.factory.config.ConfigurableBeanFactory; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -52,7 +51,7 @@ public abstract class AbstractBeanFactoryPointcutAdvisor extends AbstractPointcu @Nullable private transient volatile Advice advice; - private transient volatile Object adviceMonitor = new Object(); + private transient Object adviceMonitor = new Object(); /** @@ -78,16 +77,6 @@ public abstract class AbstractBeanFactoryPointcutAdvisor extends AbstractPointcu @Override public void setBeanFactory(BeanFactory beanFactory) { this.beanFactory = beanFactory; - resetAdviceMonitor(); - } - - private void resetAdviceMonitor() { - if (this.beanFactory instanceof ConfigurableBeanFactory cbf) { - this.adviceMonitor = cbf.getSingletonMutex(); - } - else { - this.adviceMonitor = new Object(); - } } /** @@ -118,9 +107,7 @@ public abstract class AbstractBeanFactoryPointcutAdvisor extends AbstractPointcu return advice; } else { - // No singleton guarantees from the factory -> let's lock locally but - // reuse the factory's singleton lock, just in case a lazy dependency - // of our advice bean happens to trigger the singleton lock implicitly... + // No singleton guarantees from the factory -> let's lock locally. synchronized (this.adviceMonitor) { advice = this.advice; if (advice == null) { @@ -155,7 +142,7 @@ public abstract class AbstractBeanFactoryPointcutAdvisor extends AbstractPointcu ois.defaultReadObject(); // Initialize transient fields. - resetAdviceMonitor(); + this.adviceMonitor = new Object(); } } diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/config/SingletonBeanRegistry.java b/spring-beans/src/main/java/org/springframework/beans/factory/config/SingletonBeanRegistry.java index ee6d4a778c..3afc063f50 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/config/SingletonBeanRegistry.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/config/SingletonBeanRegistry.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2024 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. @@ -129,7 +129,10 @@ public interface SingletonBeanRegistry { * Return the singleton mutex used by this registry (for external collaborators). * @return the mutex object (never {@code null}) * @since 4.2 + * @deprecated as of 6.2, in favor of lenient singleton locking + * (with this method returning an arbitrary object to lock on) */ + @Deprecated(since = "6.2") Object getSingletonMutex(); } 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 5883af47ef..2b01d4a236 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 @@ -972,59 +972,54 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac */ @Nullable private FactoryBean getSingletonFactoryBeanForTypeCheck(String beanName, RootBeanDefinition mbd) { - synchronized (getSingletonMutex()) { - 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; - } + 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; - 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(); - } - } - 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)) { - 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; - } - finally { - // Finished partial creation of this bean. - afterSingletonCreation(beanName); - } - - FactoryBean fb = getFactoryBean(beanName, instance); - if (bw != 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); } - return fb; } + 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)) { + 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; + } + finally { + // Finished partial creation of this bean. + afterSingletonCreation(beanName); + } + + return getFactoryBean(beanName, instance); } /** @@ -1912,10 +1907,8 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac */ @Override protected void removeSingleton(String beanName) { - synchronized (getSingletonMutex()) { - super.removeSingleton(beanName); - this.factoryBeanInstanceCache.remove(beanName); - } + super.removeSingleton(beanName); + this.factoryBeanInstanceCache.remove(beanName); } /** @@ -1923,10 +1916,8 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac */ @Override protected void clearSingletonCache() { - synchronized (getSingletonMutex()) { - super.clearSingletonCache(); - this.factoryBeanInstanceCache.clear(); - } + 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 9b189b3431..da49209e89 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 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. @@ -25,6 +25,8 @@ import java.util.LinkedHashSet; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.springframework.beans.factory.BeanCreationException; import org.springframework.beans.factory.BeanCreationNotAllowedException; @@ -84,7 +86,9 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements private final Map earlySingletonObjects = new ConcurrentHashMap<>(16); /** Set of registered singletons, containing the bean names in registration order. */ - private final Set registeredSingletons = new LinkedHashSet<>(256); + private final Set registeredSingletons = Collections.synchronizedSet(new LinkedHashSet<>(256)); + + private final Lock singletonLock = new ReentrantLock(); /** Names of beans that are currently in creation. */ private final Set singletonsCurrentlyInCreation = @@ -94,6 +98,9 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements private final Set inCreationCheckExclusions = Collections.newSetFromMap(new ConcurrentHashMap<>(16)); + @Nullable + private volatile Thread singletonCreationThread; + /** Collection of suppressed Exceptions, available for associating related causes. */ @Nullable private Set suppressedExceptions; @@ -118,7 +125,8 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements public void registerSingleton(String beanName, Object singletonObject) throws IllegalStateException { Assert.notNull(beanName, "Bean name must not be null"); Assert.notNull(singletonObject, "Singleton object must not be null"); - synchronized (this.singletonObjects) { + this.singletonLock.lock(); + try { Object oldObject = this.singletonObjects.get(beanName); if (oldObject != null) { throw new IllegalStateException("Could not register object [" + singletonObject + @@ -126,6 +134,9 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements } addSingleton(beanName, singletonObject); } + finally { + this.singletonLock.unlock(); + } } /** @@ -135,12 +146,10 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements * @param singletonObject the singleton object */ protected void addSingleton(String beanName, Object singletonObject) { - synchronized (this.singletonObjects) { - this.singletonObjects.put(beanName, singletonObject); - this.singletonFactories.remove(beanName); - this.earlySingletonObjects.remove(beanName); - this.registeredSingletons.add(beanName); - } + this.singletonObjects.put(beanName, singletonObject); + this.singletonFactories.remove(beanName); + this.earlySingletonObjects.remove(beanName); + this.registeredSingletons.add(beanName); } /** @@ -153,13 +162,9 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements */ 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); - } - } + this.singletonFactories.put(beanName, singletonFactory); + this.earlySingletonObjects.remove(beanName); + this.registeredSingletons.add(beanName); } @Override @@ -183,7 +188,8 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements if (singletonObject == null && isSingletonCurrentlyInCreation(beanName)) { singletonObject = this.earlySingletonObjects.get(beanName); if (singletonObject == null && allowEarlyReference) { - synchronized (this.singletonObjects) { + this.singletonLock.lock(); + try { // Consistent creation of early reference within full singleton lock singletonObject = this.singletonObjects.get(beanName); if (singletonObject == null) { @@ -198,6 +204,9 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements } } } + finally { + this.singletonLock.unlock(); + } } } return singletonObject; @@ -213,9 +222,33 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements */ public Object getSingleton(String beanName, ObjectFactory singletonFactory) { Assert.notNull(beanName, "Bean name must not be null"); - synchronized (this.singletonObjects) { + + boolean locked = this.singletonLock.tryLock(); + try { Object singletonObject = this.singletonObjects.get(beanName); if (singletonObject == null) { + if (locked) { + this.singletonCreationThread = Thread.currentThread(); + } + else { + Thread otherThread = this.singletonCreationThread; + if (otherThread != null) { + // Another thread is busy in a singleton factory callback, potentially blocked. + // Fallback as of 6.2: process given singleton bean outside of singleton lock. + // Thread-safe exposure is still guaranteed, there is just a risk of collisions + // when triggering creation of other beans as dependencies of the current bean. + if (logger.isInfoEnabled()) { + logger.info("Creating singleton bean '" + beanName + "' in thread \"" + + Thread.currentThread().getName() + "\" while thread \"" + otherThread.getName() + + "\" holds singleton lock for other beans " + this.singletonsCurrentlyInCreation); + } + } + else { + // Singleton lock currently held by some other registration method -> wait. + this.singletonLock.lock(); + locked = true; + } + } if (this.singletonsCurrentlyInDestruction) { throw new BeanCreationNotAllowedException(beanName, "Singleton bean creation not allowed while singletons of this factory are in destruction " + @@ -226,10 +259,11 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements } beforeSingletonCreation(beanName); boolean newSingleton = false; - boolean recordSuppressedExceptions = (this.suppressedExceptions == null); + boolean recordSuppressedExceptions = (locked && this.suppressedExceptions == null); if (recordSuppressedExceptions) { this.suppressedExceptions = new LinkedHashSet<>(); } + this.singletonCreationThread = Thread.currentThread(); try { singletonObject = singletonFactory.getObject(); newSingleton = true; @@ -251,6 +285,7 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements throw ex; } finally { + this.singletonCreationThread = null; if (recordSuppressedExceptions) { this.suppressedExceptions = null; } @@ -262,6 +297,11 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements } return singletonObject; } + finally { + if (locked) { + this.singletonLock.unlock(); + } + } } /** @@ -274,10 +314,8 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements * @see BeanCreationException#getRelatedCauses() */ protected void onSuppressedException(Exception ex) { - synchronized (this.singletonObjects) { - if (this.suppressedExceptions != null && this.suppressedExceptions.size() < SUPPRESSED_EXCEPTIONS_LIMIT) { - this.suppressedExceptions.add(ex); - } + if (this.suppressedExceptions != null && this.suppressedExceptions.size() < SUPPRESSED_EXCEPTIONS_LIMIT) { + this.suppressedExceptions.add(ex); } } @@ -285,15 +323,12 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements * Remove the bean with the given name from the singleton cache of this factory, * to be able to clean up eager registration of a singleton if creation failed. * @param beanName the name of the bean - * @see #getSingletonMutex() */ protected void removeSingleton(String beanName) { - synchronized (this.singletonObjects) { - this.singletonObjects.remove(beanName); - this.singletonFactories.remove(beanName); - this.earlySingletonObjects.remove(beanName); - this.registeredSingletons.remove(beanName); - } + this.singletonObjects.remove(beanName); + this.singletonFactories.remove(beanName); + this.earlySingletonObjects.remove(beanName); + this.registeredSingletons.remove(beanName); } @Override @@ -303,16 +338,12 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements @Override public String[] getSingletonNames() { - synchronized (this.singletonObjects) { - return StringUtils.toStringArray(this.registeredSingletons); - } + return StringUtils.toStringArray(this.registeredSingletons); } @Override public int getSingletonCount() { - synchronized (this.singletonObjects) { - return this.registeredSingletons.size(); - } + return this.registeredSingletons.size(); } @@ -508,9 +539,13 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements if (logger.isTraceEnabled()) { logger.trace("Destroying singletons in " + this); } - synchronized (this.singletonObjects) { + this.singletonLock.lock(); + try { this.singletonsCurrentlyInDestruction = true; } + finally { + this.singletonLock.unlock(); + } String[] disposableBeanNames; synchronized (this.disposableBeans) { @@ -524,7 +559,13 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements this.dependentBeanMap.clear(); this.dependenciesForBeanMap.clear(); - clearSingletonCache(); + this.singletonLock.lock(); + try { + clearSingletonCache(); + } + finally { + this.singletonLock.unlock(); + } } /** @@ -532,13 +573,11 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements * @since 4.3.15 */ protected void clearSingletonCache() { - synchronized (this.singletonObjects) { - this.singletonObjects.clear(); - this.singletonFactories.clear(); - this.earlySingletonObjects.clear(); - this.registeredSingletons.clear(); - this.singletonsCurrentlyInDestruction = false; - } + this.singletonObjects.clear(); + this.singletonFactories.clear(); + this.earlySingletonObjects.clear(); + this.registeredSingletons.clear(); + this.singletonsCurrentlyInDestruction = false; } /** @@ -549,7 +588,13 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements */ public void destroySingleton(String beanName) { // Remove a registered singleton of the given name, if any. - removeSingleton(beanName); + this.singletonLock.lock(); + try { + removeSingleton(beanName); + } + finally { + this.singletonLock.unlock(); + } // Destroy the corresponding DisposableBean instance. DisposableBean disposableBean; @@ -621,16 +666,10 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements this.dependenciesForBeanMap.remove(beanName); } - /** - * Exposes the singleton mutex to subclasses and external collaborators. - *

Subclasses should synchronize on the given Object if they perform - * any sort of extended singleton creation phase. In particular, subclasses - * should not have their own mutexes involved in singleton creation, - * to avoid the potential for deadlocks in lazy-init situations. - */ + @Deprecated(since = "6.2") @Override public final Object getSingletonMutex() { - return this.singletonObjects; + return new Object(); } } 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 bd19a2f4fc..bab9915aaf 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-2023 the original author or authors. + * Copyright 2002-2024 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. @@ -118,41 +118,39 @@ public abstract class FactoryBeanRegistrySupport extends DefaultSingletonBeanReg */ protected Object getObjectFromFactoryBean(FactoryBean factory, String beanName, boolean shouldPostProcess) { if (factory.isSingleton() && containsSingleton(beanName)) { - synchronized (getSingletonMutex()) { - 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 - // (e.g. because of circular reference processing triggered by custom getBean calls) - Object alreadyThere = this.factoryBeanObjectCache.get(beanName); - if (alreadyThere != null) { - object = alreadyThere; + 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 + // (e.g. 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); + } } - 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); - } - } - if (containsSingleton(beanName)) { - this.factoryBeanObjectCache.put(beanName, object); - } + if (containsSingleton(beanName)) { + this.factoryBeanObjectCache.put(beanName, object); } } - return object; } + return object; } else { Object object = doGetObjectFromFactoryBean(factory, beanName); @@ -234,10 +232,8 @@ public abstract class FactoryBeanRegistrySupport extends DefaultSingletonBeanReg */ @Override protected void removeSingleton(String beanName) { - synchronized (getSingletonMutex()) { - super.removeSingleton(beanName); - this.factoryBeanObjectCache.remove(beanName); - } + super.removeSingleton(beanName); + this.factoryBeanObjectCache.remove(beanName); } /** @@ -245,10 +241,8 @@ public abstract class FactoryBeanRegistrySupport extends DefaultSingletonBeanReg */ @Override protected void clearSingletonCache() { - synchronized (getSingletonMutex()) { - super.clearSingletonCache(); - this.factoryBeanObjectCache.clear(); - } + super.clearSingletonCache(); + this.factoryBeanObjectCache.clear(); } } diff --git a/spring-beans/src/test/java/org/springframework/beans/factory/BeanFactoryLockingTests.java b/spring-beans/src/test/java/org/springframework/beans/factory/BeanFactoryLockingTests.java new file mode 100644 index 0000000000..e5966f675a --- /dev/null +++ b/spring-beans/src/test/java/org/springframework/beans/factory/BeanFactoryLockingTests.java @@ -0,0 +1,65 @@ +/* + * Copyright 2002-2024 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.beans.factory; + +import org.junit.jupiter.api.Test; + +import org.springframework.beans.factory.support.DefaultListableBeanFactory; +import org.springframework.beans.factory.support.RootBeanDefinition; +import org.springframework.beans.testfixture.beans.TestBean; + +/** + * @author Juergen Hoeller + * @since 6.2 + */ +class BeanFactoryLockingTests { + + @Test + void fallbackForThreadDuringInitialization() { + DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory(); + beanFactory.registerBeanDefinition("bean1", new RootBeanDefinition(ThreadDuringInitialization.class)); + beanFactory.registerBeanDefinition("bean2", new RootBeanDefinition(TestBean.class)); + beanFactory.getBean(ThreadDuringInitialization.class); + } + + + static class ThreadDuringInitialization implements BeanFactoryAware, InitializingBean { + + private BeanFactory beanFactory; + + private volatile boolean initialized; + + @Override + public void setBeanFactory(BeanFactory beanFactory) { + this.beanFactory = beanFactory; + } + + @Override + public void afterPropertiesSet() throws Exception { + Thread thread = new Thread(() -> { + beanFactory.getBean(TestBean.class); + initialized = true; + }); + thread.start(); + thread.join(); + if (!initialized) { + throw new IllegalStateException("Thread not executed"); + } + } + } + +} diff --git a/spring-jms/src/main/java/org/springframework/jms/config/JmsListenerEndpointRegistrar.java b/spring-jms/src/main/java/org/springframework/jms/config/JmsListenerEndpointRegistrar.java index 3848ba8954..faaf911973 100644 --- a/spring-jms/src/main/java/org/springframework/jms/config/JmsListenerEndpointRegistrar.java +++ b/spring-jms/src/main/java/org/springframework/jms/config/JmsListenerEndpointRegistrar.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 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. @@ -22,7 +22,6 @@ import java.util.List; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryAware; import org.springframework.beans.factory.InitializingBean; -import org.springframework.beans.factory.config.ConfigurableBeanFactory; import org.springframework.lang.Nullable; import org.springframework.messaging.handler.annotation.support.DefaultMessageHandlerMethodFactory; import org.springframework.messaging.handler.annotation.support.MessageHandlerMethodFactory; @@ -57,8 +56,6 @@ public class JmsListenerEndpointRegistrar implements BeanFactoryAware, Initializ private boolean startImmediately; - private Object mutex = this.endpointDescriptors; - /** * Set the {@link JmsListenerEndpointRegistry} instance to use. @@ -124,9 +121,6 @@ public class JmsListenerEndpointRegistrar implements BeanFactoryAware, Initializ @Override public void setBeanFactory(BeanFactory beanFactory) { this.beanFactory = beanFactory; - if (beanFactory instanceof ConfigurableBeanFactory cbf) { - this.mutex = cbf.getSingletonMutex(); - } } @@ -137,13 +131,11 @@ public class JmsListenerEndpointRegistrar implements BeanFactoryAware, Initializ protected void registerAllEndpoints() { Assert.state(this.endpointRegistry != null, "No JmsListenerEndpointRegistry set"); - synchronized (this.mutex) { - for (JmsListenerEndpointDescriptor descriptor : this.endpointDescriptors) { - this.endpointRegistry.registerListenerContainer( - descriptor.endpoint, resolveContainerFactory(descriptor)); - } - this.startImmediately = true; // trigger immediate startup + for (JmsListenerEndpointDescriptor descriptor : this.endpointDescriptors) { + this.endpointRegistry.registerListenerContainer( + descriptor.endpoint, resolveContainerFactory(descriptor)); } + this.startImmediately = true; // trigger immediate startup } private JmsListenerContainerFactory resolveContainerFactory(JmsListenerEndpointDescriptor descriptor) { @@ -180,15 +172,13 @@ public class JmsListenerEndpointRegistrar implements BeanFactoryAware, Initializ // Factory may be null, we defer the resolution right before actually creating the container JmsListenerEndpointDescriptor descriptor = new JmsListenerEndpointDescriptor(endpoint, factory); - synchronized (this.mutex) { - if (this.startImmediately) { // register and start immediately - Assert.state(this.endpointRegistry != null, "No JmsListenerEndpointRegistry set"); - this.endpointRegistry.registerListenerContainer(descriptor.endpoint, - resolveContainerFactory(descriptor), true); - } - else { - this.endpointDescriptors.add(descriptor); - } + if (this.startImmediately) { // register and start immediately + Assert.state(this.endpointRegistry != null, "No JmsListenerEndpointRegistry set"); + this.endpointRegistry.registerListenerContainer(descriptor.endpoint, + resolveContainerFactory(descriptor), true); + } + else { + this.endpointDescriptors.add(descriptor); } }