spring-projects / spring-boot

Spring Boot helps you to create Spring-powered, production-grade applications and services with absolute minimum fuss.
https://spring.io/projects/spring-boot
Apache License 2.0
75.23k stars 40.7k forks source link

Prevent initializers from being applied twice in AOT mode #32262

Open sdeleuze opened 2 years ago

sdeleuze commented 2 years ago

Running the following application prints 2 times Foo constructor at startup while a single time as expected in regular mode.

@SpringBootApplication
public class CommandlinerunnerApplication {

    public static void main(String[] args) {
        SpringApplication app = new SpringApplication(CommandlinerunnerApplication.class);
        app.addInitializers((ApplicationContextInitializer<GenericApplicationContext>) context ->
                context.registerBean(Foo.class));
        app.run(args);
    }
}

public class Foo {

    public Foo() {
        System.out.println("Foo constructor");
    }
}

A look at the generated code shows that Foo__BeanDefinitions generated during AOT transformations based on the initializer is created as expected, but it seems the second invocation comes from the fact that SpringApplication#applyInitializers is still invoked at runtime.

Notice that on native, this second invocation fails with Runtime reflection is not supported for public com.example.commandlinerunner.Foo() because unlike the AOT generated one, it requires ExecutableMode#INVOKE reflection hints (not inferred because not needed by the AOT generated code which is fine with ExecutableMode#INTROSPECT).

Both issues should be solved by removing the second invocation of the initializers, but this need to be explored because it could involve unwanted side effects with some initializers.

philwebb commented 2 years ago

I'm not sure we can skip running ApplicationContextInitializers in AOT mode. In this example, the initializer is registering a bean but it's entirely possible that an initializer does something else entirely. I can't think of any good way to decide which to run and which to skip.

Perhaps the initializer iteself should be updated to not attempt bean registration if running in AOT mode?

wilkinsona commented 2 years ago

Perhaps the initializer iteself should be updated to not attempt bean registration if running in AOT mode?

If this becomes a common pattern, perhaps we can provide a convenience mechanism to do this such as an annotation to add to the initializer class or an interface for it to implement.

snicoll commented 2 years ago

I'm not sure we can skip running ApplicationContextInitializers in AOT mode. In this example, the initializer is registering a bean but it's entirely possible that an initializer does something else entirely. I can't think of any good way to decide which to run and which to skip.

This is a problem that has a wider scope. Based on the experience of integrating features in Spring Native, I see the following:

While the first one looks harmless (just replaying what the AOT-generated code has done), it really isn't. If there is an instance supplier, AOT can't optimize it. If there isn't, then it won't work in a native image as the bean definition contributed by the (replayed) initializer will require reflection.

If we don't run the initializer, then the change to the environment or the presence of singletons is lost. To me, this looks like something that should be addressed in framework somehow. We need to guide users what they can or cannot do if they want to optimize their context Ahead-of-Time as well as the escape hatch if they chose to use something that we can't optimize.

snicoll commented 2 years ago

See #32422 for another example that is test-specific. We have ContextCustomizer implementations that are only impacting the BeanRegistry, or register a singleton that is only used as part of preparing the BeanRegistry. As such, running them again is harmless but require the necessary hints.

Rather than contributing hints for something that shouldn't run, I went down the route of using AotDetector from preventing those to run again. I would prefer this to be explicit/declarative rather.

philwebb commented 2 years ago

I've raised https://github.com/spring-projects/spring-framework/issues/29484 to see if we can have some framework support for this.