Skip to content

Commit 87855e2

Browse files
committed
Clear ShadowMatch instances when they are no longer needed
This commit gathers the ShadowMatch instances that AspectJ requires in a dedicated class that can be used to clear the instances when they are no longer required. As those are mainly triggered via AspectJAwareAdvisorAutoProxyCreator, it now implements the necessary callbacks to clear the cache. Closes gh-12334
1 parent 331bdb0 commit 87855e2

File tree

3 files changed

+140
-77
lines changed

3 files changed

+140
-77
lines changed

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

Lines changed: 50 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,10 @@
1616

1717
package org.springframework.aop.aspectj;
1818

19-
import java.io.IOException;
20-
import java.io.ObjectInputStream;
2119
import java.lang.reflect.Method;
2220
import java.lang.reflect.Proxy;
2321
import java.util.Arrays;
24-
import java.util.Map;
2522
import java.util.Set;
26-
import java.util.concurrent.ConcurrentHashMap;
2723

2824
import org.aopalliance.intercept.MethodInvocation;
2925
import org.apache.commons.logging.Log;
@@ -115,8 +111,6 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
115111
@Nullable
116112
private transient PointcutExpression pointcutExpression;
117113

118-
private transient Map<Method, ShadowMatch> shadowMatchCache = new ConcurrentHashMap<>(32);
119-
120114

121115
/**
122116
* Create a new default AspectJExpressionPointcut.
@@ -447,72 +441,66 @@ private ShadowMatch getTargetShadowMatch(Method method, Class<?> targetClass) {
447441
}
448442

449443
private ShadowMatch getShadowMatch(Method targetMethod, Method originalMethod) {
450-
// Avoid lock contention for known Methods through concurrent access...
451-
ShadowMatch shadowMatch = this.shadowMatchCache.get(targetMethod);
444+
String expression = resolveExpression();
445+
ShadowMatch shadowMatch = ShadowMatchUtils.getShadowMatch(this, targetMethod);
452446
if (shadowMatch == null) {
453-
synchronized (this.shadowMatchCache) {
454-
// Not found - now check again with full lock...
455-
PointcutExpression fallbackExpression = null;
456-
shadowMatch = this.shadowMatchCache.get(targetMethod);
457-
if (shadowMatch == null) {
458-
Method methodToMatch = targetMethod;
447+
PointcutExpression fallbackExpression = null;
448+
Method methodToMatch = targetMethod;
449+
try {
450+
try {
451+
shadowMatch = obtainPointcutExpression().matchesMethodExecution(methodToMatch);
452+
}
453+
catch (ReflectionWorldException ex) {
454+
// Failed to introspect target method, probably because it has been loaded
455+
// in a special ClassLoader. Let's try the declaring ClassLoader instead...
459456
try {
460-
try {
461-
shadowMatch = obtainPointcutExpression().matchesMethodExecution(methodToMatch);
462-
}
463-
catch (ReflectionWorldException ex) {
464-
// Failed to introspect target method, probably because it has been loaded
465-
// in a special ClassLoader. Let's try the declaring ClassLoader instead...
466-
try {
467-
fallbackExpression = getFallbackPointcutExpression(methodToMatch.getDeclaringClass());
468-
if (fallbackExpression != null) {
469-
shadowMatch = fallbackExpression.matchesMethodExecution(methodToMatch);
470-
}
471-
}
472-
catch (ReflectionWorldException ex2) {
473-
fallbackExpression = null;
474-
}
475-
}
476-
if (targetMethod != originalMethod && (shadowMatch == null ||
477-
(Proxy.isProxyClass(targetMethod.getDeclaringClass()) &&
478-
(shadowMatch.neverMatches() || containsAnnotationPointcut())))) {
479-
// Fall back to the plain original method in case of no resolvable match or a
480-
// negative match on a proxy class (which doesn't carry any annotations on its
481-
// redeclared methods), as well as for annotation pointcuts.
482-
methodToMatch = originalMethod;
483-
try {
484-
shadowMatch = obtainPointcutExpression().matchesMethodExecution(methodToMatch);
485-
}
486-
catch (ReflectionWorldException ex) {
487-
// Could neither introspect the target class nor the proxy class ->
488-
// let's try the original method's declaring class before we give up...
489-
try {
490-
fallbackExpression = getFallbackPointcutExpression(methodToMatch.getDeclaringClass());
491-
if (fallbackExpression != null) {
492-
shadowMatch = fallbackExpression.matchesMethodExecution(methodToMatch);
493-
}
494-
}
495-
catch (ReflectionWorldException ex2) {
496-
fallbackExpression = null;
497-
}
498-
}
457+
fallbackExpression = getFallbackPointcutExpression(methodToMatch.getDeclaringClass());
458+
if (fallbackExpression != null) {
459+
shadowMatch = fallbackExpression.matchesMethodExecution(methodToMatch);
499460
}
500461
}
501-
catch (Throwable ex) {
502-
// Possibly AspectJ 1.8.10 encountering an invalid signature
503-
logger.debug("PointcutExpression matching rejected target method", ex);
462+
catch (ReflectionWorldException ex2) {
504463
fallbackExpression = null;
505464
}
506-
if (shadowMatch == null) {
507-
shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null);
465+
}
466+
if (targetMethod != originalMethod && (shadowMatch == null ||
467+
(Proxy.isProxyClass(targetMethod.getDeclaringClass()) &&
468+
(shadowMatch.neverMatches() || containsAnnotationPointcut())))) {
469+
// Fall back to the plain original method in case of no resolvable match or a
470+
// negative match on a proxy class (which doesn't carry any annotations on its
471+
// redeclared methods), as well as for annotation pointcuts.
472+
methodToMatch = originalMethod;
473+
try {
474+
shadowMatch = obtainPointcutExpression().matchesMethodExecution(methodToMatch);
508475
}
509-
else if (shadowMatch.maybeMatches() && fallbackExpression != null) {
510-
shadowMatch = new DefensiveShadowMatch(shadowMatch,
511-
fallbackExpression.matchesMethodExecution(methodToMatch));
476+
catch (ReflectionWorldException ex) {
477+
// Could neither introspect the target class nor the proxy class ->
478+
// let's try the original method's declaring class before we give up...
479+
try {
480+
fallbackExpression = getFallbackPointcutExpression(methodToMatch.getDeclaringClass());
481+
if (fallbackExpression != null) {
482+
shadowMatch = fallbackExpression.matchesMethodExecution(methodToMatch);
483+
}
484+
}
485+
catch (ReflectionWorldException ex2) {
486+
fallbackExpression = null;
487+
}
512488
}
513-
this.shadowMatchCache.put(targetMethod, shadowMatch);
514489
}
515490
}
491+
catch (Throwable ex) {
492+
// Possibly AspectJ 1.8.10 encountering an invalid signature
493+
logger.debug("PointcutExpression matching rejected target method", ex);
494+
fallbackExpression = null;
495+
}
496+
if (shadowMatch == null) {
497+
shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null);
498+
}
499+
else if (shadowMatch.maybeMatches() && fallbackExpression != null) {
500+
shadowMatch = new DefensiveShadowMatch(shadowMatch,
501+
fallbackExpression.matchesMethodExecution(methodToMatch));
502+
}
503+
shadowMatch = ShadowMatchUtils.setShadowMatch(this, targetMethod, shadowMatch);
516504
}
517505
return shadowMatch;
518506
}
@@ -558,19 +546,6 @@ public String toString() {
558546
return sb.toString();
559547
}
560548

561-
//---------------------------------------------------------------------
562-
// Serialization support
563-
//---------------------------------------------------------------------
564-
565-
private void readObject(ObjectInputStream ois) throws IOException, ClassNotFoundException {
566-
// Rely on default serialization, just initialize state after deserialization.
567-
ois.defaultReadObject();
568-
569-
// Initialize transient fields.
570-
// pointcutExpression will be initialized lazily by checkReadyToMatch()
571-
this.shadowMatchCache = new ConcurrentHashMap<>(32);
572-
}
573-
574549

575550
/**
576551
* Handler for the Spring-specific {@code bean()} pointcut designator
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
/*
2+
* Copyright 2002-2024 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.aop.aspectj;
18+
19+
import java.lang.reflect.Method;
20+
import java.util.Map;
21+
import java.util.concurrent.ConcurrentHashMap;
22+
23+
import org.aspectj.weaver.tools.ShadowMatch;
24+
25+
import org.springframework.aop.support.ExpressionPointcut;
26+
import org.springframework.lang.Nullable;
27+
28+
/**
29+
* Internal {@link ShadowMatch} utilities.
30+
*
31+
* @author Stephane Nicoll
32+
* @since 6.2
33+
*/
34+
public abstract class ShadowMatchUtils {
35+
36+
private static final Map<Key, ShadowMatch> shadowMatchCache = new ConcurrentHashMap<>(256);
37+
38+
/**
39+
* Clear the cache of computed {@link ShadowMatch} instances.
40+
*/
41+
public static void clearCache() {
42+
shadowMatchCache.clear();
43+
}
44+
45+
/**
46+
* Return the {@link ShadowMatch} for the specified {@link ExpressionPointcut}
47+
* and {@link Method} or {@code null} if none is found.
48+
* @param expression the expression
49+
* @param method the method
50+
* @return the {@code ShadowMatch} to use for the specified expression and method
51+
*/
52+
@Nullable
53+
static ShadowMatch getShadowMatch(ExpressionPointcut expression, Method method) {
54+
return shadowMatchCache.get(new Key(expression, method));
55+
}
56+
57+
/**
58+
* Associate the {@link ShadowMatch} to the specified {@link ExpressionPointcut}
59+
* and method. If an entry already exists, the given {@code shadowMatch} is
60+
* ignored.
61+
* @param expression the expression
62+
* @param method the method
63+
* @param shadowMatch the shadow match to use for this expression and method
64+
* if none already exists
65+
* @return the shadow match to use for the specified expression and method
66+
*/
67+
static ShadowMatch setShadowMatch(ExpressionPointcut expression, Method method, ShadowMatch shadowMatch) {
68+
ShadowMatch existing = shadowMatchCache.putIfAbsent(new Key(expression, method), shadowMatch);
69+
return (existing != null ? existing : shadowMatch);
70+
}
71+
72+
73+
private record Key(ExpressionPointcut expression, Method method) {}
74+
75+
}

spring-aop/src/main/java/org/springframework/aop/aspectj/autoproxy/AspectJAwareAdvisorAutoProxyCreator.java

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2024 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.
@@ -28,8 +28,11 @@
2828
import org.springframework.aop.aspectj.AbstractAspectJAdvice;
2929
import org.springframework.aop.aspectj.AspectJPointcutAdvisor;
3030
import org.springframework.aop.aspectj.AspectJProxyUtils;
31+
import org.springframework.aop.aspectj.ShadowMatchUtils;
3132
import org.springframework.aop.framework.autoproxy.AbstractAdvisorAutoProxyCreator;
3233
import org.springframework.aop.interceptor.ExposeInvocationInterceptor;
34+
import org.springframework.beans.factory.DisposableBean;
35+
import org.springframework.beans.factory.SmartInitializingSingleton;
3336
import org.springframework.core.Ordered;
3437
import org.springframework.util.ClassUtils;
3538

@@ -44,7 +47,8 @@
4447
* @since 2.0
4548
*/
4649
@SuppressWarnings("serial")
47-
public class AspectJAwareAdvisorAutoProxyCreator extends AbstractAdvisorAutoProxyCreator {
50+
public class AspectJAwareAdvisorAutoProxyCreator extends AbstractAdvisorAutoProxyCreator
51+
implements SmartInitializingSingleton, DisposableBean {
4852

4953
private static final Comparator<Advisor> DEFAULT_PRECEDENCE_COMPARATOR = new AspectJPrecedenceComparator();
5054

@@ -108,6 +112,15 @@ protected boolean shouldSkip(Class<?> beanClass, String beanName) {
108112
return super.shouldSkip(beanClass, beanName);
109113
}
110114

115+
@Override
116+
public void afterSingletonsInstantiated() {
117+
ShadowMatchUtils.clearCache();
118+
}
119+
120+
@Override
121+
public void destroy() throws Exception {
122+
ShadowMatchUtils.clearCache();
123+
}
111124

112125
/**
113126
* Implements AspectJ's {@link PartialComparable} interface for defining partial orderings.

0 commit comments

Comments
 (0)