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

LoadTimeWeaver no longer weaves bean classes annotated with @Component #26199

Closed
emillundstrm opened this issue Dec 2, 2020 · 13 comments
Closed
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: backported An issue that has been backported to maintenance branches type: regression A bug that is also a regression
Milestone

Comments

@emillundstrm
Copy link

emillundstrm commented Dec 2, 2020

We are using a combination of XML and Annotation configuration in a Spring web application. After upgrading to Spring 5.3.1 (from Spring 4), some classes are not processed by the AspectJ Load Time Weaver. In our XML configuration we have:

<context:component-scan base-package="com.acme.spring"/>
<context:load-time-weaver aspectj-weaving="on" />

But AspectJ weaving only works for a subset of all classes. In particular, it does not work for classes found by component-scan. If the same bean is declared in XML, it works as expected.

We have managed to condense it into a small sample application:

https://github.com/emillundstrm/springaspectjtest

Running mvn jetty:run you will find that there is no message about com.acme.spring.TestBean being woven.

However, if you remove the @Component annotation from TestBean and instead declare it in ApplicationContext.xml, this is logged:

[WebAppClassLoader@58437801] debug weaving 'com.acme.spring.TestBean'
[WebAppClassLoader@58437801] weaveinfo Join point 'method-execution(void com.acme.spring.TestBean.run())' in Type 'com.acme.spring.TestBean' (TestBean.java:11) advised by around advice from 'org.springframework.transaction.aspectj.AnnotationTransactionAspect' (AbstractTransactionAspect.aj:66)
[WebAppClassLoader@58437801] debug generating class 'com.acme.spring.TestBean$AjcClosure1'

A workaround is using aspectjweaver.jar as -javaagent, but that is inconvenient if you don't control the JVM flags.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 2, 2020
@weaselmetal
Copy link

Ha, came here by chance. Isn't it the case that jetty is one of the servlet containers that doesn't contain a load time weaver and therefore requires a java agent?
See this piece of information I read just yesterday: https://docs.spring.io/spring-framework/docs/4.2.x/spring-framework-reference/html/aop.html#aop-aj-ltw-environment-generic

@emillundstrm
Copy link
Author

Ha, came here by chance. Isn't it the case that jetty is one of the servlet containers that doesn't contain a load time weaver and therefore requires a java agent?
See this piece of information I read just yesterday: https://docs.spring.io/spring-framework/docs/4.2.x/spring-framework-reference/html/aop.html#aop-aj-ltw-environment-generic

If this were the case, there'd be an error similar to:

java.lang.IllegalStateException: ClassLoader [jdk.internal.loader.ClassLoaders$AppClassLoader] does NOT provide an 'addTransformer(ClassFileTransformer)' method. Specify a custom LoadTimeWeaver or start your Java virtual machine with Spring's agent: -javaagent:spring-instrument-{version}.jar

But this is not the case. There is no error, because Jetty 9.x provides addTransformer.

And it wouldn't explain why it works from XML, but not from annotations.

@emillundstrm
Copy link
Author

emillundstrm commented Dec 3, 2020

I believe the difference from beans defined by @Component and XML stems from this code in ConfigurationClassPostProcessor:

if ((configClassAttr != null || methodMetadata != null) && beanDef instanceof AbstractBeanDefinition) {
    // Configuration class (full or lite) or a configuration-derived @Bean method
    // -> resolve bean class at this point...
    AbstractBeanDefinition abd = (AbstractBeanDefinition) beanDef;
    if (!abd.hasBeanClass()) {
        try {
            abd.resolveBeanClass(this.beanClassLoader);
        }
        catch (Throwable ex) {
            throw new IllegalStateException(
                "Cannot load configuration class: " + beanDef.getBeanClassName(), ex);
        }
    }
}

The XML bean has null configClassAttr, while the annotation bean has the value "lite". This causes the annotated bean class to be resolved, which is seemingly too early for the AspectJ LoadTimeWeaver to be active.

@goetzseb
Copy link

goetzseb commented Feb 11, 2021

Hey guys.

I stumbled across this searching for a solution to my problem which I have reported today on stackoverflow. Reading the issue description this seems to be the same issue I have. Same situation: upgrade from 4.3.14 to 5.3.3 under Tomcat 9. Only few classes are being woven. But until now I was not able to figure out which one did work.

I can confirm the workaround with the aspectjweaver agent is working, but not applicable for my use case.

Has there been any progress made on this or is there another workaround than the one with the javaagent?

@andrei-ivanov
Copy link

AFAIK, this was the workaround, from the days of the application servers, which would deploy multiple applications and you could not install a JVM agent that would affect all, while in the modern days, the agent is the recommended way.

@goetzseb
Copy link

goetzseb commented Feb 12, 2021

For me this is quite a problem as the -javaagent:aspectjweaver.jar approach rises the server startup to four times the normal duration (60sec. vs 250sec.). The class loading mechanism or the weaving must have changed somewhere between 5.1.20.RELEASE and 5.2.0.RELEASE. But I am not deep enough into the spring framework to investigate this any further.

@sbrannen sbrannen added the in: core Issues in core modules (aop, beans, core, context, expression) label Feb 12, 2021
@sbrannen
Copy link
Member

sbrannen commented Feb 12, 2021

@emillundstrm, thanks for sharing your analysis.

The XML bean has null configClassAttr, while the annotation bean has the value "lite". This causes the annotated bean class to be resolved, which is seemingly too early for the AspectJ LoadTimeWeaver to be active.

If that's the case, this might be a regression caused by the changes in 40c6213, stemming from the fact that a @Component class that is picked up via @ComponentScan is considered to be a candidate configuration class in "lite" mode (i.e., without the @Configuration annotation and even without any @Bean methods) and therefore has its class eagerly loaded.

@jhoeller, thoughts?

@sbrannen sbrannen changed the title Spring LoadTimeWeaver does not weave bean class created by ComponentScan annotation Spring LoadTimeWeaver no longer weaves bean class picked up via @ComponentScan Feb 12, 2021
@sbrannen sbrannen self-assigned this Feb 16, 2021
@sbrannen sbrannen added type: regression A bug that is also a regression and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Feb 16, 2021
@sbrannen sbrannen added this to the 5.3.5 milestone Feb 16, 2021
@mcolbert-unicon
Copy link

Ran into this issue yesterday. We upgraded to 5.2.12 recently and noticed yesterday that our app would stop working after a while in our test environment. Threads were waiting on database connections from the connection pool. It turns out that our @Transactional beans were no longer being advised by org.springframework.transaction.aspectj.AnnotationTransactionAspect and therefore none of our connections were transactional. Glad we caught it and glad there is a fix coming. We dropped back to 5.1.20 based on the reporting by @goetzseb.

@sbrannen
Copy link
Member

Has there been any progress made on this

We will be working on a fix for Spring Framework 5.3.5 and potentially backporting to 5.2.14.

or is there another workaround than the one with the javaagent?

Well, I haven't tried this out, but based on my initial analysis of the regression, it appears that annotating your class with @javax.inject.Named instead of @Component or @Service might be a viable workaround. Note, however, that this workaround would not apply to replacing @Controller, @RestController, or @Repository with @Named.

@goetzseb, please try annotating some of your component scanned classes with @Named and let us know if that works for you as an interim solution.

@sbrannen
Copy link
Member

sbrannen commented Feb 18, 2021

I have confirmed that this is a regression introduced in Spring Framework 5.2.0.

Well, I haven't tried this out, but based on my initial analysis of the regression, it appears that annotating your class with @javax.inject.Named instead of @Component or @Service might be a viable workaround. Note, however, that this workaround would not apply to replacing @Controller, @RestController, or @Repository with @Named.

Using @Named instead of @Component or @Service works with component scanning.

I have also confirmed that the mere presence or meta-presence of @Component on a bean will result in the reported behavior, even if the bean is not registered via component scanning.

@sbrannen sbrannen changed the title Spring LoadTimeWeaver no longer weaves bean class picked up via @ComponentScan Spring LoadTimeWeaver no longer weaves bean class annotated with @Component Feb 21, 2021
@spring-projects-issues spring-projects-issues added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-5.2.x labels Feb 21, 2021
@sbrannen sbrannen changed the title Spring LoadTimeWeaver no longer weaves bean class annotated with @Component LoadTimeWeaver no longer weaves bean classes annotated with @Component Feb 21, 2021
sbrannen added a commit to sbrannen/spring-framework that referenced this issue Feb 22, 2021
Since Spring Framework 5.2, the LoadTimeWeaver no longer weaves bean
classes annotated with @component. This is a regression caused by the
changes in 40c6213, stemming from the fact that any class annotated
or meta-annotated with @component is considered to be a candidate
configuration class in 'configuration lite' mode (i.e., a class without
the @configuration annotation and without any @bean methods) and
therefore now has its class eagerly loaded. This results in the class
being loaded before the LoadTimeWeaver has a chance to weave it.

This commit fixes this regression by explicitly avoiding eager class
loading for any 'lite' @configuration or @component class without @bean
methods.

Closes spring-projectsgh-26199
@sbrannen
Copy link
Member

This regression has been fixed in master and 5.2.x.

Feel free to try it out in the upcoming snapshots for Spring Framework 5.3.5 and 5.2.14, and please report back on whether it works for you.

@goetzseb
Copy link

Great to hear and thank you very much!

This was referenced Mar 17, 2021
lxbzmy pushed a commit to lxbzmy/spring-framework that referenced this issue Mar 26, 2022
Since Spring Framework 5.2, the LoadTimeWeaver no longer weaves bean
classes annotated with @component. This is a regression caused by the
changes in 40c6213, stemming from the fact that any class annotated
or meta-annotated with @component is considered to be a candidate
configuration class in 'configuration lite' mode (i.e., a class without
the @configuration annotation and without any @bean methods) and
therefore now has its class eagerly loaded. This results in the class
being loaded before the LoadTimeWeaver has a chance to weave it.

This commit fixes this regression by explicitly avoiding eager class
loading for any 'lite' @configuration or @component class without @bean
methods.

Closes spring-projectsgh-26199
@pdeneve
Copy link
Contributor

pdeneve commented Aug 14, 2023

This regression has been fixed in master and 5.2.x.

Feel free to try it out in the upcoming snapshots for Spring Framework 5.3.5 and 5.2.14, and please report back on whether it works for you.

@sbrannen Doesn't work for me in either versions. It can be reproduced by cloning this repo, checking out branch 5.3.5 or 5.2.14 and running mvn test -pl spring.ltw.

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) status: backported An issue that has been backported to maintenance branches type: regression A bug that is also a regression
Projects
None yet
Development

No branches or pull requests

8 participants