search

spring-attic / spring-native

Spring Native is now superseded by Spring Boot 3 official native support
https://docs.spring.io/spring-boot/docs/current/reference/html/native-image.html
Apache License 2.0
2.74k stars 356 forks source link

RefreshScope beans are created albeit the feature being disabled explicitly #1413

Closed hdeadman closed 2 years ago

hdeadman commented 2 years ago

I am trying to build a native image for CAS, and it is probably a long way from happening because CAS is a large project, but trying to make some progress. I am currently building and trying to run without any generated json b/c the generated json was an unmaintainable 40K+ lines and I was hitting this NPE issue during build so I couldn't make much progress finding issues in CAS that would prevent native builds. I have a public project building a native image with a github workflow and it builds the binary and then errors out when it runs due to:

2021-12-30 03:30:40.833  WARN 2236 --- [           main] o.a.cas.web.CasWebApplicationContext     : Exception encountered during context initialization - 
cancelling refresh attempt: org.springframework.beans.factory.BeanCreationException: Error creating bean with name
 'org.springframework.context.annotation.internalConfigurationAnnotationProcessor': Instantiation of bean failed; nested exception is org.springframework.beans.BeanInstantiationException: 
Failed to instantiate [org.springframework.context.annotation.ConfigurationClassPostProcessor]: 
No default constructor found; nested exception is java.lang.NoSuchMethodException: org.springframework.context.annotation.ConfigurationClassPostProcessor.<init>()

That seems like an internal spring class so it doesn't seem like something I should be putting in reflect-config.json, but maybe it is a class whose work is supposed to be handled by AOT processing? Could the fact that CAS has custom app context that extends AnnotationConfigServletWebServerApplicationContext be a problem? The custom application context is just overriding the onRefresh() method. Work has been done in CAS to not use cglib proxies for any configuration classes.

Here is the project: graal-starter branch https://github.com/hdeadman/cas-overlay-template.git (Not much code b/c it pulls in code from https://github.com/apereo/cas)

Here are the logs from build and the error when running native image. It uses mac-os b/c mac github runners have 14GB memory and ubuntu runners with 8GB weren't enough to build (gc overhead exceeded). This is building with the latest snapshot of spring-native and using the Liberica Native Image Kit. https://github.com/hdeadman/cas-overlay-template/runs/4663765980?check_suite_focus=true#step:6:132

snicoll commented 2 years ago

Could the fact that CAS has custom app context that extends AnnotationConfigServletWebServerApplicationContext be a problem?

Yes. An AOT-runtime context is not annotation-based at all. By forcing this at runtime (not sure how), you're bringing configuration class parsing in the native image again. I got a bit lost by the overlay and how things are wrapped up together. Can you share how the override of the context takes place at runtime?

hdeadman commented 2 years ago

I backed out the custom context override (in a fork of CAS) in order to see if that made the problem go away. You can see the change here: https://github.com/hdeadman/cas/commit/d7daf5a36fe33ba775d8e2443cc7a8c33d6eced8

I think the custom context that I removed uses this class to validate that there aren't properties that are invalid for the configuration beans. I suppose that would qualify as runtime configuration processing and that won't be doable at runtime?

With the custom app context removed, I updated the github action workflow to build that custom of CAS in the workflow and tried to build the native image with it. I got past the error above and it's getting a different error related to refresh scope, possibly on the messageSource bean?. 

2022-01-04 05:43:34.653 ERROR 51295 --- [           main] o.s.boot.SpringApplication               : Application run failed

java.lang.IllegalStateException: No Scope registered for scope name 'refresh'
    at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:368) ~[na:na]
    at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:213) ~[na:na]
    at org.springframework.context.support.AbstractApplicationContext.initMessageSource(AbstractApplicationContext.java:772) ~[na:na]
    at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:571) ~[na:na]
    at org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext.refresh(ServletWebServerApplicationContext.java:145) ~[na:na]
    at org.springframework.boot.SpringApplication.refresh(SpringApplication.java:730) ~[cas:2.6.2]
    at org.springframework.boot.SpringApplication.refreshContext(SpringApplication.java:412) ~[cas:2.6.2]
    at org.springframework.boot.SpringApplication.run(SpringApplication.java:302) ~[cas:2.6.2]
    at org.springframework.boot.builder.SpringApplicationBuilder.run(SpringApplicationBuilder.java:164) ~[na:na]
    at org.apereo.cas.web.CasWebApplication.main(CasWebApplication.java:64) ~[cas:6.5.0-HD-SNAPSHOT]

Full logs here: https://github.com/hdeadman/cas-overlay-template/runs/4698338180?check_suite_focus=true#step:8:129

CAS has configuration classes that are annotated with @RefreshScope(proxyMode = ScopedProxyMode.DEFAULT). Is that causing a problem? Or is it a problem specific to the messageSource bean that CAS overrides with this bean that has the RefreshScope annotation?

snicoll commented 2 years ago

Thanks for the follow-up.

I think the custom context that I removed uses this class to validate that there aren't properties that are invalid for the configuration beans

If that is the case, there isn't really any reason I can think of that requires a custom context for this. A library such as CAS requiring a custom context is highly unusual so stopping that irrespective of Spring Native is an improvement.

That call in onRefresh can be called in plenty of other locations. For instance, you could write an ApplicationContextInitializer that gives you back the context prior of it being refreshed.

I got past the error above and it's getting a different error related to refresh scope

Unfortunately, refresh scope isn't supported, see the reference documentation.

hdeadman commented 2 years ago

We don't really need refreshscope for a native build (it would just be a feature that wouldn't be available natively), but the documentation wording implies to me that spring native is disabling refreshscope automatically. I am going to try setting that property to false when I do nativeCompile, or can the code not use the refreshscope annotations at all?

snicoll commented 2 years ago

Yes, this is a good point. We do disable the feature but AOT doesn't take that into account, detect the scope and generate some code for it to be created at runtime. This is new in 0.11 as 0.10 was doing all that work at runtime (and the feature was disabled with the property we've discussed).

To fix this properly we might need a contributor that knows refresh scope is disabled and prevents the scope proxy to be created.

sdeleuze commented 2 years ago

I have created a reproducer, just build it with mvn -Pnative -DskipTests clean package, run target/demo-refresh-scope, you should get the java.lang.IllegalStateException: No Scope registered for scope name 'refresh' error.

demo-refresh-scope.zip

hdeadman commented 2 years ago

@snicoll Thanks for the fix. I tried it out and its using the new code but it got a ConcurrentModificationException.

https://github.com/hdeadman/cas-overlay-template/runs/4729204318?check_suite_focus=true#step:8:134

java.util.ConcurrentModificationException: null
    at java.util.HashMap.computeIfAbsent(HashMap.java:1135) ~[cas:na]
    at org.springframework.aot.beans.factory.config.NoOpScope.get(NoOpScope.java:41) ~[na:na]
    at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:371) ~[na:na]
    at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:213) ~[na:na]
    at org.springframework.context.support.AbstractApplicationContext.initMessageSource(AbstractApplicationContext.java:772) ~[na:na]
    at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:571) ~[na:na]
    at org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext.refresh(ServletWebServerApplicationContext.java:145) ~[na:na]
    at org.springframework.boot.SpringApplication.refresh(SpringApplication.java:730) ~[cas:2.6.2]
    at org.springframework.boot.SpringApplication.refreshContext(SpringApplication.java:412) ~[cas:2.6.2]
    at org.springframework.boot.SpringApplication.run(SpringApplication.java:302) ~[cas:2.6.2]
    at org.springframework.boot.builder.SpringApplicationBuilder.run(SpringApplicationBuilder.java:164) ~[na:na]
    at org.apereo.cas.web.CasWebApplication.main(CasWebApplication.java:63) ~[cas:6.5.0-HD-SNAPSHOT]

I see that line is in the middle of synchronized block on the map being used so that seems a bit odd...

snicoll commented 2 years ago

Thanks for testing! I think that might be the lambda callback. I'll have another look.

hdeadman commented 2 years ago

Thanks, the latest fix gets me past that RefreshScope issue. Now I am running into Caffeine library needing reflect-config.json but according to this issue https://github.com/spring-projects-experimental/spring-native/issues/465, not sure anyone has gotten that working. Looks like Caffeine 3.x might be blocked from working by an open Graal issue but CAS is using Caffeine 2.x so maybe there is some hope it can be made to work, I will try adding reflect-config.json for caffeine 2.x.

hdeadman commented 2 years ago

When try to build the app in NATIVE_AGENT mode and then run the app with java and the native-image-agent, to trace and generate metadata files, java -DspringAot=true -agentlib:native-image-agent=access-filter-file=access-filter.json,config-merge-dir=./src/main/resources/META-INF/native-image/org/apereo/cas/overlay,config-write-period-secs=300,config-write-initial-delay-secs=15 -jar build/libs/cas.jar

I am getting this error like it isn't using the new NoOpScope.

2022-01-08 00:52:15.666 ERROR 43787 --- [           main] o.s.boot.SpringApplication               : Application run failed

java.lang.IllegalStateException: No Scope registered for scope name 'refresh'
    at

https://github.com/hdeadman/cas-overlay-template/runs/4745129666?check_suite_focus=true#step:8:165

I don't understand everything that is going on with but when I looked at the BOOT-INF/lib/spring-native-0.11.2-SNAPSHOT.jar jar inside cas.jar, it contains the new NoOpScope.class but it doesn't appear to be using it in when running with java vs. running inside the native image.

Am I doing something wrong?