Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pointcut evaluation fails against AbstractHandlerMethodMapping$MappingRegistry with AspectJ 1.8.10 [SPR-15019] #19586

Closed
spring-issuemaster opened this issue Dec 14, 2016 · 10 comments

Comments

@spring-issuemaster
Copy link
Collaborator

commented Dec 14, 2016

Eduard Dudar opened SPR-15019 and commented

Two days ago a context in some of our builds stopped starting up. Further investigation led to the fact that AspectJ 1.8.10 was released 2 days ago and our Gradle script picked it up through "1.8.+" dependency. When spring-web is in dependency list the following exception occurs. We use spring-boot but the exception is in core classes thus the ticket here. Rolling back AspectJ 1.8.9 fixes this issue.

org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'requestMappingHandlerMapping' defined in class path resource [org/springframework/boot/autoconfigure/web/WebMvcAutoConfiguration$EnableWebMvcConfiguration.class]: Initialization of bean failed; nested exception is java.lang.IllegalStateException: Expected raw type form of org.springframework.web.servlet.handler.AbstractHandlerMethodMapping$MappingRegistry
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:562) ~[spring-beans-4.3.4.RELEASE.jar:4.3.4.RELEASE]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:482) ~[spring-beans-4.3.4.RELEASE.jar:4.3.4.RELEASE]
	at org.springframework.beans.factory.support.AbstractBeanFactory$1.getObject(AbstractBeanFactory.java:306) ~[spring-beans-4.3.4.RELEASE.jar:4.3.4.RELEASE]
	at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:230) ~[spring-beans-4.3.4.RELEASE.jar:4.3.4.RELEASE]
	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:302) ~[spring-beans-4.3.4.RELEASE.jar:4.3.4.RELEASE]
	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:197) ~[spring-beans-4.3.4.RELEASE.jar:4.3.4.RELEASE]
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingletons(DefaultListableBeanFactory.java:754) ~[spring-beans-4.3.4.RELEASE.jar:4.3.4.RELEASE]
	at org.springframework.context.support.AbstractApplicationContext.finishBeanFactoryInitialization(AbstractApplicationContext.java:866) ~[spring-context-4.3.4.RELEASE.jar:4.3.4.RELEASE]
	at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:542) ~[spring-context-4.3.4.RELEASE.jar:4.3.4.RELEASE]
	at org.springframework.boot.context.embedded.EmbeddedWebApplicationContext.refresh(EmbeddedWebApplicationContext.java:122) ~[spring-boot-1.4.2.RELEASE.jar:1.4.2.RELEASE]
	at org.springframework.boot.SpringApplication.refresh(SpringApplication.java:761) [spring-boot-1.4.2.RELEASE.jar:1.4.2.RELEASE]
	at org.springframework.boot.SpringApplication.refreshContext(SpringApplication.java:371) [spring-boot-1.4.2.RELEASE.jar:1.4.2.RELEASE]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:315) [spring-boot-1.4.2.RELEASE.jar:1.4.2.RELEASE]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:1186) [spring-boot-1.4.2.RELEASE.jar:1.4.2.RELEASE]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:1175) [spring-boot-1.4.2.RELEASE.jar:1.4.2.RELEASE]
Caused by: java.lang.IllegalStateException: Expected raw type form of org.springframework.web.servlet.handler.AbstractHandlerMethodMapping$MappingRegistry
	at org.aspectj.weaver.reflect.JavaLangTypeToResolvedTypeConverter.fromType(JavaLangTypeToResolvedTypeConverter.java:71) ~[aspectjweaver-1.8.10.jar:1.8.10]
	at org.aspectj.weaver.reflect.Java15GenericSignatureInformationProvider.getGenericReturnType(Java15GenericSignatureInformationProvider.java:61) ~[aspectjweaver-1.8.10.jar:1.8.10]
	at org.aspectj.weaver.reflect.ReflectionBasedResolvedMemberImpl.getGenericReturnType(ReflectionBasedResolvedMemberImpl.java:122) ~[aspectjweaver-1.8.10.jar:1.8.10]
	at org.aspectj.weaver.ResolvedMemberImpl.parameterizedWith(ResolvedMemberImpl.java:789) ~[aspectjweaver-1.8.10.jar:1.8.10]
	at org.aspectj.weaver.ResolvedMemberImpl.parameterizedWith(ResolvedMemberImpl.java:742) ~[aspectjweaver-1.8.10.jar:1.8.10]
	at org.aspectj.weaver.ReferenceType.getDeclaredMethods(ReferenceType.java:859) ~[aspectjweaver-1.8.10.jar:1.8.10]
	at org.aspectj.weaver.ResolvedType$MethodGetterIncludingItds.get(ResolvedType.java:252) ~[aspectjweaver-1.8.10.jar:1.8.10]
	at org.aspectj.weaver.ResolvedType$MethodGetterIncludingItds.get(ResolvedType.java:250) ~[aspectjweaver-1.8.10.jar:1.8.10]
	at org.aspectj.weaver.Iterators$4$1.hasNext(Iterators.java:213) ~[aspectjweaver-1.8.10.jar:1.8.10]
	at org.aspectj.weaver.Iterators$6.hasNext(Iterators.java:288) ~[aspectjweaver-1.8.10.jar:1.8.10]
	at org.aspectj.weaver.Iterators$4.hasNext(Iterators.java:230) ~[aspectjweaver-1.8.10.jar:1.8.10]
	at org.aspectj.weaver.ResolvedType.lookupResolvedMember(ResolvedType.java:619) ~[aspectjweaver-1.8.10.jar:1.8.10]
	at org.aspectj.weaver.JoinPointSignatureIterator.findSignaturesFromSupertypes(JoinPointSignatureIterator.java:192) ~[aspectjweaver-1.8.10.jar:1.8.10]
	at org.aspectj.weaver.JoinPointSignatureIterator.findSignaturesFromSupertypes(JoinPointSignatureIterator.java:229) ~[aspectjweaver-1.8.10.jar:1.8.10]
	at org.aspectj.weaver.JoinPointSignatureIterator.hasNext(JoinPointSignatureIterator.java:68) ~[aspectjweaver-1.8.10.jar:1.8.10]
	at org.aspectj.weaver.patterns.SignaturePattern.matches(SignaturePattern.java:317) ~[aspectjweaver-1.8.10.jar:1.8.10]
	at org.aspectj.weaver.patterns.KindedPointcut.matchInternal(KindedPointcut.java:197) ~[aspectjweaver-1.8.10.jar:1.8.10]
	at org.aspectj.weaver.patterns.Pointcut.match(Pointcut.java:137) ~[aspectjweaver-1.8.10.jar:1.8.10]
	at org.aspectj.weaver.internal.tools.PointcutExpressionImpl.getShadowMatch(PointcutExpressionImpl.java:319) ~[aspectjweaver-1.8.10.jar:1.8.10]
	at org.aspectj.weaver.internal.tools.PointcutExpressionImpl.matchesExecution(PointcutExpressionImpl.java:129) ~[aspectjweaver-1.8.10.jar:1.8.10]
	at org.aspectj.weaver.internal.tools.PointcutExpressionImpl.matchesMethodExecution(PointcutExpressionImpl.java:110) ~[aspectjweaver-1.8.10.jar:1.8.10]
	at org.springframework.aop.aspectj.AspectJExpressionPointcut.getShadowMatch(AspectJExpressionPointcut.java:416) ~[spring-aop-4.3.4.RELEASE.jar:4.3.4.RELEASE]
	at org.springframework.aop.aspectj.AspectJExpressionPointcut.matches(AspectJExpressionPointcut.java:271) ~[spring-aop-4.3.4.RELEASE.jar:4.3.4.RELEASE]
	at org.springframework.aop.support.AopUtils.canApply(AopUtils.java:241) ~[spring-aop-4.3.4.RELEASE.jar:4.3.4.RELEASE]
	at org.springframework.aop.support.AopUtils.canApply(AopUtils.java:279) ~[spring-aop-4.3.4.RELEASE.jar:4.3.4.RELEASE]
	at org.springframework.aop.support.AopUtils.findAdvisorsThatCanApply(AopUtils.java:311) ~[spring-aop-4.3.4.RELEASE.jar:4.3.4.RELEASE]
	at org.springframework.aop.framework.autoproxy.AbstractAdvisorAutoProxyCreator.findAdvisorsThatCanApply(AbstractAdvisorAutoProxyCreator.java:118) ~[spring-aop-4.3.4.RELEASE.jar:4.3.4.RELEASE]
	at org.springframework.aop.framework.autoproxy.AbstractAdvisorAutoProxyCreator.findEligibleAdvisors(AbstractAdvisorAutoProxyCreator.java:88) ~[spring-aop-4.3.4.RELEASE.jar:4.3.4.RELEASE]
	at org.springframework.aop.framework.autoproxy.AbstractAdvisorAutoProxyCreator.getAdvicesAndAdvisorsForBean(AbstractAdvisorAutoProxyCreator.java:69) ~[spring-aop-4.3.4.RELEASE.jar:4.3.4.RELEASE]
	at org.springframework.aop.framework.autoproxy.AbstractAutoProxyCreator.wrapIfNecessary(AbstractAutoProxyCreator.java:347) ~[spring-aop-4.3.4.RELEASE.jar:4.3.4.RELEASE]
	at org.springframework.aop.framework.autoproxy.AbstractAutoProxyCreator.postProcessAfterInitialization(AbstractAutoProxyCreator.java:299) ~[spring-aop-4.3.4.RELEASE.jar:4.3.4.RELEASE]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.applyBeanPostProcessorsAfterInitialization(AbstractAutowireCapableBeanFactory.java:422) ~[spring-beans-4.3.4.RELEASE.jar:4.3.4.RELEASE]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.initializeBean(AbstractAutowireCapableBeanFactory.java:1588) ~[spring-beans-4.3.4.RELEASE.jar:4.3.4.RELEASE]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:553) ~[spring-beans-4.3.4.RELEASE.jar:4.3.4.RELEASE]
	... 15 common frames omitted

Affects: 4.2.8, 4.3.4

Issue Links:

  • #19616 IllegalStateException after upgrading aspectjweaver to 1.8.10 ("is duplicated by")
  • #13973 Unsafe fallback pointcut construction in AspectJExpressionPointcut
  • #17693 When use a @args as pointcut, there is case that occur a NPE at calling the unrelated method
  • #16166 Provide public methods to register and un-register handler method mappings
  • #20525 Upgrade to AspectJ 1.9 beta 7

Backported to: 4.2.9

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 14, 2016

Juergen Hoeller commented

This looks like a regression in AspectJ itself, with an IllegalStateException appearing against a target signature that it cannot match... We're defensive against ReflectionWorldException already, simply not assuming a match then; I guess we have to extend that catch block to IllegalStateException for 4.3.5 and 4.2.9.

Andy Clement, any insight here? Also, AspectJ 1.8.10 doesn't seem to show up on Maven Central yet...

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 14, 2016

Juergen Hoeller commented

I've added defensive catch (IllegalStateException) blocks to our match attempts in AspectJPointcutExpression. This should achieve compatibility with AspectJ 1.8.10 for a start. To be backported to 4.3.5 later this week.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 14, 2016

Andy Clement commented

Looks like a new diagnostic assert I put into 1.8.10 is firing on something here. So something we were previously silently getting away with.

You can add the defensive catch but something weird is going on that is worth getting to the bottom of. Eduard, I don't suppose you have a small testcase that shows this occurring? The Spring Framework tests themselves seem to pass with 1.8.10.

(Juergen, I can see it in central -> http://search.maven.org/#artifactdetails%7Corg.aspectj%7Caspectjweaver%7C1.8.10%7Cjar )

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 14, 2016

Juergen Hoeller commented

I can see 1.8.10 in Maven Central now as well. Could it have synced that late? Anyway, it's there now...

In terms of defensive catching in our AspectJExpressionPointcut, we're doing this for ReflectionWorldException and others as well. If in doubt, we assumed that the pointcut simply wouldn't match before already (e.g. #13973, #17693). So it's not unusual for us to catch IllegalStateException there either. I'm primarily worried about a seamless upgrade experience, so rather opting for silently ignoring what we silently ignored before.

Of course, it makes sense to find out about the root cause in any case!

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 14, 2016

Juergen Hoeller commented

The class that AspectJ chokes on here - AbstractHandlerMethodMapping$MappingRegistry - has been introduced in 4.2 as part of #16166. I'm not sure why pointcut evaluation would even touch that class though since it's not a managed bean itself, rather just an inner delegate class.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 14, 2016

Eduard Dudar commented

Andy, check out this simple project: https://github.com/edudar/aspectj-failure-example. The key there is specific pointcut defined in
AspectCheck.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 14, 2016

Andy Clement commented

Eduard, thanks so much for the fast turnaround on that. I've recreated it with your project. I'll dig deeper.

I'm primarily worried about a seamless upgrade experience
Yes, me too...

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 15, 2016

Andy Clement commented

I've raised https://bugs.eclipse.org/bugs/show_bug.cgi?id=509327 to cover this in AspectJ. The problem is due to inner types of parameterized types. Whether the inner type is parameterized or not, it still presents itself as parameterized if there is an outer type that is parameterized, this was tripping up some converter code that maps from reflectively resolved types to AspectJ types. Spring uses reflectively discovered types when matching whereas 'regular AspectJ' uses information retrieved directly from the bytecode. We have far fewer tests for the reflective option than the bytecode option. Here, specifically it is the AbstractHandlerMethodMapping type that is generic and the inner type is MappingRegistry.

Based on that I distilled Eduards testcode down to a minimal regression test. The question is, now that the diagnostic has served its purpose, should I sort it out and do a quick 1.8.11 including the necessary changes to avoid all those folks on older spring that might trip over it when using 1.8.10.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 16, 2016

Juergen Hoeller commented

Andy Clement, a 1.8.11 release certainly wouldn't hurt... but I wouldn't rush it just for this either. Let's rather release it once a bit more feedback came in. After all, most people use specific dependency versions and would first of all have to discover 1.8.10 and choose to upgrade to it.

BTW, are there further assertions of this kind in the pointcut matching code path? More specifically, are all other such assertions also throwing IllegalStateException?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.