SPR-5668 introduced occasional ConcurrentModificationException in callees of AspectJExpressionPointcut.getShadowMatch() [SPR-5687]

[spring-projects-issues]

Nikita Tovstoles opened SPR-5687 and commented

I ported the fix for #10339 to spring-aop 2.5.6 and now I periodically see the following under concurrent load:

java.util.ConcurrentModificationException
	at java.util.HashMap$HashIterator.nextEntry(HashMap.java:793)
	at java.util.HashMap$KeyIterator.next(HashMap.java:828)
	at org.aspectj.apache.bcel.util.NonCachingClassLoaderRepository$SoftHashMap.clear(NonCachingClassLoaderRepository.java:143)
	at org.aspectj.apache.bcel.util.NonCachingClassLoaderRepository.clear(NonCachingClassLoaderRepository.java:243)
	at org.aspectj.weaver.reflect.Java15AnnotationFinder.getAnnotations(Java15AnnotationFinder.java:240)
	at org.aspectj.weaver.reflect.ReflectionBasedResolvedMemberImpl.unpackAnnotations(ReflectionBasedResolvedMemberImpl.java:213)
	at org.aspectj.weaver.reflect.ReflectionBasedResolvedMemberImpl.hasAnnotation(ReflectionBasedResolvedMemberImpl.java:169)
	at org.aspectj.weaver.patterns.ExactAnnotationTypePattern.matches(ExactAnnotationTypePattern.java:108)
	at org.aspectj.weaver.patterns.ExactAnnotationTypePattern.matches(ExactAnnotationTypePattern.java:94)
	at org.aspectj.weaver.patterns.AnnotationPointcut.matchInternal(AnnotationPointcut.java:157)
	at org.aspectj.weaver.patterns.Pointcut.match(Pointcut.java:146)
	at org.aspectj.weaver.internal.tools.PointcutExpressionImpl.getShadowMatch(PointcutExpressionImpl.java:235)
	at org.aspectj.weaver.internal.tools.PointcutExpressionImpl.matchesExecution(PointcutExpressionImpl.java:101)
	at org.aspectj.weaver.internal.tools.PointcutExpressionImpl.matchesMethodExecution(PointcutExpressionImpl.java:92)
	at org.springframework.aop.aspectj.AspectJExpressionPointcut.getShadowMatch(AspectJExpressionPointcut.java:354)
	at org.springframework.aop.aspectj.AspectJExpressionPointcut.matches(AspectJExpressionPointcut.java:247)
	at org.springframework.aop.support.MethodMatchers.matches(MethodMatchers.java:93)
	at org.springframework.aop.framework.DefaultAdvisorChainFactory.getInterceptorsAndDynamicInterceptionAdvice(DefaultAdvisorChainFactory.java:63)
	at org.springframework.aop.framework.AdvisedSupport.getInterceptorsAndDynamicInterceptionAdvice(AdvisedSupport.java:481)
	at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:190)
	at $Proxy95.getCustomSpacesInfoByOwner(Unknown Source)

it appears that PointcutExpressionImpl.matchesMethodExecution() isn't thread-safe and should be called from a synchronized block

---

Affects: 3.0 M2

Referenced from: commits 1b9b513

[spring-projects-issues]

Nikita Tovstoles commented

Please set component to Spring-AOP

[spring-projects-issues]

Nikita Tovstoles commented

Suggested Fix

• Started with revision 1021 of AspectJExpressionPointcut
• changed getShadowMatch() to synchronize calls to aspectJ (during cache misses only):

private ShadowMatch getShadowMatch(Method targetMethod, Method originalMethod) {
     ShadowMatch shadowMatch = this.shadowMatchCache.get(targetMethod);
     if (shadowMatch == null) {
          try {
           synchronized(this){
               shadowMatch = this.pointcutExpression.matchesMethodExecution(targetMethod);
           }
          }
          catch (ReflectionWorld.ReflectionWorldException ex) {
               // Failed to introspect target method, probably because it has been loaded
               // in a special ClassLoader. Let's try the original method instead...
               if (targetMethod == originalMethod) {
                        shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null);
               }
               else {
                        try {
                   synchronized(this){
                       shadowMatch = this.pointcutExpression.matchesMethodExecution(originalMethod);
                   }
                        }
                        catch (ReflectionWorld.ReflectionWorldException ex2) {
                                 // Could neither introspect the target class nor the proxy class ->
                                 // let's simply consider this method as non-matching.
                                 shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null);
                        }
               }
          }
          this.shadowMatchCache.put(targetMethod, shadowMatch);
     }
     return shadowMatch;
}

can no longer repro exception.

[spring-projects-issues]

Juergen Hoeller commented

Thanks for pointing this out... I've revised the code with a general synchronization block for the not-found case, which also avoids re-creation of the same ShadowMatch object.

Juergen

[spring-projects-issues]

Amir Kibbar commented

I'm running into the same issue in both 2.5.6.SEC01 and 3.0.0.M4. Could it be that the issue is not resolve in the 3.0 line? can you please also backport the fix to 2.5.x?

thanks,
Amir