Skip to content

Commit 902e570

Browse files
committed
Revise singleton registry for lenient locking (fallback instead of deadlock)
Closes gh-23501
1 parent f529386 commit 902e570

File tree

8 files changed

+259
-195
lines changed

8 files changed

+259
-195
lines changed

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

+2-7
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -109,13 +109,8 @@ public Object getAspectCreationMutex() {
109109
// Rely on singleton semantics provided by the factory -> no local lock.
110110
return null;
111111
}
112-
else if (this.beanFactory instanceof ConfigurableBeanFactory cbf) {
113-
// No singleton guarantees from the factory -> let's lock locally but
114-
// reuse the factory's singleton lock, just in case a lazy dependency
115-
// of our advice bean happens to trigger the singleton lock implicitly...
116-
return cbf.getSingletonMutex();
117-
}
118112
else {
113+
// No singleton guarantees from the factory -> let's lock locally.
119114
return this;
120115
}
121116
}

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

+4-17
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -23,7 +23,6 @@
2323

2424
import org.springframework.beans.factory.BeanFactory;
2525
import org.springframework.beans.factory.BeanFactoryAware;
26-
import org.springframework.beans.factory.config.ConfigurableBeanFactory;
2726
import org.springframework.lang.Nullable;
2827
import org.springframework.util.Assert;
2928

@@ -52,7 +51,7 @@ public abstract class AbstractBeanFactoryPointcutAdvisor extends AbstractPointcu
5251
@Nullable
5352
private transient volatile Advice advice;
5453

55-
private transient volatile Object adviceMonitor = new Object();
54+
private transient Object adviceMonitor = new Object();
5655

5756

5857
/**
@@ -78,16 +77,6 @@ public String getAdviceBeanName() {
7877
@Override
7978
public void setBeanFactory(BeanFactory beanFactory) {
8079
this.beanFactory = beanFactory;
81-
resetAdviceMonitor();
82-
}
83-
84-
private void resetAdviceMonitor() {
85-
if (this.beanFactory instanceof ConfigurableBeanFactory cbf) {
86-
this.adviceMonitor = cbf.getSingletonMutex();
87-
}
88-
else {
89-
this.adviceMonitor = new Object();
90-
}
9180
}
9281

9382
/**
@@ -118,9 +107,7 @@ public Advice getAdvice() {
118107
return advice;
119108
}
120109
else {
121-
// No singleton guarantees from the factory -> let's lock locally but
122-
// reuse the factory's singleton lock, just in case a lazy dependency
123-
// of our advice bean happens to trigger the singleton lock implicitly...
110+
// No singleton guarantees from the factory -> let's lock locally.
124111
synchronized (this.adviceMonitor) {
125112
advice = this.advice;
126113
if (advice == null) {
@@ -155,7 +142,7 @@ private void readObject(ObjectInputStream ois) throws IOException, ClassNotFound
155142
ois.defaultReadObject();
156143

157144
// Initialize transient fields.
158-
resetAdviceMonitor();
145+
this.adviceMonitor = new Object();
159146
}
160147

161148
}

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

+4-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2015 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -129,7 +129,10 @@ public interface SingletonBeanRegistry {
129129
* Return the singleton mutex used by this registry (for external collaborators).
130130
* @return the mutex object (never {@code null})
131131
* @since 4.2
132+
* @deprecated as of 6.2, in favor of lenient singleton locking
133+
* (with this method returning an arbitrary object to lock on)
132134
*/
135+
@Deprecated(since = "6.2")
133136
Object getSingletonMutex();
134137

135138
}

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

+46-55
Original file line numberDiff line numberDiff line change
@@ -972,59 +972,54 @@ protected Object getEarlyBeanReference(String beanName, RootBeanDefinition mbd,
972972
*/
973973
@Nullable
974974
private FactoryBean<?> getSingletonFactoryBeanForTypeCheck(String beanName, RootBeanDefinition mbd) {
975-
synchronized (getSingletonMutex()) {
976-
BeanWrapper bw = this.factoryBeanInstanceCache.get(beanName);
977-
if (bw != null) {
978-
return (FactoryBean<?>) bw.getWrappedInstance();
979-
}
980-
Object beanInstance = getSingleton(beanName, false);
981-
if (beanInstance instanceof FactoryBean<?> factoryBean) {
982-
return factoryBean;
983-
}
984-
if (isSingletonCurrentlyInCreation(beanName) ||
985-
(mbd.getFactoryBeanName() != null && isSingletonCurrentlyInCreation(mbd.getFactoryBeanName()))) {
986-
return null;
987-
}
975+
BeanWrapper bw = this.factoryBeanInstanceCache.get(beanName);
976+
if (bw != null) {
977+
return (FactoryBean<?>) bw.getWrappedInstance();
978+
}
979+
Object beanInstance = getSingleton(beanName, false);
980+
if (beanInstance instanceof FactoryBean<?> factoryBean) {
981+
return factoryBean;
982+
}
983+
if (isSingletonCurrentlyInCreation(beanName) ||
984+
(mbd.getFactoryBeanName() != null && isSingletonCurrentlyInCreation(mbd.getFactoryBeanName()))) {
985+
return null;
986+
}
988987

989-
Object instance;
990-
try {
991-
// Mark this bean as currently in creation, even if just partially.
992-
beforeSingletonCreation(beanName);
993-
// Give BeanPostProcessors a chance to return a proxy instead of the target bean instance.
994-
instance = resolveBeforeInstantiation(beanName, mbd);
995-
if (instance == null) {
996-
bw = createBeanInstance(beanName, mbd, null);
997-
instance = bw.getWrappedInstance();
998-
}
988+
Object instance;
989+
try {
990+
// Mark this bean as currently in creation, even if just partially.
991+
beforeSingletonCreation(beanName);
992+
// Give BeanPostProcessors a chance to return a proxy instead of the target bean instance.
993+
instance = resolveBeforeInstantiation(beanName, mbd);
994+
if (instance == null) {
995+
bw = createBeanInstance(beanName, mbd, null);
996+
instance = bw.getWrappedInstance();
997+
this.factoryBeanInstanceCache.put(beanName, bw);
999998
}
1000-
catch (UnsatisfiedDependencyException ex) {
1001-
// Don't swallow, probably misconfiguration...
999+
}
1000+
catch (UnsatisfiedDependencyException ex) {
1001+
// Don't swallow, probably misconfiguration...
1002+
throw ex;
1003+
}
1004+
catch (BeanCreationException ex) {
1005+
// Don't swallow a linkage error since it contains a full stacktrace on
1006+
// first occurrence... and just a plain NoClassDefFoundError afterwards.
1007+
if (ex.contains(LinkageError.class)) {
10021008
throw ex;
10031009
}
1004-
catch (BeanCreationException ex) {
1005-
// Don't swallow a linkage error since it contains a full stacktrace on
1006-
// first occurrence... and just a plain NoClassDefFoundError afterwards.
1007-
if (ex.contains(LinkageError.class)) {
1008-
throw ex;
1009-
}
1010-
// Instantiation failure, maybe too early...
1011-
if (logger.isDebugEnabled()) {
1012-
logger.debug("Bean creation exception on singleton FactoryBean type check: " + ex);
1013-
}
1014-
onSuppressedException(ex);
1015-
return null;
1016-
}
1017-
finally {
1018-
// Finished partial creation of this bean.
1019-
afterSingletonCreation(beanName);
1020-
}
1021-
1022-
FactoryBean<?> fb = getFactoryBean(beanName, instance);
1023-
if (bw != null) {
1024-
this.factoryBeanInstanceCache.put(beanName, bw);
1010+
// Instantiation failure, maybe too early...
1011+
if (logger.isDebugEnabled()) {
1012+
logger.debug("Bean creation exception on singleton FactoryBean type check: " + ex);
10251013
}
1026-
return fb;
1014+
onSuppressedException(ex);
1015+
return null;
1016+
}
1017+
finally {
1018+
// Finished partial creation of this bean.
1019+
afterSingletonCreation(beanName);
10271020
}
1021+
1022+
return getFactoryBean(beanName, instance);
10281023
}
10291024

10301025
/**
@@ -1912,21 +1907,17 @@ protected Object postProcessObjectFromFactoryBean(Object object, String beanName
19121907
*/
19131908
@Override
19141909
protected void removeSingleton(String beanName) {
1915-
synchronized (getSingletonMutex()) {
1916-
super.removeSingleton(beanName);
1917-
this.factoryBeanInstanceCache.remove(beanName);
1918-
}
1910+
super.removeSingleton(beanName);
1911+
this.factoryBeanInstanceCache.remove(beanName);
19191912
}
19201913

19211914
/**
19221915
* Overridden to clear FactoryBean instance cache as well.
19231916
*/
19241917
@Override
19251918
protected void clearSingletonCache() {
1926-
synchronized (getSingletonMutex()) {
1927-
super.clearSingletonCache();
1928-
this.factoryBeanInstanceCache.clear();
1929-
}
1919+
super.clearSingletonCache();
1920+
this.factoryBeanInstanceCache.clear();
19301921
}
19311922

19321923
/**

0 commit comments

Comments
 (0)