Skip to content

Reliably apply AspectJ weaving to @Component classes with LoadTimeWeaver setup on Tomcat #29609

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

Open
kuku20045 opened this issue Nov 29, 2022 · 12 comments
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement

Comments

@kuku20045
Copy link

Affects: 6.0.2
We are trying to migrate from 5.3.19 to 6.0.2 and it seems that @service beans and @component beans are not woven anymore.
In our case, the result is that our @Transactionnal methods are no longer advised by the AnnotationTransactionAspect.

Looks like the issue was already present in older spring version.
I use the -javaagent:aspectjweaver.jar as a workaround but it is clearly not ideal for our project.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Nov 29, 2022
@kuku20045
Copy link
Author

kuku20045 commented Nov 30, 2022

@sbrannen sbrannen added the in: core Issues in core modules (aop, beans, core, context, expression) label Nov 30, 2022
@rlm6
Copy link

rlm6 commented Jan 3, 2023

Same issue here,

  • if we set java agent through command line, it works
  • if we set java agent through code (attach to running VM thanks to org.aspectj.weaver.loadtime.Agent.getInstrumentation()), it works
  • if we define it in context configuration through annotation @EnableLoadTimeWeaving, it doesn't work

The most weird thing is that if the bean is declared in context with @bean annotation, it will be woven, if it's declared with @component and scanned with a base package, it will not be woven.

It also affects version 6.0.3

@pdeneve
Copy link
Contributor

pdeneve commented Jun 26, 2023

I've analysed this issue and here's the report, including a workaround. Analysis happened against version 6.0.10.

In PostProcessorRegistrationDelegate::invokeBeanFactoryPostProcessors, there are the following steps:

  1. Invoke BeanDefinitionRegistryPostProcessors that have been registered with ConfigurableApplicationContext::addBeanFactoryPostProcessor
  2. Find BeanDefinitionRegistryPostProcessors that have been registered up until this point (*)
  3. Invoke PriorityOrdered BeanDefinitionRegistryPostProcessors found in the previous step.
  4. Find again BeanDefinitionRegistryPostProcessors. In the process, for every BeanDefinition there is a check if the bean is a FactoryBean, which causes eventually the Class::forName method being called for the bean.
  5. Invoke Ordered BeanDefinitionRegistryPostProcessors from the previous step that were not invoked in the third step.
  6. Further processing ...

Workaround:

The LoadTimeWeaver must be registered before the first class for which aspects have to be applied is loaded by the class loader (before step 4). In order to do so, add a PriorityOrdered BeanDefinitionRegistryPostProcessor that adds the LoadTimeWeaver to the application context and enables AspectJ load time weaving (to ensure transformation happens when in step 3). I've tested this with AnnotationConfigApplicationContext and AnnotationConfigWebApplicationContext, cf. this repository (sub projects spring.ltw and spring.web.ltw).

(*) This includes ConfigurationClassPostProcessor. ConfigurationClassPostProcessor will register BeanDefinitions for for beans configured via @Configuration classes. In order to do so, it inspects classes via ClassLoader::getResourceAsStream. In Tomcat, ClassLoader::getResourceAsStream causes transformers to be applied. AspectJ remembers classes that have been transformed. When in step 4 (or later) when classes are loaded via Class::forName, when a class was previously transformed because of the invocation of ClassLoader::getResourceAsStream, AspectJ doesn't transform the class again and returns the non-transformed class (when the -debug flag is on in aop.xml, it prints "cannot weave class ..."). To work around this, you need to ensure that ConfigurationClassPostProcessor is invoked before the load time weaving is enabled. This can be accomplished by giving the lowest order to the BeanDefinitionRegistryPostProcessor that configures the load time weaving.

@kuku20045
Copy link
Author

I have tried your workaroud and it works perfectly, thanks for your time.

@pdeneve
Copy link
Contributor

pdeneve commented Jun 29, 2023

I have tried your workaroud and it works perfectly, thanks for your time.

You're welcome. Maybe keep the issue open in order to keep it on the radar for a structural solution.

@kuku20045 kuku20045 reopened this Jun 29, 2023
@pop1213
Copy link

pop1213 commented Aug 6, 2023

I've analysed this issue and here's the report, including a workaround. Analysis happened against version 6.0.10.

In PostProcessorRegistrationDelegate::invokeBeanFactoryPostProcessors, there are the following steps:

  1. Invoke BeanDefinitionRegistryPostProcessors that have been registered with ConfigurableApplicationContext::addBeanFactoryPostProcessor
  2. Find BeanDefinitionRegistryPostProcessors that have been registered up until this point (*)
  3. Invoke PriorityOrdered BeanDefinitionRegistryPostProcessors found in the previous step.
  4. Find again BeanDefinitionRegistryPostProcessors. In the process, for every BeanDefinition there is a check if the bean is a FactoryBean, which causes eventually the Class:forName method being called for the bean.
  5. Invoke Ordered BeanDefinitionRegistryPostProcessors from the previous step that were not invoked in the third step.
  6. Further processing ...

Workaround:

The LoadTimeWeaver must be registered before the first class for which aspects have to be applied is loaded by the class loader (before step 4). In order to do so, add a PriorityOrdered BeanDefinitionRegistryPostProcessor that adds the LoadTimeWeaver to the application context and enables AspectJ load time weaving (to ensure transformation happens when in step 3). I've tested this with AnnotationConfigApplicationContext and AnnotationConfigWebApplicationContext, cf. this repository (sub projects spring.ltw and spring.web.ltw).

(*) This includes ConfigurationClassPostProcessor. ConfigurationClassPostProcessor will register BeanDefinitions for for beans configured via @Configuration classes. In order to do so, it inspects classes via ClassLoader::getResourceAsStream. In Tomcat, ClassLoader::getResourceAsStream causes transformers to be applied. AspectJ remembers classes that have been transformed. When in step 4 (or later) when classes are loaded via Class:forName, when a class was previously transformed because of the invocation of ClassLoader::getResourceAsStream, AspectJ doesn't transform the class again and returns the non-transformed class (when the -debug flag is on in aop.xml, it prints "cannot weave class ..."). To work around this, you need to ensure that ConfigurationClassPostProcessor is invoked before the load time weaving is enabled. This can be accomplished by giving the lowest order to the BeanDefinitionRegistryPostProcessor that configures the load time weaving.

@pdeneve Why is it that only when applying transformers in the fourth step can LTW (Load-Time Weaving) take effect? I look forward to your answer. Thank you.

@pdeneve
Copy link
Contributor

pdeneve commented Aug 7, 2023

@pdeneve Why is it that only when applying transformers in the fourth step can LTW (Load-Time Weaving) take effect? I look forward to your answer. Thank you.

In essence, the transformation is applied for newly loaded classes from the moment AspectJ load time weaving is enabled. Because @Component classes are loaded in step 4, you have to ensure the load time weaving is enabled before step 4.

@pop1213
Copy link

pop1213 commented Aug 8, 2023

In essence, the transformation is applied for newly loaded classes from the moment AspectJ load time weaving is enabled.
Because classes for @Beans are loaded in step 4, you have to ensure the load time weaving is enabled before step 4.

I think I understand what you mean, thank you. By the way, I have a question about the @EnableLoadTimeWeaving annotation. From its source code, it can be seen that this annotation registers a singleton LoadTimeWeaver.As it is well known, the creation of singleton beans is completed in the final stage of the Spring container refreshing, which is much later than the mentioned step 4. This means that ltw may not take effect for most beans. Does this imply that the @EnableLoadTimeWeaving annotation has some imperfections?

@pdeneve
Copy link
Contributor

pdeneve commented Aug 9, 2023

I think I understand what you mean, thank you. By the way, I have a question about the @EnableLoadTimeWeaving annotation. From its source code, it can be seen that this annotation registers a singleton LoadTimeWeaver.As it is well known, the creation of singleton beans is completed in the final stage of the Spring container refreshing, which is much later than the mentioned step 4. This means that ltw may not take effect for most beans. Does this imply that the @EnableLoadTimeWeaving annotation has some imperfections?

Yes, I in that respect it has some imperfections.

@pdeneve
Copy link
Contributor

pdeneve commented Aug 14, 2023

I've found a better workaround, one that does not involve adding a BeanDefinitionRegistryPostProcessor.

In AbstractApplicationContext::prepareBeanFactory there's a check if a bean with the name loadTimeWeaver is present, and if so a temporary class loader (other than the class loader for which the weaving will be enabled) is registered that will be used for type matching, i.e. also to determine if a bean is a FactoryBean.

The workaround involves making sure that DefaultContextLoadTimeWeaver.class and AspectJWeavingEnabler.class are registered before the application context is refreshed. However, one must ensure that DefaultContextLoadTimeWeaver.class is registered with the bean name loadTimeWeaver. In order to do so, define an @Component("loadTimeWeaver") class that extends DefaultContextLoadTimeWeaver and register that class instead.

The workaround works for AnnotationConfig(Web)ApplicationContexts and Spring Boot applications. However, for Spring Boot applications, suddenly for some auto-configured beans an IllegalAccessError will be thrown and you have to define them manually, e.g. JdbcConnectionDetails. I've reported an issue in Spring Boot project for this.

I've updated the reference repository with the new workaround.

@pop1213
Copy link

pop1213 commented Aug 15, 2023

Using the temporary class loader is indeed a better idea. I haven't come across the issue you mentioned where some auto-configured beans cannot be found. If you have already raised the issue, could you provide a link ?

@pdeneve
Copy link
Contributor

pdeneve commented Aug 19, 2023

Yet another workaround involves using XML configuration. Unfortunately it's not possible to use this workaround in a Spring Boot application, because Spring Boot only allows for registering classes upfront, not XML configuration files.
Sample project can be found here (module spring.ltw.xml).

@jhoeller jhoeller changed the title LoadTimeWeaver no longer weaves bean classes annotated with @Component Reliably apply AspectJ weaving to @Component classes with LoadTimeWeaver setup on Tomcat Jan 12, 2024
@jhoeller jhoeller added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 12, 2024
@jhoeller jhoeller added this to the 6.x Backlog milestone Jan 12, 2024
@jhoeller jhoeller modified the milestones: 6.x Backlog, General Backlog Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

7 participants