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

Configuration property binding processes JavaBean methods in a non-deterministic order which may result in variable behavior #24068

Closed
emilnkrastev opened this issue Nov 6, 2020 · 2 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@emilnkrastev
Copy link

emilnkrastev commented Nov 6, 2020

Hello team,

I'm currently using Spring Boot 2.3.5.RELEASE and I'm trying to provide property via environment variable but the Spring boot app fails to start with the following exception:
`


APPLICATION FAILED TO START


Description:

Failed to bind properties under "vault" to com.example.relaxedbinding.Configuration:

Property: vault.token-runtime
Value: 242342
Origin: System Environment Property "VAULT_TOKEN_RUNTIME"
Reason: Failed to bind properties under 'vault' to com.example.relaxedbinding.Configuration

Action:

Update your application's configuration
`

The weird thing is that it does not fail on each run. The failure rate is 30%-ish.

Here is a link to my githup repo sample project:
https://github.com/emilnkrastev/relaxed-binding

Step to reproduce the issue:

  1. export VAULT_TOKEN_RUNTIME=242342
  2. mvn clean install
  3. java -jar target/relaxed-binding-0.0.1-SNAPSHOT.jar
  4. If it does not fail go to step 3 and try again.

Do you have an idea what is wrong with the relaxed binding?

Regards!

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 6, 2020
@wilkinsona
Copy link
Member

Thanks for the sample.

You currently have two ways of getting the value of your token-runtime property. The getTokenRuntime() method and getToken().getRuntime(). The former fails with a NullPointerException if setToken(Token) hasn't been called first. You can see this NPE by running your app with --debug:

Caused by: java.lang.IllegalStateException: Unable to get value for property token-runtime
	at org.springframework.boot.context.properties.bind.JavaBeanBinder$BeanProperty.lambda$getValue$0(JavaBeanBinder.java:336) ~[spring-boot-2.3.5.RELEASE.jar!/:2.3.5.RELEASE]
	at org.springframework.boot.context.properties.bind.JavaBeanBinder.bind(JavaBeanBinder.java:100) ~[spring-boot-2.3.5.RELEASE.jar!/:2.3.5.RELEASE]
	at org.springframework.boot.context.properties.bind.JavaBeanBinder.bind(JavaBeanBinder.java:80) ~[spring-boot-2.3.5.RELEASE.jar!/:2.3.5.RELEASE]
	at org.springframework.boot.context.properties.bind.JavaBeanBinder.bind(JavaBeanBinder.java:56) ~[spring-boot-2.3.5.RELEASE.jar!/:2.3.5.RELEASE]
	at org.springframework.boot.context.properties.bind.Binder.lambda$bindDataObject$5(Binder.java:451) ~[spring-boot-2.3.5.RELEASE.jar!/:2.3.5.RELEASE]
	at org.springframework.boot.context.properties.bind.Binder$Context.withIncreasedDepth(Binder.java:571) ~[spring-boot-2.3.5.RELEASE.jar!/:2.3.5.RELEASE]
	at org.springframework.boot.context.properties.bind.Binder$Context.withDataObject(Binder.java:557) ~[spring-boot-2.3.5.RELEASE.jar!/:2.3.5.RELEASE]
	at org.springframework.boot.context.properties.bind.Binder$Context.access$300(Binder.java:512) ~[spring-boot-2.3.5.RELEASE.jar!/:2.3.5.RELEASE]
	at org.springframework.boot.context.properties.bind.Binder.bindDataObject(Binder.java:449) ~[spring-boot-2.3.5.RELEASE.jar!/:2.3.5.RELEASE]
	at org.springframework.boot.context.properties.bind.Binder.bindObject(Binder.java:390) ~[spring-boot-2.3.5.RELEASE.jar!/:2.3.5.RELEASE]
	at org.springframework.boot.context.properties.bind.Binder.bind(Binder.java:319) ~[spring-boot-2.3.5.RELEASE.jar!/:2.3.5.RELEASE]
	... 46 common frames omitted
Caused by: java.lang.reflect.InvocationTargetException: null
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[na:1.8.0_252]
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[na:1.8.0_252]
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[na:1.8.0_252]
	at java.lang.reflect.Method.invoke(Method.java:498) ~[na:1.8.0_252]
	at org.springframework.boot.context.properties.bind.JavaBeanBinder$BeanProperty.lambda$getValue$0(JavaBeanBinder.java:333) ~[spring-boot-2.3.5.RELEASE.jar!/:2.3.5.RELEASE]
	... 56 common frames omitted
Caused by: java.lang.NullPointerException: null
	at com.example.relaxedbinding.Configuration$Token.access$000(Configuration.java:81) ~[classes!/:0.0.1-SNAPSHOT]
	at com.example.relaxedbinding.Configuration.getTokenRuntime(Configuration.java:38) ~[classes!/:0.0.1-SNAPSHOT]
	... 61 common frames omitted

Java reflection returns a class's methods in an unspecified and variable order. It's this variable order that causes the failure to be intermittent – it only happens when getTokenRuntime() is returned and therefore called before setToken(Token).

You can fix your problem by removing your getTokenRuntime() method.

We'll keep this issue open to look at making the ordering in which the methods are processed deterministic. We may be able to achieve this by sorting them by their names.

@emilnkrastev emilnkrastev changed the title Relaxed binding does not with with nested property class Relaxed binding does not work with nested property class Nov 6, 2020
@wilkinsona wilkinsona changed the title Relaxed binding does not work with nested property class Configuration property binding processes JavaBean methods in a non-deterministic order which may result in variable behaviour Nov 6, 2020
@wilkinsona wilkinsona added type: bug A general bug for: team-attention An issue we'd like other members of the team to review and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 6, 2020
@wilkinsona
Copy link
Member

Framework's configuration class parsing uses ASM to ensure consistent @Bean method ordering. I don't think we should use ASM, but there is some precedent for trying to make the order of the class's methods stable.

@philwebb philwebb removed the for: team-attention An issue we'd like other members of the team to review label Nov 18, 2020
@philwebb philwebb added this to the 2.2.x milestone Nov 18, 2020
@philwebb philwebb changed the title Configuration property binding processes JavaBean methods in a non-deterministic order which may result in variable behaviour Configuration property binding processes JavaBean methods in a non-deterministic order which may result in variable behavior Dec 10, 2020
@philwebb philwebb modified the milestones: 2.2.x, 2.2.12 Dec 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants