Avoid class loading during AspectJ expression evaluation

Fixed through downcasting to AspectJ's ReflectionType and ReflectionBasedReferenceTypeDelegate, obtaining the myClass field there. We only fall back to regular class loading if we encounter any other kind of type.

Issue: SPR-11344

spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2013 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.
@@ -42,6 +42,7 @@ import org.aspectj.weaver.tools.PointcutParameter;
 import org.aspectj.weaver.tools.PointcutParser;
 import org.aspectj.weaver.tools.PointcutPrimitive;
 import org.aspectj.weaver.tools.ShadowMatch;
+
 import org.springframework.aop.ClassFilter;
 import org.springframework.aop.IntroductionAwareMethodMatcher;
 import org.springframework.aop.MethodMatcher;
@@ -55,6 +56,7 @@ import org.springframework.beans.factory.BeanFactoryAware;
 import org.springframework.beans.factory.BeanFactoryUtils;
 import org.springframework.beans.factory.FactoryBean;
 import org.springframework.beans.factory.config.ConfigurableBeanFactory;
+import org.springframework.util.ClassUtils;
 import org.springframework.util.ObjectUtils;
 import org.springframework.util.StringUtils;
 
@@ -98,11 +100,11 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
 
 	private static final Log logger = LogFactory.getLog(AspectJExpressionPointcut.class);
 
-	private Class pointcutDeclarationScope;
+	private Class<?> pointcutDeclarationScope;
 
 	private String[] pointcutParameterNames = new String[0];
 
-	private Class[] pointcutParameterTypes = new Class[0];
+	private Class<?>[] pointcutParameterTypes = new Class<?>[0];
 
 	private BeanFactory beanFactory;
 
@@ -123,7 +125,7 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
 	 * @param paramNames the parameter names for the pointcut
 	 * @param paramTypes the parameter types for the pointcut
 	 */
-	public AspectJExpressionPointcut(Class declarationScope, String[] paramNames, Class[] paramTypes) {
+	public AspectJExpressionPointcut(Class<?> declarationScope, String[] paramNames, Class<?>[] paramTypes) {
 		this.pointcutDeclarationScope = declarationScope;
 		if (paramNames.length != paramTypes.length) {
 			throw new IllegalStateException(
@@ -137,21 +139,21 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
 	/**
 	 * Set the declaration scope for the pointcut.
 	 */
-	public void setPointcutDeclarationScope(Class pointcutDeclarationScope) {
+	public void setPointcutDeclarationScope(Class<?> pointcutDeclarationScope) {
 		this.pointcutDeclarationScope = pointcutDeclarationScope;
 	}
 
 	/**
 	 * Set the parameter names for the pointcut.
 	 */
-	public void setParameterNames(String[] names) {
+	public void setParameterNames(String... names) {
 		this.pointcutParameterNames = names;
 	}
 
 	/**
 	 * Set the parameter types for the pointcut.
 	 */
-	public void setParameterTypes(Class[] types) {
+	public void setParameterTypes(Class<?>... types) {
 		this.pointcutParameterTypes = types;
 	}
 
@@ -188,9 +190,9 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
 	 * Build the underlying AspectJ pointcut expression.
 	 */
 	private PointcutExpression buildPointcutExpression() {
-		ClassLoader cl = (this.beanFactory instanceof ConfigurableBeanFactory ? ((ConfigurableBeanFactory) this.beanFactory)
-				.getBeanClassLoader() : Thread.currentThread()
-				.getContextClassLoader());
+		ClassLoader cl = (this.beanFactory instanceof ConfigurableBeanFactory ?
+				((ConfigurableBeanFactory) this.beanFactory).getBeanClassLoader() :
+				ClassUtils.getDefaultClassLoader());
 		return buildPointcutExpression(cl);
 	}
 
@@ -202,8 +204,7 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
 		PointcutParameter[] pointcutParameters = new PointcutParameter[this.pointcutParameterNames.length];
 		for (int i = 0; i < pointcutParameters.length; i++) {
 			pointcutParameters[i] = parser.createPointcutParameter(
-					this.pointcutParameterNames[i],
-					this.pointcutParameterTypes[i]);
+					this.pointcutParameterNames[i], this.pointcutParameterTypes[i]);
 		}
 		return parser.parsePointcutExpression(
 				replaceBooleanOperators(getExpression()),
@@ -244,20 +245,19 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
 		return this.pointcutExpression;
 	}
 
-	public boolean matches(Class targetClass) {
+	public boolean matches(Class<?> targetClass) {
 		checkReadyToMatch();
 		try {
 			return this.pointcutExpression.couldMatchJoinPointsInType(targetClass);
-		} catch (ReflectionWorldException e) {
-			logger.debug("PointcutExpression matching rejected target class", e);
+		}
+		catch (ReflectionWorldException rwe) {
+			logger.debug("PointcutExpression matching rejected target class", rwe);
 			try {
-				// Actually this is still a "maybe" - treat the pointcut as dynamic if we
-				// don't know enough yet
+				// Actually this is still a "maybe" - treat the pointcut as dynamic if we don't know enough yet
 				return getFallbackPointcutExpression(targetClass).couldMatchJoinPointsInType(targetClass);
-			} catch (BCException ex) {
-				logger.debug(
-						"Fallback PointcutExpression matching rejected target class",
-						ex);
+			}
+			catch (BCException bce) {
+				logger.debug("Fallback PointcutExpression matching rejected target class", bce);
 				return false;
 			}
 		}
@@ -267,7 +267,7 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
 		}
 	}
 
-	public boolean matches(Method method, Class targetClass, boolean beanHasIntroductions) {
+	public boolean matches(Method method, Class<?> targetClass, boolean beanHasIntroductions) {
 		checkReadyToMatch();
 		Method targetMethod = AopUtils.getMostSpecificMethod(method, targetClass);
 		ShadowMatch shadowMatch = getShadowMatch(targetMethod, method);
@@ -283,11 +283,19 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
 		}
 		else {
 			// the maybe case
-			return (beanHasIntroductions || matchesIgnoringSubtypes(shadowMatch) || matchesTarget(shadowMatch, targetClass));
+			if (beanHasIntroductions) {
+				return true;
+			}
+			// A match test returned maybe - if there are any subtype sensitive variables
+			// involved in the test (this, target, at_this, at_target, at_annotation) then
+			// we say this is not a match as in Spring there will never be a different
+			// runtime subtype.
+			RuntimeTestWalker walker = getRuntimeTestWalker(shadowMatch);
+			return (!walker.testsSubtypeSensitiveVars() || walker.testTargetInstanceOfResidue(targetClass));
 		}
 	}
 
-	public boolean matches(Method method, Class targetClass) {
+	public boolean matches(Method method, Class<?> targetClass) {
 		return matches(method, targetClass, false);
 	}
 
@@ -296,7 +304,7 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
 		return this.pointcutExpression.mayNeedDynamicTest();
 	}
 
-	public boolean matches(Method method, Class targetClass, Object[] args) {
+	public boolean matches(Method method, Class<?> targetClass, Object[] args) {
 		checkReadyToMatch();
 		ShadowMatch shadowMatch = getShadowMatch(AopUtils.getMostSpecificMethod(method, targetClass), method);
 		ShadowMatch originalShadowMatch = getShadowMatch(method, method);
@@ -336,14 +344,13 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
 			if (!originalMethodResidueTest.testThisInstanceOfResidue(thisObject.getClass())) {
 				return false;
 			}
-		}
-		if (joinPointMatch.matches() && pmi != null) {
-			bindParameters(pmi, joinPointMatch);
+			if (joinPointMatch.matches()) {
+				bindParameters(pmi, joinPointMatch);
+			}
 		}
 		return joinPointMatch.matches();
 	}
 
-
 	protected String getCurrentProxiedBeanName() {
 		return ProxyCreationContext.getCurrentProxiedBeanName();
 	}
@@ -353,29 +360,14 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
 	 * Get a new pointcut expression based on a target class's loader, rather
 	 * than the default.
 	 */
-	private PointcutExpression getFallbackPointcutExpression(
-			Class<?> targetClass) {
+	private PointcutExpression getFallbackPointcutExpression(Class<?> targetClass) {
 		ClassLoader classLoader = targetClass.getClassLoader();
-		return classLoader == null ? this.pointcutExpression : buildPointcutExpression(classLoader);
-	}
-
-	/**
-	 * A match test returned maybe - if there are any subtype sensitive variables
-	 * involved in the test (this, target, at_this, at_target, at_annotation) then
-	 * we say this is not a match as in Spring there will never be a different
-	 * runtime subtype.
-	 */
-	private boolean matchesIgnoringSubtypes(ShadowMatch shadowMatch) {
-		return !(getRuntimeTestWalker(shadowMatch).testsSubtypeSensitiveVars());
-	}
-
-	private boolean matchesTarget(ShadowMatch shadowMatch, Class targetClass) {
-		return getRuntimeTestWalker(shadowMatch).testTargetInstanceOfResidue(targetClass);
+		return (classLoader != null ? buildPointcutExpression(classLoader) : this.pointcutExpression);
 	}
 
 	private RuntimeTestWalker getRuntimeTestWalker(ShadowMatch shadowMatch) {
 		if (shadowMatch instanceof DefensiveShadowMatch) {
-			return new RuntimeTestWalker(((DefensiveShadowMatch)shadowMatch).primary);
+			return new RuntimeTestWalker(((DefensiveShadowMatch) shadowMatch).primary);
 		}
 		return new RuntimeTestWalker(shadowMatch);
 	}
@@ -409,7 +401,8 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
 						try {
 							fallbackPointcutExpression = getFallbackPointcutExpression(methodToMatch.getDeclaringClass());
 							shadowMatch = fallbackPointcutExpression.matchesMethodExecution(methodToMatch);
-						} catch (ReflectionWorld.ReflectionWorldException e) {
+						}
+						catch (ReflectionWorld.ReflectionWorldException ex2) {
 							if (targetMethod == originalMethod) {
 								shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null);
 							}
@@ -417,21 +410,22 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
 								try {
 									shadowMatch = this.pointcutExpression.matchesMethodExecution(originalMethod);
 								}
-								catch (ReflectionWorld.ReflectionWorldException ex2) {
+								catch (ReflectionWorld.ReflectionWorldException ex3) {
 									// Could neither introspect the target class nor the proxy class ->
 									// let's simply consider this method as non-matching.
 									methodToMatch = originalMethod;
 									fallbackPointcutExpression = getFallbackPointcutExpression(methodToMatch.getDeclaringClass());
 									try {
 										shadowMatch = fallbackPointcutExpression.matchesMethodExecution(methodToMatch);
-									} catch (ReflectionWorld.ReflectionWorldException e2) {
+									}
+									catch (ReflectionWorld.ReflectionWorldException ex4) {
 										shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null);
 									}
 								}
 							}
 						}
 					}
-					if (shadowMatch.maybeMatches() && fallbackPointcutExpression!=null) {
+					if (shadowMatch.maybeMatches() && fallbackPointcutExpression != null) {
 						shadowMatch = new DefensiveShadowMatch(shadowMatch,
 								fallbackPointcutExpression.matchesMethodExecution(methodToMatch));
 					}
@@ -601,9 +595,11 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
 		this.shadowMatchCache = new ConcurrentHashMap<Method, ShadowMatch>(32);
 	}
 
+
 	private static class DefensiveShadowMatch implements ShadowMatch {
 
 		private final ShadowMatch primary;
+
 		private final ShadowMatch other;
 
 		public DefensiveShadowMatch(ShadowMatch primary, ShadowMatch other) {
@@ -612,31 +608,30 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
 		}
 
 		public boolean alwaysMatches() {
-			return primary.alwaysMatches();
+			return this.primary.alwaysMatches();
 		}
 
 		public boolean maybeMatches() {
-			return primary.maybeMatches();
+			return this.primary.maybeMatches();
 		}
 
 		public boolean neverMatches() {
-			return primary.neverMatches();
+			return this.primary.neverMatches();
 		}
 
-		public JoinPointMatch matchesJoinPoint(Object thisObject,
-				Object targetObject, Object[] args) {
+		public JoinPointMatch matchesJoinPoint(Object thisObject, Object targetObject, Object[] args) {
 			try {
-				return primary.matchesJoinPoint(thisObject, targetObject, args);
-			} catch (ReflectionWorldException e) {
-				return other.matchesJoinPoint(thisObject, targetObject, args);
+				return this.primary.matchesJoinPoint(thisObject, targetObject, args);
+			}
+			catch (ReflectionWorldException ex) {
+				return this.other.matchesJoinPoint(thisObject, targetObject, args);
 			}
 		}
 
 		public void setMatchingContext(MatchingContext aMatchContext) {
-			primary.setMatchingContext(aMatchContext);
-			other.setMatchingContext(aMatchContext);
+			this.primary.setMatchingContext(aMatchContext);
+			this.other.setMatchingContext(aMatchContext);
 		}
-
 	}
 
 }

spring-aop/src/main/java/org/springframework/aop/aspectj/RuntimeTestWalker.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2013 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.
@@ -18,6 +18,8 @@ package org.springframework.aop.aspectj;
 
 import java.lang.reflect.Field;
 
+import org.aspectj.weaver.ReferenceType;
+import org.aspectj.weaver.ReferenceTypeDelegate;
 import org.aspectj.weaver.ResolvedType;
 import org.aspectj.weaver.ast.And;
 import org.aspectj.weaver.ast.Call;
@@ -30,6 +32,7 @@ import org.aspectj.weaver.ast.Not;
 import org.aspectj.weaver.ast.Or;
 import org.aspectj.weaver.ast.Test;
 import org.aspectj.weaver.internal.tools.MatchingContextBasedTest;
+import org.aspectj.weaver.reflect.ReflectionBasedReferenceTypeDelegate;
 import org.aspectj.weaver.reflect.ReflectionVar;
 import org.aspectj.weaver.reflect.ShadowMatchImpl;
 import org.aspectj.weaver.tools.ShadowMatch;
@@ -55,25 +58,36 @@ import org.springframework.util.ReflectionUtils;
  */
 class RuntimeTestWalker {
 
-	private final Test runtimeTest;
+	private static final Field residualTestField;
 
+	private static final Field varTypeField;
 
-	public RuntimeTestWalker(ShadowMatch shadowMatch) {
-		ShadowMatchImpl shadowMatchImplementation = (ShadowMatchImpl) shadowMatch;
+	private static final Field myClassField;
+
+
+	static {
 		try {
-			Field testField = shadowMatchImplementation.getClass().getDeclaredField("residualTest");
-			ReflectionUtils.makeAccessible(testField);
-			this.runtimeTest = (Test) testField.get(shadowMatch);
+			residualTestField = ShadowMatchImpl.class.getDeclaredField("residualTest");
+			varTypeField = ReflectionVar.class.getDeclaredField("varType");
+			myClassField = ReflectionBasedReferenceTypeDelegate.class.getDeclaredField("myClass");
 		}
-		catch (NoSuchFieldException noSuchFieldEx) {
+		catch (NoSuchFieldException ex) {
 			throw new IllegalStateException("The version of aspectjtools.jar / aspectjweaver.jar " +
-					"on the classpath is incompatible with this version of Spring: Expected field " +
-					"'runtimeTest' is not present on ShadowMatchImpl class.");
+					"on the classpath is incompatible with this version of Spring: " + ex);
+		}
+	}
+
+
+	private final Test runtimeTest;
+
+
+	public RuntimeTestWalker(ShadowMatch shadowMatch) {
+		try {
+			ReflectionUtils.makeAccessible(residualTestField);
+			this.runtimeTest = (Test) residualTestField.get(shadowMatch);
 		}
-		catch (IllegalAccessException illegalAccessEx) {
-			// Famous last words... but I don't see how this can happen given the
-			// makeAccessible call above
-			throw new IllegalStateException("Unable to access ShadowMatchImpl.residualTest field");
+		catch (IllegalAccessException ex) {
+			throw new IllegalStateException(ex);
 		}
 	}
 
@@ -87,12 +101,12 @@ class RuntimeTestWalker {
 				new SubtypeSensitiveVarTypeTestVisitor().testsSubtypeSensitiveVars(this.runtimeTest));
 	}
 
-	public boolean testThisInstanceOfResidue(Class thisClass) {
+	public boolean testThisInstanceOfResidue(Class<?> thisClass) {
 		return (this.runtimeTest != null &&
 				new ThisInstanceOfResidueTestVisitor(thisClass).thisInstanceOfMatches(this.runtimeTest));
 	}
 
-	public boolean testTargetInstanceOfResidue(Class targetClass) {
+	public boolean testTargetInstanceOfResidue(Class<?> targetClass) {
 		return (this.runtimeTest != null &&
 				new TargetInstanceOfResidueTestVisitor(targetClass).targetInstanceOfMatches(this.runtimeTest));
 	}
@@ -140,19 +154,11 @@ class RuntimeTestWalker {
 
 		protected int getVarType(ReflectionVar v) {
 			try {
-				Field varTypeField = ReflectionVar.class.getDeclaredField("varType");
 				ReflectionUtils.makeAccessible(varTypeField);
 				return (Integer) varTypeField.get(v);
 			}
-			catch (NoSuchFieldException noSuchFieldEx) {
-				throw new IllegalStateException("the version of aspectjtools.jar / aspectjweaver.jar " +
-						"on the classpath is incompatible with this version of Spring:- expected field " +
-						"'varType' is not present on ReflectionVar class");
-			}
-			catch (IllegalAccessException illegalAccessEx) {
-				// Famous last words... but I don't see how this can happen given the
-				// makeAccessible call above
-				throw new IllegalStateException("Unable to access ReflectionVar.varType field");
+			catch (IllegalAccessException ex) {
+				throw new IllegalStateException(ex);
 			}
 		}
 	}
@@ -160,11 +166,13 @@ class RuntimeTestWalker {
 
 	private static abstract class InstanceOfResidueTestVisitor extends TestVisitorAdapter {
 
-		private Class matchClass;
+		private final Class<?> matchClass;
+
 		private boolean matches;
-		private int matchVarType;
 
-		public InstanceOfResidueTestVisitor(Class matchClass, boolean defaultMatches, int matchVarType) {
+		private final int matchVarType;
+
+		public InstanceOfResidueTestVisitor(Class<?> matchClass, boolean defaultMatches, int matchVarType) {
 			this.matchClass = matchClass;
 			this.matches = defaultMatches;
 			this.matchVarType = matchVarType;
@@ -172,19 +180,34 @@ class RuntimeTestWalker {
 
 		public boolean instanceOfMatches(Test test) {
 			test.accept(this);
-			return matches;
+			return this.matches;
 		}
 
 		@Override
 		public void visit(Instanceof i) {
-			ResolvedType type = (ResolvedType) i.getType();
 			int varType = getVarType((ReflectionVar) i.getVar());
 			if (varType != this.matchVarType) {
 				return;
 			}
+			Class<?> typeClass = null;
+			ResolvedType type = (ResolvedType) i.getType();
+			if (type instanceof ReferenceType) {
+				ReferenceTypeDelegate delegate = ((ReferenceType) type).getDelegate();
+				if (delegate instanceof ReflectionBasedReferenceTypeDelegate) {
+					try {
+						ReflectionUtils.makeAccessible(myClassField);
+						typeClass = (Class<?>) myClassField.get(delegate);
+					}
+					catch (IllegalAccessException ex) {
+						throw new IllegalStateException(ex);
+					}
+				}
+			}
 			try {
-				Class typeClass = ClassUtils.forName(type.getName(), this.matchClass.getClassLoader());
-				// Don't use ReflectionType.isAssignableFrom() as it won't be aware of (Spring) mixins
+				// Don't use ResolvedType.isAssignableFrom() as it won't be aware of (Spring) mixins
+				if (typeClass == null) {
+					typeClass = ClassUtils.forName(type.getName(), this.matchClass.getClassLoader());
+				}
 				this.matches = typeClass.isAssignableFrom(this.matchClass);
 			}
 			catch (ClassNotFoundException ex) {
@@ -199,7 +222,7 @@ class RuntimeTestWalker {
 	 */
 	private static class TargetInstanceOfResidueTestVisitor extends InstanceOfResidueTestVisitor {
 
-		public TargetInstanceOfResidueTestVisitor(Class targetClass) {
+		public TargetInstanceOfResidueTestVisitor(Class<?> targetClass) {
 			super(targetClass, false, TARGET_VAR);
 		}
 
@@ -214,7 +237,7 @@ class RuntimeTestWalker {
 	 */
 	private static class ThisInstanceOfResidueTestVisitor extends InstanceOfResidueTestVisitor {
 
-		public ThisInstanceOfResidueTestVisitor(Class thisClass) {
+		public ThisInstanceOfResidueTestVisitor(Class<?> thisClass) {
 			super(thisClass, true, THIS_VAR);
 		}
 
@@ -228,8 +251,11 @@ class RuntimeTestWalker {
 	private static class SubtypeSensitiveVarTypeTestVisitor extends TestVisitorAdapter {
 
 		private final Object thisObj = new Object();
+
 		private final Object targetObj = new Object();
+
 		private final Object[] argsObjs = new Object[0];
+
 		private boolean testsSubtypeSensitiveVars = false;
 
 		public boolean testsSubtypeSensitiveVars(Test aTest) {
@@ -240,8 +266,8 @@ class RuntimeTestWalker {
 		@Override
 		public void visit(Instanceof i) {
 			ReflectionVar v = (ReflectionVar) i.getVar();
-			Object varUnderTest = v.getBindingAtJoinPoint(thisObj,targetObj,argsObjs);
-			if ((varUnderTest == thisObj) || (varUnderTest == targetObj)) {
+			Object varUnderTest = v.getBindingAtJoinPoint(this.thisObj, this.targetObj, this.argsObjs);
+			if (varUnderTest == this.thisObj || varUnderTest == this.targetObj) {
 				this.testsSubtypeSensitiveVars = true;
 			}
 		}
@@ -251,7 +277,7 @@ class RuntimeTestWalker {
 			// If you thought things were bad before, now we sink to new levels of horror...
 			ReflectionVar v = (ReflectionVar) hasAnn.getVar();
 			int varType = getVarType(v);
-				if ((varType == AT_THIS_VAR) || (varType == AT_TARGET_VAR) || (varType == AT_ANNOTATION_VAR)) {
+			if (varType == AT_THIS_VAR || varType == AT_TARGET_VAR || varType == AT_ANNOTATION_VAR) {
 				this.testsSubtypeSensitiveVars = true;
 			}
 		}