Skip to content

Commit ecd3dd8

Browse files
committed
Consistent local synchronization in getObjectFromFactoryBean
Closes gh-35545
1 parent 332953c commit ecd3dd8

File tree

1 file changed

+34
-39
lines changed

1 file changed

+34
-39
lines changed

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

Lines changed: 34 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -128,53 +128,48 @@ protected Object getObjectFromFactoryBean(FactoryBean<?> factory, String beanNam
128128
locked = (lockFlag && this.singletonLock.tryLock());
129129
}
130130
try {
131-
Object object = this.factoryBeanObjectCache.get(beanName);
132-
if (object == null) {
133-
if (locked) {
134-
// The common case: within general singleton lock.
131+
// Defensively synchronize against non-thread-safe FactoryBean.getObject() implementations,
132+
// potentially to be called from a background thread while the main thread currently calls
133+
// the same getObject() method within the singleton lock.
134+
synchronized (factory) {
135+
Object object = this.factoryBeanObjectCache.get(beanName);
136+
if (object == null) {
135137
object = doGetObjectFromFactoryBean(factory, beanName);
136-
}
137-
else {
138-
// Fall back to local synchronization on the given FactoryBean instance,
139-
// as a defensive measure for non-thread-safe FactoryBean implementations.
140-
synchronized (factory) {
141-
object = doGetObjectFromFactoryBean(factory, beanName);
138+
// Only post-process and store if not put there already during getObject() call above
139+
// (for example, because of circular reference processing triggered by custom getBean calls)
140+
Object alreadyThere = this.factoryBeanObjectCache.get(beanName);
141+
if (alreadyThere != null) {
142+
object = alreadyThere;
142143
}
143-
}
144-
// Only post-process and store if not put there already during getObject() call above
145-
// (for example, because of circular reference processing triggered by custom getBean calls)
146-
Object alreadyThere = this.factoryBeanObjectCache.get(beanName);
147-
if (alreadyThere != null) {
148-
object = alreadyThere;
149-
}
150-
else {
151-
if (shouldPostProcess) {
152-
if (locked) {
153-
if (isSingletonCurrentlyInCreation(beanName)) {
154-
// Temporarily return non-post-processed object, not storing it yet
155-
return object;
156-
}
157-
beforeSingletonCreation(beanName);
158-
}
159-
try {
160-
object = postProcessObjectFromFactoryBean(object, beanName);
161-
}
162-
catch (Throwable ex) {
163-
throw new BeanCreationException(beanName,
164-
"Post-processing of FactoryBean's singleton object failed", ex);
165-
}
166-
finally {
144+
else {
145+
if (shouldPostProcess) {
167146
if (locked) {
168-
afterSingletonCreation(beanName);
147+
if (isSingletonCurrentlyInCreation(beanName)) {
148+
// Temporarily return non-post-processed object, not storing it yet
149+
return object;
150+
}
151+
beforeSingletonCreation(beanName);
152+
}
153+
try {
154+
object = postProcessObjectFromFactoryBean(object, beanName);
155+
}
156+
catch (Throwable ex) {
157+
throw new BeanCreationException(beanName,
158+
"Post-processing of FactoryBean's singleton object failed", ex);
159+
}
160+
finally {
161+
if (locked) {
162+
afterSingletonCreation(beanName);
163+
}
169164
}
170165
}
171-
}
172-
if (containsSingleton(beanName)) {
173-
this.factoryBeanObjectCache.put(beanName, object);
166+
if (containsSingleton(beanName)) {
167+
this.factoryBeanObjectCache.put(beanName, object);
168+
}
174169
}
175170
}
171+
return object;
176172
}
177-
return object;
178173
}
179174
finally {
180175
if (locked) {

0 commit comments

Comments
 (0)