Spring may invoke @Bean methods too early in case of a circular reference [SPR-12018]

[spring-projects-issues]

Christopher Smith opened SPR-12018 and commented

If an @Configuration superclass provides a method annotated @Bean, the JavaConfig system can start invoking the superclass's @Bean methods before the actual object is fully constructed. If a subclass overrides one of these @Bean methods, and the superclass depends on retrieving a valid object from it, the superclass's initialization will fail.

The JavaConfig system should not treat @Configuration objects as live service objects until all dependencies have been injected and the @PostConstruct methods called.

---

Affects: 4.0.6

Issue Links:

• @Autowired List<ToAutoWire> into Configuration that defines @Bean ToAutoWire fails [SPR-12646] #17247 @Autowired List into Configuration that defines @Bean ToAutoWire fails
• Non-stable BeanCurrentlyInCreationException with wrong Java config [SPR-12004] #16620 Non-stable BeanCurrentlyInCreationException with wrong Java config
• Autowire bean in @Configuration class itself [SPR-12127] #16743 Autowire bean in @Configuration class itself
• Regression in @Condition exclusion [SPR-12233] #16848 Regression in @Condition exclusion

[spring-projects-issues]

Christopher Smith commented

I have posted a minimal test case on GitHub. Note that the container invokes the @PostConstruct method before autowiring the field.

[spring-projects-issues]

Christopher Smith commented

I will also note in passing that this is an issue specifically because @Configuration classes don't support constructor injection because of how they're handled by the proxying system. I don't know whether there's any more suitable means to inject dependencies in classes that are required to have no-arg constructors.

[spring-projects-issues]

Juergen Hoeller commented

I couldn't reproduce that the @PostConstruct method gets invoked before injection of the field... For me, it fails with an NPE in the in serviceBean() method, since provider() happens to return null (the uninitialized provider field. This is a consequence of the container's best-effort attempt to resolve the circular reference that is inherent to your scenario, where the configuration class tries to have a type injected that one of its own {{@Bean} methods is matching.

Note that the problem goes away through removing the @Bean marker on your provider() method. Since you're only using that method as a means to get serviceBean() working, that seems to be fine; the actual ServiceBeanProvider is available as an independent component bean anyway. Even with that in mind, we should at least try to provide a better exception message here if such an early @Bean call does happen...

Juergen

[spring-projects-issues]

Juergen Hoeller commented

As it seems, @PostConstruct can be called in a state where autowiring did occur but your @Autowired field happened to be injected with a null value (as returned from your provider() method at that early stage). This may lead to an impression that @PostConstruct got invoked too early when it really got invoked after a badly resolved circular reference that led to a null value being injected into that field.

Juergen

[spring-projects-issues]

Christopher Smith commented

This is a simplified version of my real-world case, in which I'm trying to modify AbstractMongoConfiguration, which assumes that it should be creating the MongoDbFactory, to accept an injected MongoDbFactory from Spring Cloud. In that case, the MongoDbFactory bean is theoretically available for autowiring to my StudioMongoConfiguration. In either, the container shouldn't be autowiring a null value into a required field. If the container is somehow detecting a circular reference, I would rather expect to get an exception indicating multiple matching beans (the one from Cloud and the incoming one from mongoDbFactory()).

I included the @PostConstruct because it demonstrates a logical error: The contract for @PostConstruct says that all injection should be finished, but it outputs null for the autowired field.

Ideally, I'd like to remove @Bean from AbstractMongoConfiguration#mongoDbFactory(), but (1) there's no way that I know of undo a method's "beanness" and (2) the other pieces that I really do want, like setting up the mapping context and template, call the method. I tried to work around this by overriding the method and supplying an injected field, but I ran into this problem. While this does look like a container bug to me, it also seems it might be a case for pulling the config hierarchy into a couple of separate classes.

[spring-projects-issues]

Juergen Hoeller commented

It's of course irritating behavior but as far as my current analysis goes, it's an unfortunate consequence of the container's best-effort circular reference resolution algorithm. A null value may be a valid bean instance in some cases (e.g. as a return value of a factory method), and therefore a valid injection result. @PostConstruct only guarantees that it's being called after the injection phase, not that non-null values have been injected. I know, it's debatable whether a null value injection makes any sense but that's exactly what's happening here - easy to verify once your @Autowired goes onto a method instead of onto a field.

As a help for detecting such cases, a non-null assertion in your factory method that is supposed to return the pre-injected field would help. But anyway, you got a point that the container should be smarter about detecting and handling such circles. I just have a suspicion that even injecting null references may be exactly what some people want in some cases...

Juergen

[spring-projects-issues]

Christopher Smith commented

The non-null assertion would make it fail sooner, but it wouldn't get the wiring actually working. The behavior I expected was either to attempt to resolve the bean externally if one was available or to throw on duplicate beans.

[spring-projects-issues]

Juergen Hoeller commented

It turns out that this self reference is the consequence of our autowire candidate filtering mechanism when trying to inject the provider field: The container ends up finding two ServiceBeanProvider matches, one the external bean and the other one the self reference to the provider() factory method. Unfortunately, since there are no clearer indications, due to the name match between the provider field and the provider() method, it ends up selecting the self reference...

That said, the general problem with the null value is that the provider() method is simply being called too early there during candidate evaluation, and a potential early null result will be cached as a singleton value from then on, which is the root of all the trouble here. Based on that analysis, the container should be smarter about automatically ignoring such self-references in its candidate choice...

Juergen

[spring-projects-issues]

Juergen Hoeller commented

Indeed, I was just referring to detecting the effect sooner - assertions do help there, and are generally recommended for factory methods which access instance state.

As for the problem itself, the correct solution is to exclude self-references not only to the very same bean (which we do for a long time already) but also to a factory method on the same bean. I've fixed this for 4.1 RC2 now; to be backported to 4.0.7 as well.

Juergen

[spring-projects-issues]

Christopher Smith commented

I think this is what I was trying to describe. Essentially, this is the reasoning I was expecting:

• I have an @Configuration class FooConfig here, which is a Spring bean.
• Before I call any of the @Bean methods on FooConfig, I should autowire it.
• I have this @Autowired field for ServiceBean.
• I see an existing ServiceBean instance in the context already, and I see an @Bean ServiceBean method.
• I should either
  • autowire that existing bean into FooConfig and then call the @Bean methods or
  • treat the candidate @Bean ServiceBean as a resolved bean (even if not yet available for injection) and throw a duplicate bean exception.

[spring-projects-issues]

Juergen Hoeller commented

After some back and forth, this feels like the right behavior to me, in particular since we've been excluding direct self references before... We're simply excluding references to factory methods on the original bean as well now.

This will be available in the upcoming 4.1.0 snapshot. Feel free to give it a try...

Juergen

[spring-projects-issues]

Christopher Smith commented

"Factory" is overloaded. In this context, does "factory method" equal @Bean?

[spring-projects-issues]

Juergen Hoeller commented

"Factory method" is any method pointed to by the "factoryMethodName" attribute on a BeanDefinition: typically derived from an @Bean method, but can also be specified in XML.

Juergen