Better error reporting for circular dependencies between JavaConfig classes [SPR-12317]

[spring-projects-issues]

Eberhard Wolff opened SPR-12317 and commented

In the attached project we have

• Java Configuration class A. It uses a bean of type Z that is @Autowired. It provides a bean of type B.
• Java Configuration class AStrich provides the bean of type Z and has a bean of type B @Autowired.
  So the two Java Configuration classes are in a cyclic dependency due to the Beans of type B and Z.
  When bean B in A is created Z is not injected - which IMHO is wrong. It should be possible to rely on Spring autowiring all fields before a bean is created. Note that the problem disappears if B is not injected in AStrich any more and no cyclic dependency is present.
  This might look like a corner case but at my client it caused some headaches during an XML->JavaConfig migration.
  The attached project can be run with mvn test and fails due to the bug.

---

Affects: 4.1.1

Attachments:

• JavaConfigBug.zip (7.50 kB)

Issue Links:

• Plain FactoryBean declaration on @Bean method leads to early call (pre injection) [SPR-12141] #16755 Plain FactoryBean declaration on @Bean method leads to early call (pre injection)

2 votes, 6 watchers

[spring-projects-issues]

Juergen Hoeller commented

Eberhard, this fails with an internal IllegalStateException for me since the non-initialized configuration class hasn't had its BeanFactory set yet. This should be the case on Spring 4.x; have you possibly tried this on 3.2.x when it actually ran into the null field?

In any case, this case should be handled and reported in a clearer fashion.

Juergen

[spring-projects-issues]

Juergen Hoeller commented

As a general note: Circular dependencies between configuration classes can often be resolved through declaring one or more of the affected dependencies as lazily needed on demand, e.g. through ObjectFactory<B> or Provider<B>, or through declaring some of the @Bean methods as static (if they don't need to access any instance state of their containing configuration class). Both approaches works fine for the given case.

[spring-projects-issues]

Eberhard Wolff commented

Jürgen,

thanks for looking into this!
However, on my machine I get:
[wolff@UltraLight-3:~/tmp/JavaConfigBug]mvn test
....
org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'a': Injection of autowired dependencies failed; nested exception is org.springframework.beans.factory.BeanCreationException: Could not autowire field: com.ewolff.cycle.Z com.ewolff.cycle.A.z; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'AStrich': Injection of autowired dependencies failed; nested exception is org.springframework.beans.factory.BeanCreationException: Could not autowire field: com.ewolff.cycle.B com.ewolff.cycle.AStrich.b; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'b' defined in class com.ewolff.cycle.A: Instantiation of bean failed; nested exception is org.springframework.beans.factory.BeanDefinitionStoreException: Factory method [public com.ewolff.cycle.B com.ewolff.cycle.A.b()] threw exception; nested exception is java.lang.NullPointerException: z is null
at org.springframework.beans.factory.annotation.AutowiredAnnotationBeanPostProcessor.postProcessPropertyValues(AutowiredAnnotationBeanPostProcessor.java:334)
....
Caused by: java.lang.NullPointerException: z is null
at com.ewolff.cycle.A.b(A.java:16)

This is the code snippet:
@Autowired(required=true)
Z z;

@Bean
public B b() {
        if (z == null) {
                throw new NullPointerException("z is null");
        }

i.e. b() is called before z is injected which is IMHO wrong. Therefore the NullPointerException is thrown.

Thanks also for the hints how to resolve this. In this particular case the customer was not even aware of the circular dependency as the Bean was silently set to null. In a complex configuration it is hard to track down the circular dependency.

[spring-projects-issues]

Juergen Hoeller commented

Hmm, when running your test standalone, even with 4.1.2-BUILD-SNAPSHOT, I get the NPE as well. Strange - within the framework's unit tests, I consistently get the internal IllegalStateException. I'll double-check it.

In any case, this is effectively a consequence of the best-effort circular reference resolution algorithm: Due to it being the end of a circular reference, the target configuration instance isn't fully initialized yet when the factory method call comes in, just like a regular component instance may not be fully initialized yet when injected into another bean as part of Spring's traditional circular reference resolution.

There are quite a few scenarios where this may result in proper runtime state even with configuration classes involved, so it's hard to outright reject all such cases to begin with. What we can do, of course, is to report such a scenario in a clearer way, e.g. checking isSingletonCurrentlyInCreation and hinting at a circular reference scenario in the BeanCreationException message in such a case.

Juergen

[spring-projects-issues]

Eberhard Wolff commented

Hi Jürgen,
thanks for taking a second look! Indeed a clearer error would be appreciated - as you can see otherwise just a null is injected and the Dependency Injections fails silently which IMHO is not such a great thing.
Eberhard

[spring-projects-issues]

Juergen Hoeller commented

This turned into a rather comprehensive revision of our bean creation exception handling, with factory method invocation failures now leading to BeanInstantiationExceptions which start with pointing out a circular reference if there is any, even before the actual invocation failure message. I hope that's noticeable enough; it's unfortunately not that easy to make something noticeable within a larger stacktrace arrangement...

Juergen

[spring-projects-issues]

Eberhard Wolff commented

Thanks a lot! :)