Skip to content

Stop assuming that AspectJ Advice has JoinPoint as the first argument - #34316

Closed
JoshuaChen wants to merge 3 commits into
spring-projects:mainfrom
JoshuaChen:aspectj
Closed

Stop assuming that AspectJ Advice has JoinPoint as the first argument#34316
JoshuaChen wants to merge 3 commits into
spring-projects:mainfrom
JoshuaChen:aspectj

Conversation

@JoshuaChen

@JoshuaChen JoshuaChen commented Jan 24, 2025 •
edited by sbrannen
Loading

Copy link
Copy Markdown
Contributor

After enabling @EnableTransactionManagement(mode = AdviceMode.ASPECTJ, proxyTargetClass = true), an exception occurs.

java.lang.IllegalStateException: Expecting to find 3 arguments to bind by name in advice, but actually found 2 arguments

The issue seems to be that AspectJ has changed its logic, and the first element in aspectJAdviceMethod.getParameterTypes() no longer returns JoinPoint, causing the exception.

2025-01-24T22:15:06.825+08:00 DEBUG 85696 --- [           main] .s.a.a.a.ReflectiveAspectJAdvisorFactory : Found AspectJ method: public java.lang.Object org.springframework.transaction.aspectj.AbstractTransactionAspect.ajc$around$org_springframework_transaction_aspectj_AbstractTransactionAspect$1$2a73e96c(java.lang.Object,org.aspectj.runtime.internal.AroundClosure,org.aspectj.lang.JoinPoint$StaticPart)
2025-01-24T22:15:06.825+08:00 DEBUG 85696 --- [           main] .s.a.a.a.ReflectiveAspectJAdvisorFactory : Ignoring incompatible advice method: public java.lang.Object org.springframework.transaction.aspectj.AbstractTransactionAspect.ajc$around$org_springframework_transaction_aspectj_AbstractTransactionAspect$1$2a73e96c(java.lang.Object,org.aspectj.runtime.internal.AroundClosure,org.aspectj.lang.JoinPoint$StaticPart)

java.lang.IllegalStateException: Expecting to find 3 arguments to bind by name in advice, but actually found 2 arguments.
	at org.springframework.aop.aspectj.AbstractAspectJAdvice.bindExplicitArguments(AbstractAspectJAdvice.java:472) ~[spring-aop-6.2.1.jar:6.2.1]
	at org.springframework.aop.aspectj.AbstractAspectJAdvice.bindArgumentsByName(AbstractAspectJAdvice.java:438) ~[spring-aop-6.2.1.jar:6.2.1]
	at org.springframework.aop.aspectj.AbstractAspectJAdvice.calculateArgumentBindings(AbstractAspectJAdvice.java:389) ~[spring-aop-6.2.1.jar:6.2.1]
	at org.springframework.aop.aspectj.annotation.ReflectiveAspectJAdvisorFactory.getAdvice(ReflectiveAspectJAdvisorFactory.java:314) ~[spring-aop-6.2.1.jar:6.2.1]
	at org.springframework.aop.aspectj.annotation.InstantiationModelAwarePointcutAdvisorImpl.instantiateAdvice(InstantiationModelAwarePointcutAdvisorImpl.java:152) ~[spring-aop-6.2.1.jar:6.2.1]
	at org.springframework.aop.aspectj.annotation.InstantiationModelAwarePointcutAdvisorImpl.<init>(InstantiationModelAwarePointcutAdvisorImpl.java:116) ~[spring-aop-6.2.1.jar:6.2.1]
	at org.springframework.aop.aspectj.annotation.ReflectiveAspectJAdvisorFactory.getAdvisor(ReflectiveAspectJAdvisorFactory.java:217) ~[spring-aop-6.2.1.jar:6.2.1]
	at org.springframework.aop.aspectj.annotation.ReflectiveAspectJAdvisorFactory.getAdvisors(ReflectiveAspectJAdvisorFactory.java:146) ~[spring-aop-6.2.1.jar:6.2.1]
	at org.springframework.aop.aspectj.annotation.BeanFactoryAspectJAdvisorsBuilder.buildAspectJAdvisors(BeanFactoryAspectJAdvisorsBuilder.java:116) ~[spring-aop-6.2.1.jar:6.2.1]
	at org.springframework.aop.aspectj.annotation.AnnotationAwareAspectJAutoProxyCreator.findCandidateAdvisors(AnnotationAwareAspectJAutoProxyCreator.java:95) ~[spring-aop-6.2.1.jar:6.2.1]
	at org.springframework.aop.aspectj.autoproxy.AspectJAwareAdvisorAutoProxyCreator.shouldSkip(AspectJAwareAdvisorAutoProxyCreator.java:105) ~[spring-aop-6.2.1.jar:6.2.1]
	at org.springframework.aop.framework.autoproxy.AbstractAutoProxyCreator.postProcessBeforeInstantiation(AbstractAutoProxyCreator.java:280) ~[spring-aop-6.2.1.jar:6.2.1]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.applyBeanPostProcessorsBeforeInstantiation(AbstractAutowireCapableBeanFactory.java:1150) ~[spring-beans-6.2.1.jar:6.2.1]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.resolveBeforeInstantiation(AbstractAutowireCapableBeanFactory.java:1125) ~[spring-beans-6.2.1.jar:6.2.1]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:512) ~[spring-beans-6.2.1.jar:6.2.1]
	at org.springframework.beans.factory.support.AbstractBeanFactory.lambda$doGetBean$0(AbstractBeanFactory.java:336) ~[spring-beans-6.2.1.jar:6.2.1]
	at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:289) ~[spring-beans-6.2.1.jar:6.2.1]
	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:334) ~[spring-beans-6.2.1.jar:6.2.1]
	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:204) ~[spring-beans-6.2.1.jar:6.2.1]
	at org.springframework.context.support.PostProcessorRegistrationDelegate.registerBeanPostProcessors(PostProcessorRegistrationDelegate.java:277) ~[spring-context-6.2.1.jar:6.2.1]
	at org.springframework.context.support.AbstractApplicationContext.registerBeanPostProcessors(AbstractApplicationContext.java:808) ~[spring-context-6.2.1.jar:6.2.1]
	at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:611) ~[spring-context-6.2.1.jar:6.2.1]
	at org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext.refresh(ServletWebServerApplicationContext.java:146) ~[spring-boot-3.4.1.jar:3.4.1]
	at org.springframework.boot.SpringApplication.refresh(SpringApplication.java:752) ~[spring-boot-3.4.1.jar:3.4.1]
	at org.springframework.boot.SpringApplication.refreshContext(SpringApplication.java:439) ~[spring-boot-3.4.1.jar:3.4.1]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:318) ~[spring-boot-3.4.1.jar:3.4.1]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:1361) ~[spring-boot-3.4.1.jar:3.4.1]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:1350) ~[spring-boot-3.4.1.jar:3.4.1]
	at com.example.demo.DemoApplication.main(DemoApplication.java:17) ~[classes/:na]

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 24, 2025
@JoshuaChen
Incorrect parameter count when enabling AspectJ transaction
80f5fe5 
Signed-off-by: Joshua Chen <27291761@qq.com>
@sbrannen

sbrannen commented Jan 24, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Hi @JoshuaChen,

Congratulations on submitting your first PR for the Spring Framework!. 👍

The issue seems to be that AspectJ has changed its logic, and the first element in aspectJAdviceMethod.getParameterTypes() no longer returns JoinPoint, causing the exception.

How did you come to the conclusion?

Please provide an example that worked with a previous version of AspectJ and began to fail with a recent version of AspectJ (for example 1.9.22.1 which we currently support).

In addition, you will need to introduce a JUnit-Jupiter based test in the test suite that covers this scenario.

Thanks

@sbrannen sbrannen added status: waiting-for-feedback We need additional information before we can continue in: core Issues in core modules (aop, beans, core, context, expression) labels Jan 24, 2025
@sbrannen sbrannen changed the title Incorrect parameter count when enabling AspectJ transaction Advice argument binding failure when enabling AspectJ mode for transactions Jan 24, 2025
@JoshuaChen

JoshuaChen commented Jan 24, 2025

Copy link
Copy Markdown
Contributor Author

hi~
Based on the original logic,

Class<?> firstArgType = this.aspectJAdviceMethod.getParameterTypes()[0];

AspectJ should return JoinPoint in the first parameter. However, when I ran and debugged it in my own demo app, I saw that it returned JoinPoint in the last element of getParameterTypes().
Therefore, I concluded that AspectJ has changed its logic.
To prevent AspectJ from changing the logic again and to maintain compatibility with previous versions, I used a loop to fix it.

Additionally, please give me some time to provide unit tests, and I will try to complete them.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jan 24, 2025
@JoshuaChen
Incorrect parameter count when enabling AspectJ transaction
c79f783 
Signed-off-by: Joshua Chen <27291761@qq.com>
@JoshuaChen

JoshuaChen commented Jan 25, 2025

Copy link
Copy Markdown
Contributor Author

hi~ @sbrannen

You are correct, this exception is not related to the AspectJ version.

I found that version 6.1.6 does not trigger the exception, but version 6.1.7 does. The change to isAspect() in version 6.1.7 (#32793) opened Pandora’s box. The “Advice argument binding failure” issue has always existed; it just wasn’t triggered before.

In version 6.1.6 and earlier (apologies, I couldn’t test all versions), there was no handling for the corresponding AspectJ method, so it “worked normally.”

Additionally, unit tests have been added. Please help me review the code.

@JoshuaChen
Incorrect parameter count when enabling AspectJ transaction
46140ca 
Signed-off-by: Joshua Chen <27291761@qq.com>
@snicoll snicoll self-assigned this Feb 4, 2025
@snicoll snicoll added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Feb 5, 2025
@snicoll snicoll added this to the 6.2.3 milestone Feb 5, 2025
@snicoll snicoll changed the title Advice argument binding failure when enabling AspectJ mode for transactions Stop assuming that AspectJ Advice has JointPoint as the first argument Feb 5, 2025
@snicoll snicoll mentioned this pull request Feb 5, 2025
Closed
@snicoll snicoll added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-6.1.x labels Feb 5, 2025
snicoll pushed a commit that referenced this pull request Feb 5, 2025
@JoshuaChen @snicoll
Handle arbitrary JoinPoint argument index
13ba770 
See gh-34316

Signed-off-by: Joshua Chen <27291761@qq.com>
snicoll added a commit that referenced this pull request Feb 5, 2025
@snicoll
Polish "Handle arbitrary JoinPoint argument index"
fb6e865 
See gh-34316
@snicoll snicoll closed this in 9c1346d Feb 5, 2025
@snicoll

snicoll commented Feb 5, 2025

Copy link
Copy Markdown
Member

Thanks very much for making your first contribution to Spring Framework, @JoshuaChen.

@jhoeller jhoeller changed the title Stop assuming that AspectJ Advice has JointPoint as the first argument Stop assuming that AspectJ Advice has JoinPoint as the first argument Feb 5, 2025
@JoshuaChen
JoshuaChen deleted the aspectj branch March 26, 2025 07:31
@JoshuaChen JoshuaChen mentioned this pull request Mar 26, 2025
Closed
@JoshuaChen JoshuaChen mentioned this pull request Mar 28, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@snicoll snicoll

Labels

in: core Issues in core modules (aop, beans, core, context, expression) status: backported An issue that has been backported to maintenance branches type: bug A general bug

Projects

None yet

Milestone

6.2.3

Development

Successfully merging this pull request may close these issues.

4 participants

@JoshuaChen @sbrannen @snicoll @spring-projects-issues