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

Make bean initialization deterministic for multiple @Autowired methods on same bean class #30359

Closed
arvyy opened this issue Apr 19, 2023 · 4 comments
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: enhancement A general enhancement
Milestone

Comments

@arvyy
Copy link
Contributor

arvyy commented Apr 19, 2023

Affects: spring 5.3 (specifically; spring boot 2.6.6)

Sorry, I realize this is a report for an old version, but maybe this is still relevant in newer versions because google wasn't of much help to me

After changing CI server to build on linux with docker, spring boot app started to undeterministically fail with NoSuchBeanDefinitionException. Ie, it works half the time (and it used to work all the time when the jar was being built on windows). I tracked it down and I think the cause was probably this:

@Service
public class Foo {
  @Autowired public void setBar(BarImpl bar) { ... }
}

@Configuration
public class Config {
  @Bean 
  public Bar makeBar() {
    return new BarImpl();
  }
}

ie., a bean is registered through interface type, and autowired through implementation type. I think this is the cause, because I stepped through wiring internals with debugger, and I found container rejecting makeBar bean due to barImplType.isAssignableFrom(barType) yielding false (where type is ResolvedType instance).

This is bad code, but regardless if it should or shouldn't work, it should behave more deterministically.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Apr 19, 2023
@snicoll
Copy link
Member

snicoll commented Apr 20, 2023

it should or shouldn't work, it should behave more deterministically.

The order in which the Service and the Configuration classes are processed may not be deterministic, which would be the reason for the failure. Unfortunately, this is impossible for us to know without more details. If you want support from us, please share a small sample we can run ourselves that replicate the problem you've described.

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Apr 20, 2023
@arvyy
Copy link
Contributor Author

arvyy commented Apr 20, 2023

Reproduced it on spring 6.0.8

https://github.com/arvyy/SpringIssue30359

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Apr 20, 2023
@snicoll
Copy link
Member

snicoll commented Apr 24, 2023

@arvyy thanks for the reproducer. I am assuming you must be on Windows or something as I can easily reproduce it with MacOS without the loop and docker, see https://github.com/snicoll-scratches/SpringIssue30359

In general, you should not expose an interface type if the bean is to be injected by a specific subtype. Regardless of the outcome of this issue, I highly recommend fixing that.

What's happening is that the bean initialization is not deterministic. If svc1 is not yet initialized when ServiceUser1 is initialized, the container only knows about a bean of type Service1 and there's no bean of type ServiceImpl1 yet. We'll investigate how we can make the initialization order more deterministic so that it isn't OS-specific.

@snicoll snicoll changed the title Flaky NoSuchBeanDefinitionException failure Make bean initialization discovered by classpath scanning more deterministic Apr 24, 2023
@snicoll snicoll added type: enhancement A general enhancement in: core Issues in core modules (aop, beans, core, context, expression) and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Apr 24, 2023
@snicoll snicoll added this to the 6.x Backlog milestone Apr 24, 2023
@jhoeller jhoeller modified the milestones: 6.x Backlog, 6.1.0-M4 Aug 3, 2023
@jhoeller
Copy link
Contributor

jhoeller commented Aug 3, 2023

The root concern are the two separate @Autowired methods on ServiceUser1: If setAnotherService gets resolved first, it works since AnotherService1 resolves a Service1 which implicitly makes the latter's actual bean class available. If setSvc gets resolved first, it fails since the ServiceImpl1 bean class has not been resolved yet.

To illustrate the case: a single setServices(AnotherService1 svc1, ServiceImpl1 svc2) method will always work, whereas a single setServices(ServiceImpl1 svc1, AnotherService1 svc2) method will always fail.

The root cause for this is the JVM using arbitrary method ordering on reflection, returning methods in a different order between runs. We address this with ASM-based method declaration sorting for multiple @Bean methods on the same configuration class already, maybe we should do the same for multiple @Autowired methods on a single class.

@jhoeller jhoeller changed the title Make bean initialization discovered by classpath scanning more deterministic Make bean initialization deterministic for multiple @Autowired methods on same bean class Aug 3, 2023
@jhoeller jhoeller modified the milestones: 6.1.0-M4, 6.0.12 Aug 3, 2023
@jhoeller jhoeller added the for: backport-to-5.3.x Marks an issue as a candidate for backport to 5.3.x label Aug 3, 2023
@github-actions github-actions bot added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-5.3.x Marks an issue as a candidate for backport to 5.3.x labels Aug 3, 2023
jhoeller added a commit that referenced this issue Aug 3, 2023
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: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants