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

Add support for private init/destroy methods in AOT mode #30654

Closed
wants to merge 1 commit into from
Closed

Add support for private init/destroy methods in AOT mode #30654

wants to merge 1 commit into from

Conversation

essobedo
Copy link

@essobedo essobedo commented Jun 13, 2023

Motivation

In AOT mode, if the init/destroy methods are private and auto-discovered thanks to an annotation, the generated code sets the init and destroy method names with the fully qualified name of the declaring class as prefix to prevent naming conflicts which causes an error of the following type when starting the application (native image or with -Dspring.aot.enabled=true):

org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'demoBean': Could not find an init method named 'com.example.demo.DemoBean.init' on bean with name 'demoBean'
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.initializeBean(AbstractAutowireCapableBeanFactory.java:1770) ~[spring-beans-6.0.10-SNAPSHOT.jar!/:6.0.10-SNAPSHOT]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:598) ~[spring-beans-6.0.10-SNAPSHOT.jar!/:6.0.10-SNAPSHOT]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:520) ~[spring-beans-6.0.10-SNAPSHOT.jar!/:6.0.10-SNAPSHOT]
	at org.springframework.beans.factory.support.AbstractBeanFactory.lambda$doGetBean$0(AbstractBeanFactory.java:326) ~[spring-beans-6.0.10-SNAPSHOT.jar!/:6.0.10-SNAPSHOT]
	at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:234) ~[spring-beans-6.0.10-SNAPSHOT.jar!/:6.0.10-SNAPSHOT]
	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:324) ~[spring-beans-6.0.10-SNAPSHOT.jar!/:6.0.10-SNAPSHOT]
	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:200) ~[spring-beans-6.0.10-SNAPSHOT.jar!/:6.0.10-SNAPSHOT]
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingletons(DefaultListableBeanFactory.java:973) ~[spring-beans-6.0.10-SNAPSHOT.jar!/:6.0.10-SNAPSHOT]
	at org.springframework.context.support.AbstractApplicationContext.finishBeanFactoryInitialization(AbstractApplicationContext.java:941) ~[spring-context-6.0.10-SNAPSHOT.jar!/:6.0.10-SNAPSHOT]
	at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:608) ~[spring-context-6.0.10-SNAPSHOT.jar!/:6.0.10-SNAPSHOT]
	at org.springframework.boot.SpringApplication.refresh(SpringApplication.java:733) ~[spring-boot-3.2.0-SNAPSHOT.jar!/:3.2.0-SNAPSHOT]
	at org.springframework.boot.SpringApplication.refreshContext(SpringApplication.java:435) ~[spring-boot-3.2.0-SNAPSHOT.jar!/:3.2.0-SNAPSHOT]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:311) ~[spring-boot-3.2.0-SNAPSHOT.jar!/:3.2.0-SNAPSHOT]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:1305) ~[spring-boot-3.2.0-SNAPSHOT.jar!/:3.2.0-SNAPSHOT]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:1294) ~[spring-boot-3.2.0-SNAPSHOT.jar!/:3.2.0-SNAPSHOT]
	at com.example.demo.DemoApplication.main(DemoApplication.java:10) ~[classes!/:0.0.1-SNAPSHOT]
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[na:na]
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77) ~[na:na]
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[na:na]
	at java.base/java.lang.reflect.Method.invoke(Method.java:568) ~[na:na]
	at org.springframework.boot.loader.MainMethodRunner.run(MainMethodRunner.java:49) ~[demo-0.0.1-SNAPSHOT.jar:0.0.1-SNAPSHOT]
	at org.springframework.boot.loader.Launcher.launch(Launcher.java:95) ~[demo-0.0.1-SNAPSHOT.jar:0.0.1-SNAPSHOT]
	at org.springframework.boot.loader.Launcher.launch(Launcher.java:58) ~[demo-0.0.1-SNAPSHOT.jar:0.0.1-SNAPSHOT]
	at org.springframework.boot.loader.JarLauncher.main(JarLauncher.java:65) ~[demo-0.0.1-SNAPSHOT.jar:0.0.1-SNAPSHOT]
Caused by: org.springframework.beans.factory.support.BeanDefinitionValidationException: Could not find an init method named 'com.example.demo.DemoBean.init' on bean with name 'demoBean'
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.invokeCustomInitMethod(AbstractAutowireCapableBeanFactory.java:1849) ~[spring-beans-6.0.10-SNAPSHOT.jar!/:6.0.10-SNAPSHOT]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.invokeInitMethods(AbstractAutowireCapableBeanFactory.java:1826) ~[spring-beans-6.0.10-SNAPSHOT.jar!/:6.0.10-SNAPSHOT]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.initializeBean(AbstractAutowireCapableBeanFactory.java:1766) ~[spring-beans-6.0.10-SNAPSHOT.jar!/:6.0.10-SNAPSHOT]

Steps to reproduce:

  • Create a demo project from https://start.spring.io/ with "GraalVM Native Support" as the only dependency
  • Add the following Configuration and Bean classes

Configuration class:

@Configuration
public class DemoConfiguration {

    @Bean
    public DemoBean demoBean() {
        return new DemoBean();
    }
}

Bean class:

public class DemoBean {

    @PostConstruct
    private void init() {
        System.out.println("Init DemoBean");
    }

    @PreDestroy
    private void destroy() {
        System.out.println("Destroy DemoBean");
    }
}

Modifications:

  • Uses the name of the method instead of the identifier of the LifecycleElement even if it won't solve the potential naming conflict but it is a more specific use case
  • Adds a unit test to illustrate the use case

Result

The init and destroy methods are called when starting the application (native image or with -Dspring.aot.enabled=true).

@pivotal-cla
Copy link

@essobedo Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 13, 2023
@sbrannen sbrannen added type: bug A general bug theme: aot An issue related to Ahead-of-time processing labels Jun 13, 2023
@sbrannen sbrannen self-assigned this Jun 13, 2023
@sbrannen sbrannen changed the title Add support of init/destroy private methods in AOT mode Add support of private init/destroy methods in AOT mode Jun 13, 2023
@sbrannen sbrannen changed the title Add support of private init/destroy methods in AOT mode Add support for private init/destroy methods in AOT mode Jun 13, 2023
@sbrannen sbrannen marked this pull request as draft June 13, 2023 15:34
@sbrannen
Copy link
Member

Hi @essobedo,

Congratulations on submitting your first PR to the Spring Framework! 👍

  • Uses the name of the method instead of the identifier of the LifecycleElement even if it won't solve the potential naming conflict but it is a more specific use case

I'm not certain that this is the correct approach to this issue.

I believe we may need to keep the fully-qualified method name in order to support multiple private init/destroy methods declared at different levels within a class hierarchy.

In light of that, I have marked this PR as a draft to allow us to further investigate the issue.

Related Issues

@sbrannen sbrannen added status: pending-design-work Needs design work before any code can be developed and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jun 13, 2023
@sbrannen sbrannen added this to the 6.0.11 milestone Jun 13, 2023
@essobedo
Copy link
Author

@sbrannen I understand, I was wondering if I should go beyond or not, but my idea was just to reduce the limitations with very limited impact. Let me try to cover all use cases. Thank you for your quick feedback.

@snicoll
Copy link
Member

snicoll commented Jun 19, 2023

@essobedo how is it going? Do you need help?

@sbrannen
Copy link
Member

FYI: I am already looking into this issue, and I think I will be taking a different approach to addressing this.

@snicoll
Copy link
Member

snicoll commented Jun 19, 2023

Can we please close it to avoid duplicated efforts?

@essobedo
Copy link
Author

Sorry, I had to finish something else first, I planned to work on it tomorrow but if you are already working on it feel free to close it

@sbrannen
Copy link
Member

sbrannen commented Jun 19, 2023

@sbrannen sbrannen closed this Jun 19, 2023
@sbrannen sbrannen removed this from the 6.0.11 milestone Jun 19, 2023
@sbrannen sbrannen added status: superseded An issue that has been superseded by another and removed status: pending-design-work Needs design work before any code can be developed labels Jun 19, 2023
@essobedo essobedo deleted the private-init-destroy-aot branch June 19, 2023 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: superseded An issue that has been superseded by another theme: aot An issue related to Ahead-of-time processing type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants