root
/
spring-framework
mirror of https://github.com/spring-projects/spring-framework.git
1
0
Fork 0
Code Issues Actions 4 Packages Projects Releases Wiki Activity

Revise singleton registry for lenient locking (fallback instead of deadlock) 

Closes gh-23501
This commit is contained in:
Juergen Hoeller 2024-02-19 15:49:33 +01:00
parent f529386ce2
commit 902e5707a8
8 changed files with 262 additions and 198 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/BeanFactoryAspectInstanceFactory.java
View File

@ -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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with 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. // Rely on singleton semantics provided by the factory -> no local lock.
return null; 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 { else {
// No singleton guarantees from the factory -> let's lock locally.
return this; return this;
} }
} }

21
spring-aop/src/main/java/org/springframework/aop/support/AbstractBeanFactoryPointcutAdvisor.java
View File

@ -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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with 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.BeanFactory;
import org.springframework.beans.factory.BeanFactoryAware; import org.springframework.beans.factory.BeanFactoryAware;
import org.springframework.beans.factory.config.ConfigurableBeanFactory;
import org.springframework.lang.Nullable; import org.springframework.lang.Nullable;
import org.springframework.util.Assert; import org.springframework.util.Assert;
@ -52,7 +51,7 @@ public abstract class AbstractBeanFactoryPointcutAdvisor extends AbstractPointcu
@Nullable @Nullable
private transient volatile Advice advice; 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 @Override
public void setBeanFactory(BeanFactory beanFactory) { public void setBeanFactory(BeanFactory beanFactory) {
this.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; return advice;
} }
else { else {
// No singleton guarantees from the factory -> let's lock locally but // No singleton guarantees from the factory -> let's lock locally.
// reuse the factory's singleton lock, just in case a lazy dependency
// of our advice bean happens to trigger the singleton lock implicitly...
synchronized (this.adviceMonitor) { synchronized (this.adviceMonitor) {
advice = this.advice; advice = this.advice;
if (advice == null) { if (advice == null) {
@ -155,7 +142,7 @@ public abstract class AbstractBeanFactoryPointcutAdvisor extends AbstractPointcu
ois.defaultReadObject(); ois.defaultReadObject();
// Initialize transient fields. // Initialize transient fields.
resetAdviceMonitor(); this.adviceMonitor = new Object();
} }
} }

5
spring-beans/src/main/java/org/springframework/beans/factory/config/SingletonBeanRegistry.java
View File

@ -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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with 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 singleton mutex used by this registry (for external collaborators).
* @return the mutex object (never {@code null}) * @return the mutex object (never {@code null})
* @since 4.2 * @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(); Object getSingletonMutex();
} }

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

@ -972,59 +972,54 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac
*/ */
@Nullable @Nullable
private FactoryBean<?> getSingletonFactoryBeanForTypeCheck(String beanName, RootBeanDefinition mbd) { private FactoryBean<?> getSingletonFactoryBeanForTypeCheck(String beanName, RootBeanDefinition mbd) {
synchronized (getSingletonMutex()) { BeanWrapper bw = this.factoryBeanInstanceCache.get(beanName);
BeanWrapper bw = this.factoryBeanInstanceCache.get(beanName); if (bw != null) {
if (bw != null) { return (FactoryBean<?>) bw.getWrappedInstance();
return (FactoryBean<?>) bw.getWrappedInstance(); }
} Object beanInstance = getSingleton(beanName, false);
Object beanInstance = getSingleton(beanName, false); if (beanInstance instanceof FactoryBean<?> factoryBean) {
if (beanInstance instanceof FactoryBean<?> factoryBean) { return factoryBean;
return factoryBean; }
} if (isSingletonCurrentlyInCreation(beanName) ||
if (isSingletonCurrentlyInCreation(beanName) || (mbd.getFactoryBeanName() != null && isSingletonCurrentlyInCreation(mbd.getFactoryBeanName()))) {
(mbd.getFactoryBeanName() != null && isSingletonCurrentlyInCreation(mbd.getFactoryBeanName()))) { return null;
return null; }
}
Object instance; Object instance;
try { try {
// Mark this bean as currently in creation, even if just partially. // Mark this bean as currently in creation, even if just partially.
beforeSingletonCreation(beanName); beforeSingletonCreation(beanName);
// Give BeanPostProcessors a chance to return a proxy instead of the target bean instance. // Give BeanPostProcessors a chance to return a proxy instead of the target bean instance.
instance = resolveBeforeInstantiation(beanName, mbd); instance = resolveBeforeInstantiation(beanName, mbd);
if (instance == null) { if (instance == null) {
bw = createBeanInstance(beanName, mbd, null); bw = createBeanInstance(beanName, mbd, null);
instance = bw.getWrappedInstance(); 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) {
this.factoryBeanInstanceCache.put(beanName, bw); 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 @Override
protected void removeSingleton(String beanName) { protected void removeSingleton(String beanName) {
synchronized (getSingletonMutex()) { super.removeSingleton(beanName);
super.removeSingleton(beanName); this.factoryBeanInstanceCache.remove(beanName);
this.factoryBeanInstanceCache.remove(beanName);
}
} }
/** /**
@ -1923,10 +1916,8 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac
*/ */
@Override @Override
protected void clearSingletonCache() { protected void clearSingletonCache() {
synchronized (getSingletonMutex()) { super.clearSingletonCache();
super.clearSingletonCache(); this.factoryBeanInstanceCache.clear();
this.factoryBeanInstanceCache.clear();
}
} }
/** /**

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

@ -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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with 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.Map;
import java.util.Set; import java.util.Set;
import java.util.concurrent.ConcurrentHashMap; 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.BeanCreationException;
import org.springframework.beans.factory.BeanCreationNotAllowedException; import org.springframework.beans.factory.BeanCreationNotAllowedException;
@ -84,7 +86,9 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements
private final Map<String, Object> earlySingletonObjects = new ConcurrentHashMap<>(16); private final Map<String, Object> earlySingletonObjects = new ConcurrentHashMap<>(16);
/** Set of registered singletons, containing the bean names in registration order. */ /** Set of registered singletons, containing the bean names in registration order. */
private final Set<String> registeredSingletons = new LinkedHashSet<>(256); private final Set<String> registeredSingletons = Collections.synchronizedSet(new LinkedHashSet<>(256));
private final Lock singletonLock = new ReentrantLock();
/** Names of beans that are currently in creation. */ /** Names of beans that are currently in creation. */
private final Set<String> singletonsCurrentlyInCreation = private final Set<String> singletonsCurrentlyInCreation =
@ -94,6 +98,9 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements
private final Set<String> inCreationCheckExclusions = private final Set<String> inCreationCheckExclusions =
Collections.newSetFromMap(new ConcurrentHashMap<>(16)); Collections.newSetFromMap(new ConcurrentHashMap<>(16));
@Nullable
private volatile Thread singletonCreationThread;
/** Collection of suppressed Exceptions, available for associating related causes. */ /** Collection of suppressed Exceptions, available for associating related causes. */
@Nullable @Nullable
private Set<Exception> suppressedExceptions; private Set<Exception> suppressedExceptions;
@ -118,7 +125,8 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements
public void registerSingleton(String beanName, Object singletonObject) throws IllegalStateException { public void registerSingleton(String beanName, Object singletonObject) throws IllegalStateException {
Assert.notNull(beanName, "Bean name must not be null"); Assert.notNull(beanName, "Bean name must not be null");
Assert.notNull(singletonObject, "Singleton object 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); Object oldObject = this.singletonObjects.get(beanName);
if (oldObject != null) { if (oldObject != null) {
throw new IllegalStateException("Could not register object [" + singletonObject + throw new IllegalStateException("Could not register object [" + singletonObject +
@ -126,6 +134,9 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements
} }
addSingleton(beanName, singletonObject); addSingleton(beanName, singletonObject);
} }
finally {
this.singletonLock.unlock();
}
} }
/** /**
@ -135,12 +146,10 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements
* @param singletonObject the singleton object * @param singletonObject the singleton object
*/ */
protected void addSingleton(String beanName, Object singletonObject) { protected void addSingleton(String beanName, Object singletonObject) {
synchronized (this.singletonObjects) { this.singletonObjects.put(beanName, singletonObject);
this.singletonObjects.put(beanName, singletonObject); this.singletonFactories.remove(beanName);
this.singletonFactories.remove(beanName); this.earlySingletonObjects.remove(beanName);
this.earlySingletonObjects.remove(beanName); this.registeredSingletons.add(beanName);
this.registeredSingletons.add(beanName);
}
} }
/** /**
@ -153,13 +162,9 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements
*/ */
protected void addSingletonFactory(String beanName, ObjectFactory<?> singletonFactory) { protected void addSingletonFactory(String beanName, ObjectFactory<?> singletonFactory) {
Assert.notNull(singletonFactory, "Singleton factory must not be null"); Assert.notNull(singletonFactory, "Singleton factory must not be null");
synchronized (this.singletonObjects) { this.singletonFactories.put(beanName, singletonFactory);
if (!this.singletonObjects.containsKey(beanName)) { this.earlySingletonObjects.remove(beanName);
this.singletonFactories.put(beanName, singletonFactory); this.registeredSingletons.add(beanName);
this.earlySingletonObjects.remove(beanName);
this.registeredSingletons.add(beanName);
}
}
} }
@Override @Override
@ -183,7 +188,8 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements
if (singletonObject == null && isSingletonCurrentlyInCreation(beanName)) { if (singletonObject == null && isSingletonCurrentlyInCreation(beanName)) {
singletonObject = this.earlySingletonObjects.get(beanName); singletonObject = this.earlySingletonObjects.get(beanName);
if (singletonObject == null && allowEarlyReference) { if (singletonObject == null && allowEarlyReference) {
synchronized (this.singletonObjects) { this.singletonLock.lock();
try {
// Consistent creation of early reference within full singleton lock // Consistent creation of early reference within full singleton lock
singletonObject = this.singletonObjects.get(beanName); singletonObject = this.singletonObjects.get(beanName);
if (singletonObject == null) { if (singletonObject == null) {
@ -198,6 +204,9 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements
} }
} }
} }
finally {
this.singletonLock.unlock();
}
} }
} }
return singletonObject; return singletonObject;
@ -213,9 +222,33 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements
*/ */
public Object getSingleton(String beanName, ObjectFactory<?> singletonFactory) { public Object getSingleton(String beanName, ObjectFactory<?> singletonFactory) {
Assert.notNull(beanName, "Bean name must not be null"); Assert.notNull(beanName, "Bean name must not be null");
synchronized (this.singletonObjects) {
boolean locked = this.singletonLock.tryLock();
try {
Object singletonObject = this.singletonObjects.get(beanName); Object singletonObject = this.singletonObjects.get(beanName);
if (singletonObject == null) { 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) { if (this.singletonsCurrentlyInDestruction) {
throw new BeanCreationNotAllowedException(beanName, throw new BeanCreationNotAllowedException(beanName,
"Singleton bean creation not allowed while singletons of this factory are in destruction " + "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); beforeSingletonCreation(beanName);
boolean newSingleton = false; boolean newSingleton = false;
boolean recordSuppressedExceptions = (this.suppressedExceptions == null); boolean recordSuppressedExceptions = (locked && this.suppressedExceptions == null);
if (recordSuppressedExceptions) { if (recordSuppressedExceptions) {
this.suppressedExceptions = new LinkedHashSet<>(); this.suppressedExceptions = new LinkedHashSet<>();
} }
this.singletonCreationThread = Thread.currentThread();
try { try {
singletonObject = singletonFactory.getObject(); singletonObject = singletonFactory.getObject();
newSingleton = true; newSingleton = true;
@ -251,6 +285,7 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements
throw ex; throw ex;
} }
finally { finally {
this.singletonCreationThread = null;
if (recordSuppressedExceptions) { if (recordSuppressedExceptions) {
this.suppressedExceptions = null; this.suppressedExceptions = null;
} }
@ -262,6 +297,11 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements
} }
return singletonObject; return singletonObject;
} }
finally {
if (locked) {
this.singletonLock.unlock();
}
}
} }
/** /**
@ -274,10 +314,8 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements
* @see BeanCreationException#getRelatedCauses() * @see BeanCreationException#getRelatedCauses()
*/ */
protected void onSuppressedException(Exception ex) { protected void onSuppressedException(Exception ex) {
synchronized (this.singletonObjects) { if (this.suppressedExceptions != null && this.suppressedExceptions.size() < SUPPRESSED_EXCEPTIONS_LIMIT) {
if (this.suppressedExceptions != null && this.suppressedExceptions.size() < SUPPRESSED_EXCEPTIONS_LIMIT) { this.suppressedExceptions.add(ex);
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, * 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. * to be able to clean up eager registration of a singleton if creation failed.
* @param beanName the name of the bean * @param beanName the name of the bean
* @see #getSingletonMutex()
*/ */
protected void removeSingleton(String beanName) { protected void removeSingleton(String beanName) {
synchronized (this.singletonObjects) { this.singletonObjects.remove(beanName);
this.singletonObjects.remove(beanName); this.singletonFactories.remove(beanName);
this.singletonFactories.remove(beanName); this.earlySingletonObjects.remove(beanName);
this.earlySingletonObjects.remove(beanName); this.registeredSingletons.remove(beanName);
this.registeredSingletons.remove(beanName);
}
} }
@Override @Override
@ -303,16 +338,12 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements
@Override @Override
public String[] getSingletonNames() { public String[] getSingletonNames() {
synchronized (this.singletonObjects) { return StringUtils.toStringArray(this.registeredSingletons);
return StringUtils.toStringArray(this.registeredSingletons);
}
} }
@Override @Override
public int getSingletonCount() { 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()) { if (logger.isTraceEnabled()) {
logger.trace("Destroying singletons in " + this); logger.trace("Destroying singletons in " + this);
} }
synchronized (this.singletonObjects) { this.singletonLock.lock();
try {
this.singletonsCurrentlyInDestruction = true; this.singletonsCurrentlyInDestruction = true;
} }
finally {
this.singletonLock.unlock();
}
String[] disposableBeanNames; String[] disposableBeanNames;
synchronized (this.disposableBeans) { synchronized (this.disposableBeans) {
@ -524,7 +559,13 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements
this.dependentBeanMap.clear(); this.dependentBeanMap.clear();
this.dependenciesForBeanMap.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 * @since 4.3.15
*/ */
protected void clearSingletonCache() { protected void clearSingletonCache() {
synchronized (this.singletonObjects) { this.singletonObjects.clear();
this.singletonObjects.clear(); this.singletonFactories.clear();
this.singletonFactories.clear(); this.earlySingletonObjects.clear();
this.earlySingletonObjects.clear(); this.registeredSingletons.clear();
this.registeredSingletons.clear(); this.singletonsCurrentlyInDestruction = false;
this.singletonsCurrentlyInDestruction = false;
}
} }
/** /**
@ -549,7 +588,13 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements
*/ */
public void destroySingleton(String beanName) { public void destroySingleton(String beanName) {
// Remove a registered singleton of the given name, if any. // 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. // Destroy the corresponding DisposableBean instance.
DisposableBean disposableBean; DisposableBean disposableBean;
@ -621,16 +666,10 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements
this.dependenciesForBeanMap.remove(beanName); this.dependenciesForBeanMap.remove(beanName);
} }
/** @Deprecated(since = "6.2")
* Exposes the singleton mutex to subclasses and external collaborators.
* <p>Subclasses should synchronize on the given Object if they perform
* any sort of extended singleton creation phase. In particular, subclasses
* should <i>not</i> have their own mutexes involved in singleton creation,
* to avoid the potential for deadlocks in lazy-init situations.
*/
@Override @Override
public final Object getSingletonMutex() { public final Object getSingletonMutex() {
return this.singletonObjects; return new Object();
} }
} }

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

@ -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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with 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) { protected Object getObjectFromFactoryBean(FactoryBean<?> factory, String beanName, boolean shouldPostProcess) {
if (factory.isSingleton() && containsSingleton(beanName)) { if (factory.isSingleton() && containsSingleton(beanName)) {
synchronized (getSingletonMutex()) { Object object = this.factoryBeanObjectCache.get(beanName);
Object object = this.factoryBeanObjectCache.get(beanName); if (object == null) {
if (object == null) { object = doGetObjectFromFactoryBean(factory, beanName);
object = doGetObjectFromFactoryBean(factory, beanName); // Only post-process and store if not put there already during getObject() call above
// 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)
// (e.g. because of circular reference processing triggered by custom getBean calls) Object alreadyThere = this.factoryBeanObjectCache.get(beanName);
Object alreadyThere = this.factoryBeanObjectCache.get(beanName); if (alreadyThere != null) {
if (alreadyThere != null) { object = alreadyThere;
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 (containsSingleton(beanName)) {
if (shouldPostProcess) { this.factoryBeanObjectCache.put(beanName, object);
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);
}
} }
} }
return object;
} }
return object;
} }
else { else {
Object object = doGetObjectFromFactoryBean(factory, beanName); Object object = doGetObjectFromFactoryBean(factory, beanName);
@ -234,10 +232,8 @@ public abstract class FactoryBeanRegistrySupport extends DefaultSingletonBeanReg
*/ */
@Override @Override
protected void removeSingleton(String beanName) { protected void removeSingleton(String beanName) {
synchronized (getSingletonMutex()) { super.removeSingleton(beanName);
super.removeSingleton(beanName); this.factoryBeanObjectCache.remove(beanName);
this.factoryBeanObjectCache.remove(beanName);
}
} }
/** /**
@ -245,10 +241,8 @@ public abstract class FactoryBeanRegistrySupport extends DefaultSingletonBeanReg
*/ */
@Override @Override
protected void clearSingletonCache() { protected void clearSingletonCache() {
synchronized (getSingletonMutex()) { super.clearSingletonCache();
super.clearSingletonCache(); this.factoryBeanObjectCache.clear();
this.factoryBeanObjectCache.clear();
}
} }
} }

65
spring-beans/src/test/java/org/springframework/beans/factory/BeanFactoryLockingTests.java Normal file
View File

@ -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");
}
}
}
}

34
spring-jms/src/main/java/org/springframework/jms/config/JmsListenerEndpointRegistrar.java
View File

@ -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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with 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.BeanFactory;
import org.springframework.beans.factory.BeanFactoryAware; import org.springframework.beans.factory.BeanFactoryAware;
import org.springframework.beans.factory.InitializingBean; import org.springframework.beans.factory.InitializingBean;
import org.springframework.beans.factory.config.ConfigurableBeanFactory;
import org.springframework.lang.Nullable; import org.springframework.lang.Nullable;
import org.springframework.messaging.handler.annotation.support.DefaultMessageHandlerMethodFactory; import org.springframework.messaging.handler.annotation.support.DefaultMessageHandlerMethodFactory;
import org.springframework.messaging.handler.annotation.support.MessageHandlerMethodFactory; import org.springframework.messaging.handler.annotation.support.MessageHandlerMethodFactory;
@ -57,8 +56,6 @@ public class JmsListenerEndpointRegistrar implements BeanFactoryAware, Initializ
private boolean startImmediately; private boolean startImmediately;
private Object mutex = this.endpointDescriptors;
/** /**
* Set the {@link JmsListenerEndpointRegistry} instance to use. * Set the {@link JmsListenerEndpointRegistry} instance to use.
@ -124,9 +121,6 @@ public class JmsListenerEndpointRegistrar implements BeanFactoryAware, Initializ
@Override @Override
public void setBeanFactory(BeanFactory beanFactory) { public void setBeanFactory(BeanFactory beanFactory) {
this.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() { protected void registerAllEndpoints() {
Assert.state(this.endpointRegistry != null, "No JmsListenerEndpointRegistry set"); Assert.state(this.endpointRegistry != null, "No JmsListenerEndpointRegistry set");
synchronized (this.mutex) { for (JmsListenerEndpointDescriptor descriptor : this.endpointDescriptors) {
for (JmsListenerEndpointDescriptor descriptor : this.endpointDescriptors) { this.endpointRegistry.registerListenerContainer(
this.endpointRegistry.registerListenerContainer( descriptor.endpoint, resolveContainerFactory(descriptor));
descriptor.endpoint, resolveContainerFactory(descriptor));
}
this.startImmediately = true; // trigger immediate startup
} }
this.startImmediately = true; // trigger immediate startup
} }
private JmsListenerContainerFactory<?> resolveContainerFactory(JmsListenerEndpointDescriptor descriptor) { 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 // Factory may be null, we defer the resolution right before actually creating the container
JmsListenerEndpointDescriptor descriptor = new JmsListenerEndpointDescriptor(endpoint, factory); JmsListenerEndpointDescriptor descriptor = new JmsListenerEndpointDescriptor(endpoint, factory);
synchronized (this.mutex) { if (this.startImmediately) { // register and start immediately
if (this.startImmediately) { // register and start immediately Assert.state(this.endpointRegistry != null, "No JmsListenerEndpointRegistry set");
Assert.state(this.endpointRegistry != null, "No JmsListenerEndpointRegistry set"); this.endpointRegistry.registerListenerContainer(descriptor.endpoint,
this.endpointRegistry.registerListenerContainer(descriptor.endpoint, resolveContainerFactory(descriptor), true);
resolveContainerFactory(descriptor), true); }
} else {
else { this.endpointDescriptors.add(descriptor);
this.endpointDescriptors.add(descriptor);
}
} }
} }