Skip to content

Commit 9857ba0

Browse files
jhoellerjhoeller
committed
revised constructor argument caching for highly concurrent creation scenarios (follow-up to SPR-7423)
1 parent 9a088b8 commit 9857ba0

File tree

5 files changed

+98
-60
lines changed

5 files changed

+98
-60
lines changed

‎org.springframework.beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -880,8 +880,18 @@ protected BeanWrapper createBeanInstance(String beanName, RootBeanDefinition mbd
880880
}
881881

882882
// Shortcut when re-creating the same bean...
883-
if (mbd.resolvedConstructorOrFactoryMethod != null && args == null) {
884-
if (mbd.constructorArgumentsResolved) {
883+
boolean resolved = false;
884+
boolean autowireNecessary = false;
885+
if (args == null) {
886+
synchronized (mbd.constructorArgumentLock) {
887+
if (mbd.resolvedConstructorOrFactoryMethod != null) {
888+
resolved = true;
889+
autowireNecessary = mbd.constructorArgumentsResolved;
890+
}
891+
}
892+
}
893+
if (resolved) {
894+
if (autowireNecessary) {
885895
return autowireConstructor(beanName, mbd, null, null);
886896
}
887897
else {

‎org.springframework.beans/src/main/java/org/springframework/beans/factory/support/ConstructorResolver.java

Lines changed: 43 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -117,14 +117,20 @@ public BeanWrapper autowireConstructor(
117117
argsToUse = explicitArgs;
118118
}
119119
else {
120-
constructorToUse = (Constructor) mbd.resolvedConstructorOrFactoryMethod;
121-
if (constructorToUse != null) {
122-
// Found a cached constructor...
123-
argsToUse = mbd.resolvedConstructorArguments;
124-
if (argsToUse == null) {
125-
argsToUse = resolvePreparedArguments(beanName, mbd, bw, constructorToUse);
120+
Object[] argsToResolve = null;
121+
synchronized (mbd.constructorArgumentLock) {
122+
constructorToUse = (Constructor) mbd.resolvedConstructorOrFactoryMethod;
123+
if (constructorToUse != null && mbd.constructorArgumentsResolved) {
124+
// Found a cached constructor...
125+
argsToUse = mbd.resolvedConstructorArguments;
126+
if (argsToUse == null) {
127+
argsToResolve = mbd.preparedConstructorArguments;
128+
}
126129
}
127130
}
131+
if (argsToResolve != null) {
132+
argsToUse = resolvePreparedArguments(beanName, mbd, bw, constructorToUse, argsToResolve);
133+
}
128134
}
129135

130136
if (constructorToUse == null) {
@@ -254,8 +260,7 @@ else if (ambiguousConstructors != null && !mbd.isLenientConstructorResolution())
254260
}
255261

256262
if (explicitArgs == null) {
257-
mbd.resolvedConstructorOrFactoryMethod = constructorToUse;
258-
argsHolderToUse.storeCache(mbd);
263+
argsHolderToUse.storeCache(mbd, constructorToUse);
259264
}
260265
}
261266

@@ -312,7 +317,9 @@ else if (!Arrays.equals(uniqueCandidate.getParameterTypes(), candidate.getParame
312317
}
313318
}
314319
}
315-
mbd.resolvedConstructorOrFactoryMethod = uniqueCandidate;
320+
synchronized (mbd.constructorArgumentLock) {
321+
mbd.resolvedConstructorOrFactoryMethod = uniqueCandidate;
322+
}
316323
}
317324

318325
/**
@@ -371,14 +378,20 @@ public BeanWrapper instantiateUsingFactoryMethod(final String beanName, final Ro
371378
argsToUse = explicitArgs;
372379
}
373380
else {
374-
factoryMethodToUse = (Method) mbd.resolvedConstructorOrFactoryMethod;
375-
if (factoryMethodToUse != null) {
376-
// Found a cached factory method...
377-
argsToUse = mbd.resolvedConstructorArguments;
378-
if (argsToUse == null && mbd.preparedConstructorArguments != null) {
379-
argsToUse = resolvePreparedArguments(beanName, mbd, bw, factoryMethodToUse);
381+
Object[] argsToResolve = null;
382+
synchronized (mbd.constructorArgumentLock) {
383+
factoryMethodToUse = (Method) mbd.resolvedConstructorOrFactoryMethod;
384+
if (factoryMethodToUse != null && mbd.constructorArgumentsResolved) {
385+
// Found a cached factory method...
386+
argsToUse = mbd.resolvedConstructorArguments;
387+
if (argsToUse == null) {
388+
argsToResolve = mbd.preparedConstructorArguments;
389+
}
380390
}
381391
}
392+
if (argsToResolve != null) {
393+
argsToUse = resolvePreparedArguments(beanName, mbd, bw, factoryMethodToUse, argsToResolve);
394+
}
382395
}
383396

384397
if (factoryMethodToUse == null || argsToUse == null) {
@@ -536,8 +549,7 @@ else if (ambiguousFactoryMethods != null && !mbd.isLenientConstructorResolution(
536549
}
537550

538551
if (explicitArgs == null) {
539-
mbd.resolvedConstructorOrFactoryMethod = factoryMethodToUse;
540-
argsHolderToUse.storeCache(mbd);
552+
argsHolderToUse.storeCache(mbd, factoryMethodToUse);
541553
}
542554
}
543555

@@ -734,11 +746,10 @@ private ArgumentsHolder createArgumentArray(
734746
* Resolve the prepared arguments stored in the given bean definition.
735747
*/
736748
private Object[] resolvePreparedArguments(
737-
String beanName, RootBeanDefinition mbd, BeanWrapper bw, Member methodOrCtor) {
749+
String beanName, RootBeanDefinition mbd, BeanWrapper bw, Member methodOrCtor, Object[] argsToResolve) {
738750

739751
Class[] paramTypes = (methodOrCtor instanceof Method ?
740752
((Method) methodOrCtor).getParameterTypes() : ((Constructor) methodOrCtor).getParameterTypes());
741-
Object[] argsToResolve = mbd.preparedConstructorArguments;
742753
TypeConverter converter = (this.beanFactory.getCustomTypeConverter() != null ?
743754
this.beanFactory.getCustomTypeConverter() : bw);
744755
BeanDefinitionValueResolver valueResolver =
@@ -789,11 +800,11 @@ protected Object resolveAutowiredArgument(
789800
*/
790801
private static class ArgumentsHolder {
791802

792-
public Object rawArguments[];
803+
public final Object rawArguments[];
793804

794-
public Object arguments[];
805+
public final Object arguments[];
795806

796-
public Object preparedArguments[];
807+
public final Object preparedArguments[];
797808

798809
public boolean resolveNecessary = false;
799810

@@ -833,14 +844,17 @@ public int getAssignabilityWeight(Class[] paramTypes) {
833844
return Integer.MAX_VALUE - 1024;
834845
}
835846

836-
public void storeCache(RootBeanDefinition mbd) {
837-
if (this.resolveNecessary) {
838-
mbd.preparedConstructorArguments = this.preparedArguments;
839-
}
840-
else {
841-
mbd.resolvedConstructorArguments = this.arguments;
847+
public void storeCache(RootBeanDefinition mbd, Object constructorOrFactoryMethod) {
848+
synchronized (mbd.constructorArgumentLock) {
849+
mbd.resolvedConstructorOrFactoryMethod = constructorOrFactoryMethod;
850+
mbd.constructorArgumentsResolved = true;
851+
if (this.resolveNecessary) {
852+
mbd.preparedConstructorArguments = this.preparedArguments;
853+
}
854+
else {
855+
mbd.resolvedConstructorArguments = this.arguments;
856+
}
842857
}
843-
mbd.constructorArgumentsResolved = true;
844858
}
845859
}
846860

‎org.springframework.beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -504,8 +504,14 @@ else if (getParentBeanFactory() instanceof ConfigurableListableBeanFactory) {
504504
*/
505505
protected boolean isAutowireCandidate(String beanName, RootBeanDefinition mbd, DependencyDescriptor descriptor) {
506506
resolveBeanClass(mbd, beanName);
507-
if (mbd.isFactoryMethodUnique && mbd.resolvedConstructorOrFactoryMethod == null) {
508-
new ConstructorResolver(this).resolveFactoryMethodIfPossible(mbd);
507+
if (mbd.isFactoryMethodUnique) {
508+
boolean resolve;
509+
synchronized (mbd.constructorArgumentLock) {
510+
resolve = (mbd.resolvedConstructorOrFactoryMethod == null);
511+
}
512+
if (resolve) {
513+
new ConstructorResolver(this).resolveFactoryMethodIfPossible(mbd);
514+
}
509515
}
510516
return getAutowireCandidateResolver().isAutowireCandidate(
511517
new BeanDefinitionHolder(mbd, beanName, getAliases(beanName)), descriptor);

‎org.springframework.beans/src/main/java/org/springframework/beans/factory/support/RootBeanDefinition.java

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2009 the original author or authors.
2+
* Copyright 2002-2010 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.
@@ -59,16 +59,18 @@ public class RootBeanDefinition extends AbstractBeanDefinition {
5959
boolean isFactoryMethodUnique;
6060

6161
/** Package-visible field for caching the resolved constructor or factory method */
62-
volatile Object resolvedConstructorOrFactoryMethod;
62+
Object resolvedConstructorOrFactoryMethod;
63+
64+
/** Package-visible field that marks the constructor arguments as resolved */
65+
boolean constructorArgumentsResolved = false;
6366

6467
/** Package-visible field for caching fully resolved constructor arguments */
65-
volatile Object[] resolvedConstructorArguments;
68+
Object[] resolvedConstructorArguments;
6669

6770
/** Package-visible field for caching partly prepared constructor arguments */
68-
volatile Object[] preparedConstructorArguments;
71+
Object[] preparedConstructorArguments;
6972

70-
/** Package-visible field that marks the constructor arguments as resolved */
71-
volatile boolean constructorArgumentsResolved = false;
73+
final Object constructorArgumentLock = new Object();
7274

7375
/** Package-visible field that indicates a before-instantiation post-processor having kicked in */
7476
volatile Boolean beforeInstantiationResolved;
@@ -78,6 +80,7 @@ public class RootBeanDefinition extends AbstractBeanDefinition {
7880

7981
final Object postProcessingLock = new Object();
8082

83+
8184
/**
8285
* Create a new RootBeanDefinition, to be configured through its bean
8386
* properties and configuration methods.
@@ -264,8 +267,10 @@ public boolean isFactoryMethod(Method candidate) {
264267
* @return the factory method, or <code>null</code> if not found or not resolved yet
265268
*/
266269
public Method getResolvedFactoryMethod() {
267-
Object candidate = this.resolvedConstructorOrFactoryMethod;
268-
return (candidate instanceof Method ? (Method) candidate : null);
270+
synchronized (this.constructorArgumentLock) {
271+
Object candidate = this.resolvedConstructorOrFactoryMethod;
272+
return (candidate instanceof Method ? (Method) candidate : null);
273+
}
269274
}
270275

271276

‎org.springframework.beans/src/main/java/org/springframework/beans/factory/support/SimpleInstantiationStrategy.java

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -45,27 +45,30 @@ public class SimpleInstantiationStrategy implements InstantiationStrategy {
4545
public Object instantiate(RootBeanDefinition beanDefinition, String beanName, BeanFactory owner) {
4646
// Don't override the class with CGLIB if no overrides.
4747
if (beanDefinition.getMethodOverrides().isEmpty()) {
48-
Constructor<?> constructorToUse = (Constructor<?>) beanDefinition.resolvedConstructorOrFactoryMethod;
49-
if (constructorToUse == null) {
50-
final Class clazz = beanDefinition.getBeanClass();
51-
if (clazz.isInterface()) {
52-
throw new BeanInstantiationException(clazz, "Specified class is an interface");
53-
}
54-
try {
55-
if (System.getSecurityManager() != null) {
56-
constructorToUse = AccessController.doPrivileged(new PrivilegedExceptionAction<Constructor>() {
57-
public Constructor run() throws Exception {
58-
return clazz.getDeclaredConstructor((Class[]) null);
59-
}
60-
});
48+
Constructor<?> constructorToUse;
49+
synchronized (beanDefinition.constructorArgumentLock) {
50+
constructorToUse = (Constructor<?>) beanDefinition.resolvedConstructorOrFactoryMethod;
51+
if (constructorToUse == null) {
52+
final Class clazz = beanDefinition.getBeanClass();
53+
if (clazz.isInterface()) {
54+
throw new BeanInstantiationException(clazz, "Specified class is an interface");
6155
}
62-
else {
63-
constructorToUse = clazz.getDeclaredConstructor((Class[]) null);
56+
try {
57+
if (System.getSecurityManager() != null) {
58+
constructorToUse = AccessController.doPrivileged(new PrivilegedExceptionAction<Constructor>() {
59+
public Constructor run() throws Exception {
60+
return clazz.getDeclaredConstructor((Class[]) null);
61+
}
62+
});
63+
}
64+
else {
65+
constructorToUse = clazz.getDeclaredConstructor((Class[]) null);
66+
}
67+
beanDefinition.resolvedConstructorOrFactoryMethod = constructorToUse;
68+
}
69+
catch (Exception ex) {
70+
throw new BeanInstantiationException(clazz, "No default constructor found", ex);
6471
}
65-
beanDefinition.resolvedConstructorOrFactoryMethod = constructorToUse;
66-
}
67-
catch (Exception ex) {
68-
throw new BeanInstantiationException(clazz, "No default constructor found", ex);
6972
}
7073
}
7174
return BeanUtils.instantiateClass(constructorToUse);

0 commit comments

Comments
 (0)