Consistent ClassLoader propagation and ConcurrentHashMap setup for AspectJ pointcuts

Issue: SPR-15040
This commit is contained in:
Juergen Hoeller 2016-12-22 17:00:04 +01:00
parent 0d0d461903
commit d64d9ab370
8 changed files with 141 additions and 134 deletions

View File

@ -187,13 +187,24 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
throw new IllegalStateException("Must set property 'expression' before attempting to match");
}
if (this.pointcutExpression == null) {
this.pointcutClassLoader = (this.beanFactory instanceof ConfigurableBeanFactory ?
((ConfigurableBeanFactory) this.beanFactory).getBeanClassLoader() :
ClassUtils.getDefaultClassLoader());
this.pointcutClassLoader = determinePointcutClassLoader();
this.pointcutExpression = buildPointcutExpression(this.pointcutClassLoader);
}
}
/**
* Determine the ClassLoader to use for pointcut evaluation.
*/
private ClassLoader determinePointcutClassLoader() {
if (this.beanFactory instanceof ConfigurableBeanFactory) {
return ((ConfigurableBeanFactory) this.beanFactory).getBeanClassLoader();
}
if (this.pointcutDeclarationScope != null) {
return this.pointcutDeclarationScope.getClassLoader();
}
return ClassUtils.getDefaultClassLoader();
}
/**
* Build the underlying AspectJ pointcut expression.
*/
@ -211,10 +222,10 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
/**
* Initialize the underlying AspectJ pointcut parser.
*/
private PointcutParser initializePointcutParser(ClassLoader cl) {
private PointcutParser initializePointcutParser(ClassLoader classLoader) {
PointcutParser parser = PointcutParser
.getPointcutParserSupportingSpecifiedPrimitivesAndUsingSpecifiedClassLoaderForResolution(
SUPPORTED_PRIMITIVES, cl);
SUPPORTED_PRIMITIVES, classLoader);
parser.registerPointcutDesignatorHandler(new BeanPointcutDesignatorHandler());
return parser;
}
@ -608,7 +619,8 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
}
private boolean matchesBean(String advisedBeanName) {
return BeanFactoryAnnotationUtils.isQualifierMatch(this.expressionPattern::matches, advisedBeanName, beanFactory);
return BeanFactoryAnnotationUtils.isQualifierMatch(
this.expressionPattern::matches, advisedBeanName, beanFactory);
}
}

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2014 the original author or authors.
* Copyright 2002-2016 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 org.springframework.aop.Pointcut;
import org.springframework.aop.support.AbstractGenericPointcutAdvisor;
import org.springframework.beans.factory.BeanFactory;
import org.springframework.beans.factory.BeanFactoryAware;
/**
* Spring AOP Advisor that can be used for any AspectJ pointcut expression.
@ -26,16 +28,11 @@ import org.springframework.aop.support.AbstractGenericPointcutAdvisor;
* @since 2.0
*/
@SuppressWarnings("serial")
public class AspectJExpressionPointcutAdvisor extends AbstractGenericPointcutAdvisor {
public class AspectJExpressionPointcutAdvisor extends AbstractGenericPointcutAdvisor implements BeanFactoryAware {
private final AspectJExpressionPointcut pointcut = new AspectJExpressionPointcut();
@Override
public Pointcut getPointcut() {
return this.pointcut;
}
public void setExpression(String expression) {
this.pointcut.setExpression(expression);
}
@ -52,12 +49,22 @@ public class AspectJExpressionPointcutAdvisor extends AbstractGenericPointcutAdv
return this.pointcut.getLocation();
}
public void setParameterTypes(Class<?>[] types) {
this.pointcut.setParameterTypes(types);
}
public void setParameterNames(String... names) {
this.pointcut.setParameterNames(names);
}
public void setParameterTypes(Class<?>... types) {
this.pointcut.setParameterTypes(types);
}
@Override
public void setBeanFactory(BeanFactory beanFactory) {
this.pointcut.setBeanFactory(beanFactory);
}
@Override
public Pointcut getPointcut() {
return this.pointcut;
}
}

View File

@ -38,7 +38,6 @@ import org.aspectj.lang.reflect.AjType;
import org.aspectj.lang.reflect.AjTypeSystem;
import org.aspectj.lang.reflect.PerClauseKind;
import org.springframework.aop.aspectj.AspectJExpressionPointcut;
import org.springframework.aop.framework.AopConfigException;
import org.springframework.core.ParameterNameDiscoverer;
import org.springframework.core.annotation.AnnotationUtils;
@ -121,49 +120,6 @@ public abstract class AbstractAspectJAdvisorFactory implements AspectJAdvisorFac
}
}
/**
* The pointcut and advice annotations both have an "argNames" member which contains a
* comma-separated list of the argument names. We use this (if non-empty) to build the
* formal parameters for the pointcut.
*/
protected AspectJExpressionPointcut createPointcutExpression(
Method annotatedMethod, Class<?> declarationScope, String[] pointcutParameterNames) {
Class<?> [] pointcutParameterTypes = new Class<?>[0];
if (pointcutParameterNames != null) {
pointcutParameterTypes = extractPointcutParameterTypes(pointcutParameterNames,annotatedMethod);
}
AspectJExpressionPointcut ajexp =
new AspectJExpressionPointcut(declarationScope,pointcutParameterNames,pointcutParameterTypes);
ajexp.setLocation(annotatedMethod.toString());
return ajexp;
}
/**
* Create the pointcut parameters needed by aspectj based on the given argument names
* and the argument types that are available from the adviceMethod. Needs to take into
* account (ignore) any JoinPoint based arguments as these are not pointcut context but
* rather part of the advice execution context (thisJoinPoint, thisJoinPointStaticPart)
*/
private Class<?>[] extractPointcutParameterTypes(String[] argNames, Method adviceMethod) {
Class<?>[] ret = new Class<?>[argNames.length];
Class<?>[] paramTypes = adviceMethod.getParameterTypes();
if (argNames.length > paramTypes.length) {
throw new IllegalStateException("Expecting at least " + argNames.length +
" arguments in the advice declaration, but only found " + paramTypes.length);
}
// Make the simplifying assumption for now that all of the JoinPoint based arguments
// come first in the advice declaration.
int typeOffset = paramTypes.length - argNames.length;
for (int i = 0; i < ret.length; i++) {
ret[i] = paramTypes[i + typeOffset];
}
return ret;
}
/**
* Find and return the first AspectJ annotation on the given method
* (there <i>should</i> only be one anyway...)

View File

@ -50,7 +50,7 @@ public class AnnotationAwareAspectJAutoProxyCreator extends AspectJAwareAdvisorA
private List<Pattern> includePatterns;
private AspectJAdvisorFactory aspectJAdvisorFactory = new ReflectiveAspectJAdvisorFactory();
private AspectJAdvisorFactory aspectJAdvisorFactory;
private BeanFactoryAspectJAdvisorsBuilder aspectJAdvisorsBuilder;
@ -74,6 +74,9 @@ public class AnnotationAwareAspectJAutoProxyCreator extends AspectJAwareAdvisorA
@Override
protected void initBeanFactory(ConfigurableListableBeanFactory beanFactory) {
super.initBeanFactory(beanFactory);
if (this.aspectJAdvisorFactory == null) {
this.aspectJAdvisorFactory = new ReflectiveAspectJAdvisorFactory(beanFactory);
}
this.aspectJAdvisorsBuilder =
new BeanFactoryAspectJAdvisorsBuilderAdapter(beanFactory, this.aspectJAdvisorFactory);
}
@ -130,6 +133,7 @@ public class AnnotationAwareAspectJAutoProxyCreator extends AspectJAwareAdvisorA
public BeanFactoryAspectJAdvisorsBuilderAdapter(
ListableBeanFactory beanFactory, AspectJAdvisorFactory advisorFactory) {
super(beanFactory, advisorFactory);
}

View File

@ -16,9 +16,9 @@
package org.springframework.aop.aspectj.annotation;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import org.aspectj.lang.reflect.PerClauseKind;
@ -50,7 +50,7 @@ import org.springframework.util.ClassUtils;
public class AspectJProxyFactory extends ProxyCreatorSupport {
/** Cache for singleton aspect instances */
private static final Map<Class<?>, Object> aspectCache = new HashMap<>();
private static final Map<Class<?>, Object> aspectCache = new ConcurrentHashMap<>();
private final AspectJAdvisorFactory aspectFactory = new ReflectiveAspectJAdvisorFactory();
@ -144,7 +144,7 @@ public class AspectJProxyFactory extends ProxyCreatorSupport {
private MetadataAwareAspectInstanceFactory createAspectInstanceFactory(
AspectMetadata am, Class<?> aspectClass, String aspectName) {
MetadataAwareAspectInstanceFactory instanceFactory = null;
MetadataAwareAspectInstanceFactory instanceFactory;
if (am.getAjType().getPerClause().getKind() == PerClauseKind.SINGLETON) {
// Create a shared aspect instance.
Object instance = getSingletonAspectInstance(aspectClass);
@ -162,15 +162,20 @@ public class AspectJProxyFactory extends ProxyCreatorSupport {
* is created if one cannot be found in the instance cache.
*/
private Object getSingletonAspectInstance(Class<?> aspectClass) {
synchronized (aspectCache) {
Object instance = aspectCache.get(aspectClass);
if (instance != null) {
return instance;
// Quick check without a lock...
Object instance = aspectCache.get(aspectClass);
if (instance == null) {
synchronized (aspectCache) {
// To be safe, check within full lock now...
instance = aspectCache.get(aspectClass);
if (instance != null) {
return instance;
}
instance = new SimpleAspectInstanceFactory(aspectClass).getAspectInstance();
aspectCache.put(aspectClass, instance);
}
instance = new SimpleAspectInstanceFactory(aspectClass).getAspectInstance();
aspectCache.put(aspectClass, instance);
return instance;
}
return instance;
}

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2015 the original author or authors.
* Copyright 2002-2016 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.
@ -35,9 +35,8 @@ import org.springframework.aop.support.ComposablePointcut;
* Metadata for an AspectJ aspect class, with an additional Spring AOP pointcut
* for the per clause.
*
* <p>Uses AspectJ 5 AJType reflection API, so is only supported on Java 5.
* Enables us to work with different AspectJ instantiation models such as
* "singleton", "pertarget" and "perthis".
* <p>Uses AspectJ 5 AJType reflection API, enabling us to work with different
* AspectJ instantiation models such as "singleton", "pertarget" and "perthis".
*
* @author Rod Johnson
* @author Juergen Hoeller
@ -102,20 +101,22 @@ public class AspectMetadata implements Serializable {
this.ajType = ajType;
switch (this.ajType.getPerClause().getKind()) {
case SINGLETON :
case SINGLETON:
this.perClausePointcut = Pointcut.TRUE;
return;
case PERTARGET : case PERTHIS :
case PERTARGET:
case PERTHIS:
AspectJExpressionPointcut ajexp = new AspectJExpressionPointcut();
ajexp.setLocation("@Aspect annotation on " + aspectClass.getName());
ajexp.setLocation(aspectClass.getName());
ajexp.setExpression(findPerClause(aspectClass));
ajexp.setPointcutDeclarationScope(aspectClass);
this.perClausePointcut = ajexp;
return;
case PERTYPEWITHIN :
case PERTYPEWITHIN:
// Works with a type pattern
this.perClausePointcut = new ComposablePointcut(new TypePatternClassFilter(findPerClause(aspectClass)));
return;
default :
default:
throw new AopConfigException(
"PerClause " + ajType.getPerClause().getKind() + " not supported by Spring AOP for " + aspectClass);
}
@ -125,8 +126,6 @@ public class AspectMetadata implements Serializable {
* Extract contents from String of form {@code pertarget(contents)}.
*/
private String findPerClause(Class<?> aspectClass) {
// TODO when AspectJ provides this, we can remove this hack. Hence we don't
// bother to make it elegant. Or efficient. Or robust :-)
String str = aspectClass.getAnnotation(Aspect.class).value();
str = str.substring(str.indexOf("(") + 1);
str = str.substring(0, str.length() - 1);

View File

@ -17,10 +17,10 @@
package org.springframework.aop.aspectj.annotation;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import org.aspectj.lang.reflect.PerClauseKind;
@ -43,12 +43,11 @@ public class BeanFactoryAspectJAdvisorsBuilder {
private final AspectJAdvisorFactory advisorFactory;
private List<String> aspectBeanNames;
private volatile List<String> aspectBeanNames;
private final Map<String, List<Advisor>> advisorsCache = new HashMap<>();
private final Map<String, List<Advisor>> advisorsCache = new ConcurrentHashMap<>();
private final Map<String, MetadataAwareAspectInstanceFactory> aspectFactoryCache =
new HashMap<>();
private final Map<String, MetadataAwareAspectInstanceFactory> aspectFactoryCache = new ConcurrentHashMap<>();
/**
@ -56,7 +55,7 @@ public class BeanFactoryAspectJAdvisorsBuilder {
* @param beanFactory the ListableBeanFactory to scan
*/
public BeanFactoryAspectJAdvisorsBuilder(ListableBeanFactory beanFactory) {
this(beanFactory, new ReflectiveAspectJAdvisorFactory());
this(beanFactory, new ReflectiveAspectJAdvisorFactory(beanFactory));
}
/**
@ -80,56 +79,57 @@ public class BeanFactoryAspectJAdvisorsBuilder {
* @see #isEligibleBean
*/
public List<Advisor> buildAspectJAdvisors() {
List<String> aspectNames = null;
List<String> aspectNames = this.aspectBeanNames;
synchronized (this) {
aspectNames = this.aspectBeanNames;
if (aspectNames == null) {
List<Advisor> advisors = new LinkedList<>();
aspectNames = new LinkedList<>();
String[] beanNames =
BeanFactoryUtils.beanNamesForTypeIncludingAncestors(this.beanFactory, Object.class, true, false);
for (String beanName : beanNames) {
if (!isEligibleBean(beanName)) {
continue;
}
// We must be careful not to instantiate beans eagerly as in this
// case they would be cached by the Spring container but would not
// have been weaved
Class<?> beanType = this.beanFactory.getType(beanName);
if (beanType == null) {
continue;
}
if (this.advisorFactory.isAspect(beanType)) {
aspectNames.add(beanName);
AspectMetadata amd = new AspectMetadata(beanType, beanName);
if (amd.getAjType().getPerClause().getKind() == PerClauseKind.SINGLETON) {
MetadataAwareAspectInstanceFactory factory =
new BeanFactoryAspectInstanceFactory(this.beanFactory, beanName);
List<Advisor> classAdvisors = this.advisorFactory.getAdvisors(factory);
if (this.beanFactory.isSingleton(beanName)) {
this.advisorsCache.put(beanName, classAdvisors);
if (aspectNames == null) {
synchronized (this) {
aspectNames = this.aspectBeanNames;
if (aspectNames == null) {
List<Advisor> advisors = new LinkedList<>();
aspectNames = new LinkedList<>();
String[] beanNames = BeanFactoryUtils.beanNamesForTypeIncludingAncestors(
this.beanFactory, Object.class, true, false);
for (String beanName : beanNames) {
if (!isEligibleBean(beanName)) {
continue;
}
// We must be careful not to instantiate beans eagerly as in this case they
// would be cached by the Spring container but would not have been weaved.
Class<?> beanType = this.beanFactory.getType(beanName);
if (beanType == null) {
continue;
}
if (this.advisorFactory.isAspect(beanType)) {
aspectNames.add(beanName);
AspectMetadata amd = new AspectMetadata(beanType, beanName);
if (amd.getAjType().getPerClause().getKind() == PerClauseKind.SINGLETON) {
MetadataAwareAspectInstanceFactory factory =
new BeanFactoryAspectInstanceFactory(this.beanFactory, beanName);
List<Advisor> classAdvisors = this.advisorFactory.getAdvisors(factory);
if (this.beanFactory.isSingleton(beanName)) {
this.advisorsCache.put(beanName, classAdvisors);
}
else {
this.aspectFactoryCache.put(beanName, factory);
}
advisors.addAll(classAdvisors);
}
else {
// Per target or per this.
if (this.beanFactory.isSingleton(beanName)) {
throw new IllegalArgumentException("Bean with name '" + beanName +
"' is a singleton, but aspect instantiation model is not singleton");
}
MetadataAwareAspectInstanceFactory factory =
new PrototypeAspectInstanceFactory(this.beanFactory, beanName);
this.aspectFactoryCache.put(beanName, factory);
advisors.addAll(this.advisorFactory.getAdvisors(factory));
}
advisors.addAll(classAdvisors);
}
else {
// Per target or per this.
if (this.beanFactory.isSingleton(beanName)) {
throw new IllegalArgumentException("Bean with name '" + beanName +
"' is a singleton, but aspect instantiation model is not singleton");
}
MetadataAwareAspectInstanceFactory factory =
new PrototypeAspectInstanceFactory(this.beanFactory, beanName);
this.aspectFactoryCache.put(beanName, factory);
advisors.addAll(this.advisorFactory.getAdvisors(factory));
}
}
this.aspectBeanNames = aspectNames;
return advisors;
}
this.aspectBeanNames = aspectNames;
return advisors;
}
}

View File

@ -46,6 +46,7 @@ import org.springframework.aop.aspectj.AspectJMethodBeforeAdvice;
import org.springframework.aop.aspectj.DeclareParentsAdvisor;
import org.springframework.aop.framework.AopConfigException;
import org.springframework.aop.support.DefaultPointcutAdvisor;
import org.springframework.beans.factory.BeanFactory;
import org.springframework.core.annotation.AnnotationUtils;
import org.springframework.core.convert.converter.Converter;
import org.springframework.core.convert.converter.ConvertingComparator;
@ -95,6 +96,30 @@ public class ReflectiveAspectJAdvisorFactory extends AbstractAspectJAdvisorFacto
}
private final BeanFactory beanFactory;
/**
* Create a new {@code ReflectiveAspectJAdvisorFactory}.
*/
public ReflectiveAspectJAdvisorFactory() {
this(null);
}
/**
* Create a new {@code ReflectiveAspectJAdvisorFactory}, propagating the given
* {@link BeanFactory} to the created {@link AspectJExpressionPointcut} instances,
* for bean pointcut handling as well as consistent {@link ClassLoader} resolution.
* @param beanFactory the BeanFactory to propagate (may be {@code null}}
* @since 4.3.6
* @see AspectJExpressionPointcut#setBeanFactory
* @see org.springframework.beans.factory.config.ConfigurableBeanFactory#getBeanClassLoader()
*/
public ReflectiveAspectJAdvisorFactory(BeanFactory beanFactory) {
this.beanFactory = beanFactory;
}
@Override
public List<Advisor> getAdvisors(MetadataAwareAspectInstanceFactory aspectInstanceFactory) {
Class<?> aspectClass = aspectInstanceFactory.getAspectMetadata().getAspectClass();
@ -161,9 +186,7 @@ public class ReflectiveAspectJAdvisorFactory extends AbstractAspectJAdvisorFacto
}
if (DeclareParents.class == declareParents.defaultImpl()) {
// This is what comes back if it wasn't set. This seems bizarre...
// TODO this restriction possibly should be relaxed
throw new IllegalStateException("defaultImpl must be set on DeclareParents");
throw new IllegalStateException("'defaultImpl' attribute must be set on DeclareParents");
}
return new DeclareParentsAdvisor(
@ -197,6 +220,7 @@ public class ReflectiveAspectJAdvisorFactory extends AbstractAspectJAdvisorFacto
AspectJExpressionPointcut ajexp =
new AspectJExpressionPointcut(candidateAspectClass, new String[0], new Class<?>[0]);
ajexp.setExpression(aspectJAnnotation.getPointcutExpression());
ajexp.setBeanFactory(this.beanFactory);
return ajexp;
}