spring-projects / spring-framework

Spring Framework
https://spring.io/projects/spring-framework
Apache License 2.0
56.46k stars 38.09k forks source link

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

Open kuku20045 opened 1 year ago

kuku20045 commented 1 year ago

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.

kuku20045 commented 1 year ago
rlm6 commented 1 year ago

Same issue here,

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 commented 1 year ago

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 commented 1 year ago

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

pdeneve commented 1 year ago

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.

pop1213 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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