Unsafe fallback pointcut construction in AspectJExpressionPointcut [SPR-9335]

[spring-projects-issues]

Krzysiek Kasprzyk opened SPR-9335 and commented

Hi,

I have a Java EE application consisting of multiple OSGi bundles running within Apache Felix container on Weblogic 10.3. Spring DM Extender is responsible for loading application contexts of my Spring-powered bundles.

After switch from Spring 3.0.5.RELEASE to Spring 3.1.1.RELEASE the following error arised in a couple of bundles:

Caused by: java.lang.IllegalArgumentException: warning no match for this type name: pl.some.package.SomeClass [Xlint:invalidAbsoluteTypeName]
        at org.aspectj.weaver.tools.PointcutParser.parsePointcutExpression(PointcutParser.java:302)
        at org.springframework.aop.aspectj.AspectJExpressionPointcut.buildPointcutExpression(AspectJExpressionPointcut.java:207)
        at org.springframework.aop.aspectj.AspectJExpressionPointcut.getFallbackPointcutExpression(AspectJExpressionPointcut.java:358)
        at org.springframework.aop.aspectj.AspectJExpressionPointcut.getShadowMatch(AspectJExpressionPointcut.java:409)
        at org.springframework.aop.aspectj.AspectJExpressionPointcut.matches(AspectJExpressionPointcut.java:272)
        at org.springframework.aop.support.AopUtils.canApply(AopUtils.java:225)
        at org.springframework.aop.support.AopUtils.canApply(AopUtils.java:263)
        at org.springframework.aop.support.AopUtils.findAdvisorsThatCanApply(AopUtils.java:295)
        at org.springframework.aop.framework.autoproxy.AbstractAdvisorAutoProxyCreator.findAdvisorsThatCanApply(AbstractAdvisorAutoProxyCreator.java:117)
        at org.springframework.aop.framework.autoproxy.AbstractAdvisorAutoProxyCreator.findEligibleAdvisors(AbstractAdvisorAutoProxyCreator.java:87)
        at org.springframework.aop.framework.autoproxy.AbstractAdvisorAutoProxyCreator.getAdvicesAndAdvisorsForBean(AbstractAdvisorAutoProxyCreator.java:68)
        at org.springframework.aop.framework.autoproxy.AbstractAutoProxyCreator.wrapIfNecessary(AbstractAutoProxyCreator.java:359)
        at org.springframework.aop.framework.autoproxy.AbstractAutoProxyCreator.postProcessAfterInitialization(AbstractAutoProxyCreator.java:322)
        at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.applyBeanPostProcessorsAfterInitialization(AbstractAutowireCapableBeanFactory.java:407)
        at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.postProcessObjectFromFactoryBean(AbstractAutowireCapableBeanFactory.java:1598)

It prevents some bundles from starting successfully. I did some debugging and found out that this exception is thrown while AspectJAwareAdvisorAutoProxyCreator is trying to match

target(pl.some.package.SomeClass) && @annotation(pl.some.package.SomeAnnotation)

pointcut against a bean which is actually an object retrieved from Weblogic JNDI registry (weblogic.jdbc.common.internal.RmiDataSource data source to be precise). pl.some.package.SomeClass is my bundle's internal class that is visible only to classloader dedicated to this bundle. On the other hand bundle's classloader is unable to load Weblogic's weblogic.jdbc.common.internal.RmiDataSource class.

After a small investigation i discovered that the following change in org.springframework.aop.aspectj.AspectJExpressionPointcut.getShadowMatch() method is responsible for mentioned error:

if (shadowMatch == null) {
	try {
		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 {
				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);
}

versus

if (shadowMatch == null) {
	try {
		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...
		try {
			fallbackPointcutExpression = getFallbackPointcutExpression(methodToMatch.getDeclaringClass());
			shadowMatch = fallbackPointcutExpression.matchesMethodExecution(methodToMatch);
		} catch (ReflectionWorld.ReflectionWorldException e) {
			if (targetMethod == originalMethod) {
				shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null);
			}
			else {
				try {
					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.
					methodToMatch = originalMethod;
					fallbackPointcutExpression = getFallbackPointcutExpression(methodToMatch.getDeclaringClass());
					try {
						shadowMatch = fallbackPointcutExpression.matchesMethodExecution(methodToMatch);
					} catch (ReflectionWorld.ReflectionWorldException e2) {
						shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null);
					}
				}
			}
		}
	}
	if (shadowMatch.maybeMatches() && fallbackPointcutExpression!=null) {
		shadowMatch = new DefensiveShadowMatch(shadowMatch,
			fallbackPointcutExpression.matchesMethodExecution(methodToMatch));
	}
	this.shadowMatchCache.put(targetMethod, shadowMatch);
}

Normally AspectJExpressionPointcut builds PointcutExpression using AspectJ's PointcutParser and classloader fetched via Thread.currentThread().getContextClassLoader() method. Spring DM Extender ensures that at this point of context creation provided classloader delegates to bundle's internal classloader. Thanks to that creation of AspectJExpressionPointcut.pointcutExpression (equal to parsing pointcut expression by AspectJ classes) works fine. Unfortunately PointcutExpression that uses bundle's classloader cant't match weblogic.jdbc.common.internal.RmiDataSource.getConnection() method of the problematic bean. ReflectionWorldException exception is thrown and Spring tries to construct fallbackPointcutExpression using classloader that loaded weblogic.jdbc.common.internal.RmiDataSource class:

private PointcutExpression getFallbackPointcutExpression(Class<?> targetClass) {
	ClassLoader classLoader = targetClass.getClassLoader();
	return classLoader == null ? this.pointcutExpression : buildPointcutExpression(classLoader);
}

Parsing pointcut expression using such classloader fails because this classloader doesn't see pl.some.package.SomeClass. AspectJ's PointcutParser throws IllegalArgumentException in this case:

public PointcutExpression parsePointcutExpression(String expression, Class inScope, PointcutParameter[] formalParameters) 
     throws UnsupportedPointcutPrimitiveException, IllegalArgumentException {
	PointcutExpressionImpl pcExpr = null;
	try {
		Pointcut pc = resolvePointcutExpression(expression, inScope, formalParameters);
		pc = concretizePointcutExpression(pc, inScope, formalParameters);
		validateAgainstSupportedPrimitives(pc, expression); // again, because we have now followed any ref'd pcuts
		pcExpr = new PointcutExpressionImpl(pc, expression, formalParameters, getWorld());
	} catch (ParserException pEx) {
		throw new IllegalArgumentException(buildUserMessageFromParserException(expression, pEx));
	} catch (ReflectionWorld.ReflectionWorldException rwEx) {
		throw new IllegalArgumentException(rwEx.getMessage());
	}
	return pcExpr;
}

However Spring is not prepared for IllegalArgumentException in this place (only for ReflectionWorldException):

try {
	fallbackPointcutExpression = getFallbackPointcutExpression(methodToMatch.getDeclaringClass());
	shadowMatch = fallbackPointcutExpression.matchesMethodExecution(methodToMatch);
} catch (ReflectionWorld.ReflectionWorldException e) {
	if (targetMethod == originalMethod) {
		shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null);					}

Finally IllegalArgumentException is propagated through the call stack and in the end initialization of application context fails. From my point of view this is undesirable behaviour so i report this as a bug.

Note: i can probably workaround this issue by excluding RmiDataSource bean from autoproxing but this is fragile IMHO.

---

Affects: 3.1.1

Issue Links:

• Pointcut evaluation fails against AbstractHandlerMethodMapping$MappingRegistry with AspectJ 1.8.10 [SPR-15019] #19586 Pointcut evaluation fails against AbstractHandlerMethodMapping$MappingRegistry with AspectJ 1.8.10
• When use a @args as pointcut, there is case that occur a NPE at calling the unrelated method [SPR-13102] #17693 When use a @args as pointcut, there is case that occur a NPE at calling the unrelated method

Referenced from: commits 89398b0, 228a586, ce4912b

Backported to: 3.2.9

5 votes, 9 watchers

[spring-projects-issues]

Dietrich Schulten commented

Same symptom and stacktrace on WebSphere 6.1 with a bean in request scope from a shared lib, which must be proxified in order to be request-scoped. Going back from 3.1.RELEASE to 3.0.7 solves the issue.

For that bean there is no simple workaround since it must be request scoped. Excluding it from autoproxying is therefore not an option. Therefore I vote for this issue, it is a showstopper.

What makes things worse is the fact that the error message points to a totally different class which is not used at all by the bean which cannot be instantiated, and it cannot match a fully qualified class name which is never mentioned as such in any pointcut.

[spring-projects-issues]

Chris Beams commented

Krzysiek, thanks for the detailed analysis.

I've added Dave Syer as a watcher here. The change he committed for #10419 seems to be the cause of this regression. It was pretty tricky stuff, so understandable that something might have cropped up as a side effect. @Dave, could you take a look and comment and/or fix? Thanks.

[spring-projects-issues]

Chris Beams commented

ping for @Dave to add any comments

[spring-projects-issues]

Dave Syer commented

There has never been a unit test for all the catch blocks that refer to ReflectionWorldException in Spring AOP. I added some tests in 3.1 to spring-context for the groovy bug I was fixing, but that only hit the first catch block and I couldn't find a way to tickle the second one.

Maybe we need to get Andy's opinion? Or maybe he could come up with a test case at least.

[spring-projects-issues]

Chris Beams commented

Andy Clement, your thoughts?

[spring-projects-issues]

Dave Syer commented

Dietrich Schulten: it seems your way of tickling the bug is easier to reproduce than the original post, but there wasn't enough detail for me to be able to do it. Can you make a test project (e.g. using the spring-issues project on github)?

[spring-projects-issues]

Chris Beams commented

per Dave's comment, see https://github.com/SpringSource/spring-framework-issues#readme

[spring-projects-issues]

Chris Beams commented

#162

[spring-projects-issues]

Krzysiek Kasprzyk commented

Are there any plans for fixing this issue in 3.2.x or 4.0.x line?

[spring-projects-issues]

Juergen Hoeller commented

We're defensively handling fallback expression parsing now, never causing an exception from it. This will be available in both 4.0.4 and 3.2.9.

Juergen

[spring-projects-issues]

Juergen Hoeller commented

Krzysiek, does this work for you now, against 4.0.4 and/or the latest 3.2.9 snapshot? We'll be releasing 4.0.5 and 3.2.9 in about a week's time, so it's a great time to give it a try...

Juergen

[spring-projects-issues]

Jason Day commented

I was having the same issue when I upgraded from 3.0.4 to 3.2.8 (Apache Felix container + Eclipse blueprint 1.0.2). I pulled the 3.2.9.BUILD-SNAPSHOT and can confirm this change resolved the error for me.

[spring-projects-issues]

Juergen Hoeller commented

Good to hear, Jason! Thanks for your feedback...

Juergen