revised constructor argument caching for highly concurrent creation scenarios (follow-up to SPR-7423)

git-svn-id: https://src.springframework.org/svn/spring-framework/trunk@3603 50f2f4bb-b051-0410-bef5-90022cba6387

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

@@ -880,8 +880,18 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac
 		}
 
 		// Shortcut when re-creating the same bean...
-		if (mbd.resolvedConstructorOrFactoryMethod != null && args == null) {
-			if (mbd.constructorArgumentsResolved) {
+		boolean resolved = false;
+		boolean autowireNecessary = false;
+		if (args == null) {
+			synchronized (mbd.constructorArgumentLock) {
+				if (mbd.resolvedConstructorOrFactoryMethod != null) {
+					resolved = true;
+					autowireNecessary = mbd.constructorArgumentsResolved;
+				}
+			}
+		}
+		if (resolved) {
+			if (autowireNecessary) {
 				return autowireConstructor(beanName, mbd, null, null);
 			}
 			else {

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

@@ -117,14 +117,20 @@ class ConstructorResolver {
 			argsToUse = explicitArgs;
 		}
 		else {
-			constructorToUse = (Constructor) mbd.resolvedConstructorOrFactoryMethod;
-			if (constructorToUse != null) {
-				// Found a cached constructor...
-				argsToUse = mbd.resolvedConstructorArguments;
-				if (argsToUse == null) {
-					argsToUse = resolvePreparedArguments(beanName, mbd, bw, constructorToUse);
+			Object[] argsToResolve = null;
+			synchronized (mbd.constructorArgumentLock) {
+				constructorToUse = (Constructor) mbd.resolvedConstructorOrFactoryMethod;
+				if (constructorToUse != null && mbd.constructorArgumentsResolved) {
+					// Found a cached constructor...
+					argsToUse = mbd.resolvedConstructorArguments;
+					if (argsToUse == null) {
+						argsToResolve = mbd.preparedConstructorArguments;
+					}
 				}
 			}
+			if (argsToResolve != null) {
+				argsToUse = resolvePreparedArguments(beanName, mbd, bw, constructorToUse, argsToResolve);
+			}
 		}
 
 		if (constructorToUse == null) {
@@ -254,8 +260,7 @@ class ConstructorResolver {
 			}
 
 			if (explicitArgs == null) {
-				mbd.resolvedConstructorOrFactoryMethod = constructorToUse;
-				argsHolderToUse.storeCache(mbd);
+				argsHolderToUse.storeCache(mbd, constructorToUse);
 			}
 		}
 
@@ -312,7 +317,9 @@ class ConstructorResolver {
 				}
 			}
 		}
-		mbd.resolvedConstructorOrFactoryMethod = uniqueCandidate;
+		synchronized (mbd.constructorArgumentLock) {
+			mbd.resolvedConstructorOrFactoryMethod = uniqueCandidate;
+		}
 	}
 
 	/**
@@ -371,14 +378,20 @@ class ConstructorResolver {
 			argsToUse = explicitArgs;
 		}
 		else {
-			factoryMethodToUse = (Method) mbd.resolvedConstructorOrFactoryMethod;
-			if (factoryMethodToUse != null) {
-				// Found a cached factory method...
-				argsToUse = mbd.resolvedConstructorArguments;
-				if (argsToUse == null && mbd.preparedConstructorArguments != null) {
-					argsToUse = resolvePreparedArguments(beanName, mbd, bw, factoryMethodToUse);
+			Object[] argsToResolve = null;
+			synchronized (mbd.constructorArgumentLock) {
+				factoryMethodToUse = (Method) mbd.resolvedConstructorOrFactoryMethod;
+				if (factoryMethodToUse != null && mbd.constructorArgumentsResolved) {
+					// Found a cached factory method...
+					argsToUse = mbd.resolvedConstructorArguments;
+					if (argsToUse == null) {
+						argsToResolve = mbd.preparedConstructorArguments;
+					}
 				}
 			}
+			if (argsToResolve != null) {
+				argsToUse = resolvePreparedArguments(beanName, mbd, bw, factoryMethodToUse, argsToResolve);
+			}
 		}
 
 		if (factoryMethodToUse == null || argsToUse == null) {
@@ -536,8 +549,7 @@ class ConstructorResolver {
 			}
 
 			if (explicitArgs == null) {
-				mbd.resolvedConstructorOrFactoryMethod = factoryMethodToUse;
-				argsHolderToUse.storeCache(mbd);
+				argsHolderToUse.storeCache(mbd, factoryMethodToUse);
 			}
 		}
 
@@ -734,11 +746,10 @@ class ConstructorResolver {
 	 * Resolve the prepared arguments stored in the given bean definition.
 	 */
 	private Object[] resolvePreparedArguments(
-			String beanName, RootBeanDefinition mbd, BeanWrapper bw, Member methodOrCtor) {
+			String beanName, RootBeanDefinition mbd, BeanWrapper bw, Member methodOrCtor, Object[] argsToResolve) {
 
 		Class[] paramTypes = (methodOrCtor instanceof Method ?
 				((Method) methodOrCtor).getParameterTypes() : ((Constructor) methodOrCtor).getParameterTypes());
-		Object[] argsToResolve = mbd.preparedConstructorArguments;
 		TypeConverter converter = (this.beanFactory.getCustomTypeConverter() != null ?
 				this.beanFactory.getCustomTypeConverter() : bw);
 		BeanDefinitionValueResolver valueResolver =
@@ -789,11 +800,11 @@ class ConstructorResolver {
 	 */
 	private static class ArgumentsHolder {
 
-		public Object rawArguments[];
+		public final Object rawArguments[];
 
-		public Object arguments[];
+		public final Object arguments[];
 
-		public Object preparedArguments[];
+		public final Object preparedArguments[];
 
 		public boolean resolveNecessary = false;
 
@@ -833,14 +844,17 @@ class ConstructorResolver {
 			return Integer.MAX_VALUE - 1024;
 		}
 
-		public void storeCache(RootBeanDefinition mbd) {
-			if (this.resolveNecessary) {
-				mbd.preparedConstructorArguments = this.preparedArguments;
-			}
-			else {
-				mbd.resolvedConstructorArguments = this.arguments;
+		public void storeCache(RootBeanDefinition mbd, Object constructorOrFactoryMethod) {
+			synchronized (mbd.constructorArgumentLock) {
+				mbd.resolvedConstructorOrFactoryMethod = constructorOrFactoryMethod;
+				mbd.constructorArgumentsResolved = true;
+				if (this.resolveNecessary) {
+					mbd.preparedConstructorArguments = this.preparedArguments;
+				}
+				else {
+					mbd.resolvedConstructorArguments = this.arguments;
+				}
 			}
-			mbd.constructorArgumentsResolved = true;
 		}
 	}
 

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

@@ -504,8 +504,14 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto
 	 */
 	protected boolean isAutowireCandidate(String beanName, RootBeanDefinition mbd, DependencyDescriptor descriptor) {
 		resolveBeanClass(mbd, beanName);
-		if (mbd.isFactoryMethodUnique && mbd.resolvedConstructorOrFactoryMethod == null) {
-			new ConstructorResolver(this).resolveFactoryMethodIfPossible(mbd);
+		if (mbd.isFactoryMethodUnique) {
+			boolean resolve;
+			synchronized (mbd.constructorArgumentLock) {
+				resolve = (mbd.resolvedConstructorOrFactoryMethod == null);
+			}
+			if (resolve) {
+				new ConstructorResolver(this).resolveFactoryMethodIfPossible(mbd);
+			}
 		}
 		return getAutowireCandidateResolver().isAutowireCandidate(
 				new BeanDefinitionHolder(mbd, beanName, getAliases(beanName)), descriptor);

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2009 the original author or authors.
+ * Copyright 2002-2010 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.
@@ -59,16 +59,18 @@ public class RootBeanDefinition extends AbstractBeanDefinition {
 	boolean isFactoryMethodUnique;
 
 	/** Package-visible field for caching the resolved constructor or factory method */
-	volatile Object resolvedConstructorOrFactoryMethod;
+	Object resolvedConstructorOrFactoryMethod;
+
+	/** Package-visible field that marks the constructor arguments as resolved */
+	boolean constructorArgumentsResolved = false;
 
 	/** Package-visible field for caching fully resolved constructor arguments */
-	volatile Object[] resolvedConstructorArguments;
+	Object[] resolvedConstructorArguments;
 
 	/** Package-visible field for caching partly prepared constructor arguments */
-	volatile Object[] preparedConstructorArguments;
+	Object[] preparedConstructorArguments;
 
-	/** Package-visible field that marks the constructor arguments as resolved */
-	volatile boolean constructorArgumentsResolved = false;
+	final Object constructorArgumentLock = new Object();
 
 	/** Package-visible field that indicates a before-instantiation post-processor having kicked in */
 	volatile Boolean beforeInstantiationResolved;
@@ -78,6 +80,7 @@ public class RootBeanDefinition extends AbstractBeanDefinition {
 
 	final Object postProcessingLock = new Object();
 
+
 	/**
 	 * Create a new RootBeanDefinition, to be configured through its bean
 	 * properties and configuration methods.
@@ -264,8 +267,10 @@ public class RootBeanDefinition extends AbstractBeanDefinition {
 	 * @return the factory method, or <code>null</code> if not found or not resolved yet
 	 */
 	public Method getResolvedFactoryMethod() {
-		Object candidate = this.resolvedConstructorOrFactoryMethod;
-		return (candidate instanceof Method ? (Method) candidate : null);
+		synchronized (this.constructorArgumentLock) {
+			Object candidate = this.resolvedConstructorOrFactoryMethod;
+			return (candidate instanceof Method ? (Method) candidate : null);
+		}
 	}
 
 

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

@@ -45,27 +45,30 @@ public class SimpleInstantiationStrategy implements InstantiationStrategy {
 	public Object instantiate(RootBeanDefinition beanDefinition, String beanName, BeanFactory owner) {
 		// Don't override the class with CGLIB if no overrides.
 		if (beanDefinition.getMethodOverrides().isEmpty()) {
-			Constructor<?> constructorToUse = (Constructor<?>) beanDefinition.resolvedConstructorOrFactoryMethod;
-			if (constructorToUse == null) {
-				final Class clazz = beanDefinition.getBeanClass();
-				if (clazz.isInterface()) {
-					throw new BeanInstantiationException(clazz, "Specified class is an interface");
-				}
-				try {
-					if (System.getSecurityManager() != null) {
-						constructorToUse = AccessController.doPrivileged(new PrivilegedExceptionAction<Constructor>() {
-							public Constructor run() throws Exception {
-								return clazz.getDeclaredConstructor((Class[]) null);
-							}
-						});
+			Constructor<?> constructorToUse;
+			synchronized (beanDefinition.constructorArgumentLock) {
+				constructorToUse = (Constructor<?>) beanDefinition.resolvedConstructorOrFactoryMethod;
+				if (constructorToUse == null) {
+					final Class clazz = beanDefinition.getBeanClass();
+					if (clazz.isInterface()) {
+						throw new BeanInstantiationException(clazz, "Specified class is an interface");
 					}
-					else {
-						constructorToUse =	clazz.getDeclaredConstructor((Class[]) null);
+					try {
+						if (System.getSecurityManager() != null) {
+							constructorToUse = AccessController.doPrivileged(new PrivilegedExceptionAction<Constructor>() {
+								public Constructor run() throws Exception {
+									return clazz.getDeclaredConstructor((Class[]) null);
+								}
+							});
+						}
+						else {
+							constructorToUse =	clazz.getDeclaredConstructor((Class[]) null);
+						}
+						beanDefinition.resolvedConstructorOrFactoryMethod = constructorToUse;
+					}
+					catch (Exception ex) {
+						throw new BeanInstantiationException(clazz, "No default constructor found", ex);
 					}
-					beanDefinition.resolvedConstructorOrFactoryMethod = constructorToUse;
-				}
-				catch (Exception ex) {
-					throw new BeanInstantiationException(clazz, "No default constructor found", ex);
 				}
 			}
 			return BeanUtils.instantiateClass(constructorToUse);