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
(cherry picked from commit c406c56)

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;
 
@@ -144,14 +146,14 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
 	/**
 	 * 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;
 	}
 
@@ -191,9 +193,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);
 	}
 
@@ -205,8 +207,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()),
@@ -252,16 +253,15 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
 		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;
 			}
 		}
@@ -288,7 +288,15 @@ 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));
 		}
 	}
 
@@ -344,14 +352,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();
 	}
@@ -361,29 +368,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);
 	}
@@ -417,7 +409,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);
 							}
@@ -425,21 +418,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));
 					}
@@ -620,9 +614,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) {
@@ -632,35 +628,34 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
 
 		@Override
 		public boolean alwaysMatches() {
-			return primary.alwaysMatches();
+			return this.primary.alwaysMatches();
 		}
 
 		@Override
 		public boolean maybeMatches() {
-			return primary.maybeMatches();
+			return this.primary.maybeMatches();
 		}
 
 		@Override
 		public boolean neverMatches() {
-			return primary.neverMatches();
+			return this.primary.neverMatches();
 		}
 
 		@Override
-		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);
 			}
 		}
 
 		@Override
 		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);
 		}
 	}
 
@@ -149,19 +163,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);
 			}
 		}
 	}
@@ -169,9 +175,11 @@ class RuntimeTestWalker {
 
 	private static abstract class InstanceOfResidueTestVisitor extends TestVisitorAdapter {
 
-		private Class<?> matchClass;
+		private final Class<?> matchClass;
+
 		private boolean matches;
-		private int matchVarType;
+
+		private final int matchVarType;
 
 		public InstanceOfResidueTestVisitor(Class<?> matchClass, boolean defaultMatches, int matchVarType) {
 			this.matchClass = matchClass;
@@ -181,19 +189,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) {
@@ -237,8 +260,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) {
@@ -249,8 +275,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;
 			}
 		}
@@ -260,7 +286,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;
 			}
 		}