Skip to content

Commit f1ddd05

Browse files
jhoellerjhoeller
committed
Restore synchronization against AspectJ race condition behind PointcutExpression
Closes gh-34735
1 parent a266e1b commit f1ddd05

File tree

2 files changed

+87
-78
lines changed

2 files changed

+87
-78
lines changed

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

Lines changed: 65 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2024 the original author or authors.
2+
* Copyright 2002-2025 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.
@@ -112,12 +112,12 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
112112
private BeanFactory beanFactory;
113113

114114
@Nullable
115-
private transient ClassLoader pointcutClassLoader;
115+
private transient volatile ClassLoader pointcutClassLoader;
116116

117117
@Nullable
118-
private transient PointcutExpression pointcutExpression;
118+
private transient volatile PointcutExpression pointcutExpression;
119119

120-
private transient boolean pointcutParsingFailed = false;
120+
private transient volatile boolean pointcutParsingFailed;
121121

122122

123123
/**
@@ -197,11 +197,14 @@ private void checkExpression() {
197197
* Lazily build the underlying AspectJ pointcut expression.
198198
*/
199199
private PointcutExpression obtainPointcutExpression() {
200-
if (this.pointcutExpression == null) {
201-
this.pointcutClassLoader = determinePointcutClassLoader();
202-
this.pointcutExpression = buildPointcutExpression(this.pointcutClassLoader);
200+
PointcutExpression pointcutExpression = this.pointcutExpression;
201+
if (pointcutExpression == null) {
202+
ClassLoader pointcutClassLoader = determinePointcutClassLoader();
203+
pointcutExpression = buildPointcutExpression(pointcutClassLoader);
204+
this.pointcutClassLoader = pointcutClassLoader;
205+
this.pointcutExpression = pointcutExpression;
203206
}
204-
return this.pointcutExpression;
207+
return pointcutExpression;
205208
}
206209

207210
/**
@@ -467,40 +470,24 @@ private ShadowMatch getTargetShadowMatch(Method method, Class<?> targetClass) {
467470
}
468471

469472
private ShadowMatch getShadowMatch(Method targetMethod, Method originalMethod) {
470-
ShadowMatch shadowMatch = ShadowMatchUtils.getShadowMatch(this, targetMethod);
473+
ShadowMatchKey key = new ShadowMatchKey(this, targetMethod);
474+
ShadowMatch shadowMatch = ShadowMatchUtils.getShadowMatch(key);
471475
if (shadowMatch == null) {
472-
PointcutExpression fallbackExpression = null;
473-
Method methodToMatch = targetMethod;
474-
try {
475-
try {
476-
shadowMatch = obtainPointcutExpression().matchesMethodExecution(methodToMatch);
477-
}
478-
catch (ReflectionWorldException ex) {
479-
// Failed to introspect target method, probably because it has been loaded
480-
// in a special ClassLoader. Let's try the declaring ClassLoader instead...
481-
try {
482-
fallbackExpression = getFallbackPointcutExpression(methodToMatch.getDeclaringClass());
483-
if (fallbackExpression != null) {
484-
shadowMatch = fallbackExpression.matchesMethodExecution(methodToMatch);
485-
}
486-
}
487-
catch (ReflectionWorldException ex2) {
488-
fallbackExpression = null;
489-
}
476+
PointcutExpression pointcutExpression = obtainPointcutExpression();
477+
synchronized (pointcutExpression) {
478+
shadowMatch = ShadowMatchUtils.getShadowMatch(key);
479+
if (shadowMatch != null) {
480+
return shadowMatch;
490481
}
491-
if (targetMethod != originalMethod && (shadowMatch == null ||
492-
(Proxy.isProxyClass(targetMethod.getDeclaringClass()) &&
493-
(shadowMatch.neverMatches() || containsAnnotationPointcut())))) {
494-
// Fall back to the plain original method in case of no resolvable match or a
495-
// negative match on a proxy class (which doesn't carry any annotations on its
496-
// redeclared methods), as well as for annotation pointcuts.
497-
methodToMatch = originalMethod;
482+
PointcutExpression fallbackExpression = null;
483+
Method methodToMatch = targetMethod;
484+
try {
498485
try {
499-
shadowMatch = obtainPointcutExpression().matchesMethodExecution(methodToMatch);
486+
shadowMatch = pointcutExpression.matchesMethodExecution(methodToMatch);
500487
}
501488
catch (ReflectionWorldException ex) {
502-
// Could neither introspect the target class nor the proxy class ->
503-
// let's try the original method's declaring class before we give up...
489+
// Failed to introspect target method, probably because it has been loaded
490+
// in a special ClassLoader. Let's try the declaring ClassLoader instead...
504491
try {
505492
fallbackExpression = getFallbackPointcutExpression(methodToMatch.getDeclaringClass());
506493
if (fallbackExpression != null) {
@@ -511,21 +498,45 @@ private ShadowMatch getShadowMatch(Method targetMethod, Method originalMethod) {
511498
fallbackExpression = null;
512499
}
513500
}
501+
if (targetMethod != originalMethod && (shadowMatch == null ||
502+
(Proxy.isProxyClass(targetMethod.getDeclaringClass()) &&
503+
(shadowMatch.neverMatches() || containsAnnotationPointcut())))) {
504+
// Fall back to the plain original method in case of no resolvable match or a
505+
// negative match on a proxy class (which doesn't carry any annotations on its
506+
// redeclared methods), as well as for annotation pointcuts.
507+
methodToMatch = originalMethod;
508+
try {
509+
shadowMatch = pointcutExpression.matchesMethodExecution(methodToMatch);
510+
}
511+
catch (ReflectionWorldException ex) {
512+
// Could neither introspect the target class nor the proxy class ->
513+
// let's try the original method's declaring class before we give up...
514+
try {
515+
fallbackExpression = getFallbackPointcutExpression(methodToMatch.getDeclaringClass());
516+
if (fallbackExpression != null) {
517+
shadowMatch = fallbackExpression.matchesMethodExecution(methodToMatch);
518+
}
519+
}
520+
catch (ReflectionWorldException ex2) {
521+
fallbackExpression = null;
522+
}
523+
}
524+
}
514525
}
526+
catch (Throwable ex) {
527+
// Possibly AspectJ 1.8.10 encountering an invalid signature
528+
logger.debug("PointcutExpression matching rejected target method", ex);
529+
fallbackExpression = null;
530+
}
531+
if (shadowMatch == null) {
532+
shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null);
533+
}
534+
else if (shadowMatch.maybeMatches() && fallbackExpression != null) {
535+
shadowMatch = new DefensiveShadowMatch(shadowMatch,
536+
fallbackExpression.matchesMethodExecution(methodToMatch));
537+
}
538+
shadowMatch = ShadowMatchUtils.setShadowMatch(key, shadowMatch);
515539
}
516-
catch (Throwable ex) {
517-
// Possibly AspectJ 1.8.10 encountering an invalid signature
518-
logger.debug("PointcutExpression matching rejected target method", ex);
519-
fallbackExpression = null;
520-
}
521-
if (shadowMatch == null) {
522-
shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null);
523-
}
524-
else if (shadowMatch.maybeMatches() && fallbackExpression != null) {
525-
shadowMatch = new DefensiveShadowMatch(shadowMatch,
526-
fallbackExpression.matchesMethodExecution(methodToMatch));
527-
}
528-
shadowMatch = ShadowMatchUtils.setShadowMatch(this, targetMethod, shadowMatch);
529540
}
530541
return shadowMatch;
531542
}
@@ -720,4 +731,8 @@ public void setMatchingContext(MatchingContext aMatchContext) {
720731
}
721732
}
722733

734+
735+
private record ShadowMatchKey(AspectJExpressionPointcut expression, Method method) {
736+
}
737+
723738
}

‎spring-aop/src/main/java/org/springframework/aop/aspectj/ShadowMatchUtils.java‎

Lines changed: 22 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2024 the original author or authors.
2+
* Copyright 2002-2025 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.
@@ -16,60 +16,54 @@
1616

1717
package org.springframework.aop.aspectj;
1818

19-
import java.lang.reflect.Method;
2019
import java.util.Map;
2120
import java.util.concurrent.ConcurrentHashMap;
2221

2322
import org.aspectj.weaver.tools.ShadowMatch;
2423

25-
import org.springframework.aop.support.ExpressionPointcut;
2624
import org.springframework.lang.Nullable;
2725

2826
/**
2927
* Internal {@link ShadowMatch} utilities.
3028
*
3129
* @author Stephane Nicoll
30+
* @author Juergen Hoeller
3231
* @since 6.2
3332
*/
3433
public abstract class ShadowMatchUtils {
3534

36-
private static final Map<Key, ShadowMatch> shadowMatchCache = new ConcurrentHashMap<>(256);
35+
private static final Map<Object, ShadowMatch> shadowMatchCache = new ConcurrentHashMap<>(256);
3736

38-
/**
39-
* Clear the cache of computed {@link ShadowMatch} instances.
40-
*/
41-
public static void clearCache() {
42-
shadowMatchCache.clear();
43-
}
4437

4538
/**
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
39+
* Find a {@link ShadowMatch} for the specified key.
40+
* @param key the key to use
41+
* @return the {@code ShadowMatch} to use for the specified key,
42+
* or {@code null} if none found
5143
*/
5244
@Nullable
53-
static ShadowMatch getShadowMatch(ExpressionPointcut expression, Method method) {
54-
return shadowMatchCache.get(new Key(expression, method));
45+
static ShadowMatch getShadowMatch(Object key) {
46+
return shadowMatchCache.get(key);
5547
}
5648

5749
/**
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
50+
* Associate the {@link ShadowMatch} with the specified key.
51+
* If an entry already exists, the given {@code shadowMatch} is ignored.
52+
* @param key the key to use
53+
* @param shadowMatch the shadow match to use for this key
6454
* if none already exists
65-
* @return the shadow match to use for the specified expression and method
55+
* @return the shadow match to use for the specified key
6656
*/
67-
static ShadowMatch setShadowMatch(ExpressionPointcut expression, Method method, ShadowMatch shadowMatch) {
68-
ShadowMatch existing = shadowMatchCache.putIfAbsent(new Key(expression, method), shadowMatch);
57+
static ShadowMatch setShadowMatch(Object key, ShadowMatch shadowMatch) {
58+
ShadowMatch existing = shadowMatchCache.putIfAbsent(key, shadowMatch);
6959
return (existing != null ? existing : shadowMatch);
7060
}
7161

72-
73-
private record Key(ExpressionPointcut expression, Method method) {}
62+
/**
63+
* Clear the cache of computed {@link ShadowMatch} instances.
64+
*/
65+
public static void clearCache() {
66+
shadowMatchCache.clear();
67+
}
7468

7569
}

0 commit comments

Comments
 (0)