Spring Framework
  1. Spring Framework
  2. SPR-9335

Unsafe fallback pointcut construction in AspectJExpressionPointcut

    Details

    • Last commented by a User:
      true

      Description

      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:

      Spring-3.0.5
      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

      Spring-3.1.1
      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.

        Activity

        Hide
        Dietrich Schulten added a comment - - edited

        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.

        Show
        Dietrich Schulten added a comment - - edited 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.
        Hide
        Chris Beams added a comment -

        Krzysiek, thanks for the detailed analysis.

        I've added Dave Syer as a watcher here. The change he committed for SPR-5749 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.

        Show
        Chris Beams added a comment - Krzysiek, thanks for the detailed analysis. I've added Dave Syer as a watcher here. The change he committed for SPR-5749 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.
        Hide
        Chris Beams added a comment -

        ping for @Dave to add any comments

        Show
        Chris Beams added a comment - ping for @Dave to add any comments
        Hide
        Dave Syer added a comment - - edited

        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.

        Show
        Dave Syer added a comment - - edited 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.
        Hide
        Chris Beams added a comment -

        Andy Clement, your thoughts?

        Show
        Chris Beams added a comment - Andy Clement , your thoughts?
        Hide
        Dave Syer added a comment -

        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)?

        Show
        Dave Syer added a comment - 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)?
        Hide
        Chris Beams added a comment -
        Show
        Chris Beams added a comment - per Dave's comment, see https://github.com/SpringSource/spring-framework-issues#readme
        Show
        Chris Beams added a comment - https://github.com/SpringSource/spring-framework/pull/162
        Hide
        Krzysiek Kasprzyk added a comment -

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

        Show
        Krzysiek Kasprzyk added a comment - Are there any plans for fixing this issue in 3.2.x or 4.0.x line?

          People

          • Assignee:
            Chris Beams
            Reporter:
            Krzysiek Kasprzyk
            Last updater:
            Krzysiek Kasprzyk
          • Votes:
            5 Vote for this issue
            Watchers:
            8 Start watching this issue

            Dates

            • Created:
              Updated:
              Days since last comment:
              20 weeks, 1 day ago