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

ApplicationContext Circular Dependency Problem [SPR-1487] #6186

Closed
spring-projects-issues opened this issue Nov 21, 2005 · 4 comments
Closed
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

Simon Raess opened SPR-1487 and commented

Spring ApplicationContext throws an exception, claiming a circular dependency, in a very simple scenario:

  • two classes, One and Two
  • One has a single constructor argument of type Two
  • Two has a single property of type One

The following context XML works:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE beans PUBLIC "-//SPRING//DTD BEAN//EN" "http://www.springframework.org/dtd/spring-beans.dtd">
<beans>
<bean id="two" class="Two">
<property name="one"><ref local="one"/></property>
</bean>
<bean id="one" class="One">
<constructor-arg><ref local="two"/></constructor-arg>
</bean>
</beans>

Just reversing the bean definitions results in a total different situation:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE beans PUBLIC "-//SPRING//DTD BEAN//EN" "http://www.springframework.org/dtd/spring-beans.dtd">
<beans>
<bean id="one" class="One">
<constructor-arg><ref local="two"/></constructor-arg>
</bean>
<bean id="two" class="Two">
<property name="one"><ref local="one"/></property>
</bean>
</beans>

The following exception is thrown:

Nov 21, 2005 1:37:15 PM org.springframework.beans.factory.support.DefaultListableBeanFactory preInstantiateSingletons
INFO: Pre-instantiating singletons in factory [org.springframework.beans.factory.support.DefaultListableBeanFactory defining beans [one,two]; root of BeanFactory hierarchy]
Nov 21, 2005 1:37:15 PM org.springframework.beans.factory.support.AbstractBeanFactory destroySingletons
INFO: Destroying singletons in factory {org.springframework.beans.factory.support.DefaultListableBeanFactory defining beans [one,two]; root of BeanFactory hierarchy}
Exception in thread "main" org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'one' defined in class path resource [context.xml]: Can't resolve reference to bean 'two' while setting property 'constructor argument'; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'two' defined in class path resource [context.xml]: Can't resolve reference to bean 'one' while setting property 'one'; nested exception is org.springframework.beans.factory.BeanCurrentlyInCreationException: Error creating bean with name 'one': Requested bean is currently in creation (circular reference when autowiring constructor?)
org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'two' defined in class path resource [context.xml]: Can't resolve reference to bean 'one' while setting property 'one'; nested exception is org.springframework.beans.factory.BeanCurrentlyInCreationException: Error creating bean with name 'one': Requested bean is currently in creation (circular reference when autowiring constructor?)
org.springframework.beans.factory.BeanCurrentlyInCreationException: Error creating bean with name 'one': Requested bean is currently in creation (circular reference when autowiring constructor?)
at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:186)
at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:147)
at org.springframework.beans.factory.support.BeanDefinitionValueResolver.resolveReference(BeanDefinitionValueResolver.java:176)
at org.springframework.beans.factory.support.BeanDefinitionValueResolver.resolveValueIfNecessary(BeanDefinitionValueResolver.java:105)
at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.applyPropertyValues(AbstractAutowireCapableBeanFactory.java:1012)
at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.populateBean(AbstractAutowireCapableBeanFactory.java:823)
at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:345)
at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:226)
at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:147)
at org.springframework.beans.factory.support.BeanDefinitionValueResolver.resolveReference(BeanDefinitionValueResolver.java:176)
at org.springframework.beans.factory.support.BeanDefinitionValueResolver.resolveValueIfNecessary(BeanDefinitionValueResolver.java:105)
at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.resolveConstructorArguments(AbstractAutowireCapableBeanFactory.java:713)
at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.autowireConstructor(AbstractAutowireCapableBeanFactory.java:611)
at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:329)
at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:226)
at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:147)
at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingletons(DefaultListableBeanFactory.java:275)
at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:318)
at org.springframework.context.support.ClassPathXmlApplicationContext.<init>(ClassPathXmlApplicationContext.java:81)
at org.springframework.context.support.ClassPathXmlApplicationContext.<init>(ClassPathXmlApplicationContext.java:66)
at org.springframework.context.support.ClassPathXmlApplicationContext.<init>(ClassPathXmlApplicationContext.java:57)
at Test.main(Test.java:7)

So first of all, the order of bean definitions is in fact significant. Second, Spring fails to construct the object graph in this simple case. Note, I was using the following simple test program:

public class Test {
public static void main(String[] args) {
BeanFactory factory = new ClassPathXmlApplicationContext("context.xml");
Two two = (Two) factory.getBean("two");
System.out.println(two);
System.out.println(two.getOne());
}
}

Using an XmlBeanFactory works, independent of the order of those two bean elements in the XML.

public class Test {
public static void main(String[] args) {
XmlBeanFactory factory = new XmlBeanFactory(new ClassPathResource("context.xml"));
Two two = (Two) factory.getBean("two");
System.out.println(two);
System.out.println(two.getOne());
}
}


Affects: 1.2.5, 1.2.6

0 votes, 5 watchers

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

The difference between the BeanFactory and ApplicationContext behavior here simply comes from the fact that a BeanFactory does by default not pre-initialize its singletons. So the first bean to initialize there is "two", because that's what you call "getBean" for: It doesn't matter whether "two" is the first bean definition in the file; it's always gonna be initialized first in that case.

The order is only significant for the startup of bean instances in an ApplicationContext, which pre-initializes all its singletons, in the order of bean definitions.

You do have a point there in that the circular reference resolution mechanism depends on the initialization order. We do not guarantee circular reference resolution for such a scenario in the first place, though: The general rule is to not use circular references at all, in particular not in combination with constructor injection.

Essentially, the current algorithm stats with creating the first bean instance and then tries to resolve its references from there on. If the first bean needs a constructor reference to some other bean, that other bean will be created. Now if that other beans wants a reference back to the first bean, no such reference can be retrieved - not even an incompletely initialized one -, since the first bean hasn't been constructed yet.

It's essentially a question of lifecycle: A passed-in reference usually has to be fully initialized. That can't be guaranteed for a circular reference, so the current algorithm just gives that guarantee for the first bean (the first one to be initialized) then, letting the second bean have an incompletely initialized reference back to the first bean - which only works if the first bean has already been constructed at that point.

The only way around the current behavior is to initialize the non-constructor bean first. However, the ApplicationContext doesn't know this upfront, as it only discovers actual circular references once it starts to initialize a bean... Unfortunately, I don't see a clean way to adapt the algorithm in that respect.

Juergen

@spring-projects-issues
Copy link
Collaborator Author

Lee Freyberg commented

The underlying problem will not go away if both beans use setter injection; you just end up in a situation where the dependencies may be satisfied at a reference level, but not at an initialization level.

Consider the following dependency graph where all beans are singletons and are aggressively loaded (i.e. in an ApplicationContext):

A --> B --> C --> D ...... --> Z
          ^_______________|

While the current algorithm will cope with this, it will not guarantee that all of the beans are fully initialized before they are set as dependencies; More specifically B may only be partially initialized when it is set on Z; Meaning that if Z depends on any data in B set by spring, it may fail in a inconsistent state, silently.

The obvious solution is not to use circular dependencies in bean properties, but in reality, with large projects this is not very practical - it may not be known that such a dependency exists if the graph is deep enough.

An init-method won't solve the problem as it will be called after all of the properties on Z have been set, but before B has finished. Z could also initialize itself in a lazy manner, but this is also not always applicable.

Another solution is to use a BeanFactoryPostProcessor to call an init method in Z after all the beans in the factory have been created. The problem with this is that you may not know that you have a circular dependency, your application may start but may also be in a non-deterministic state.

At the very least, the BeanFactory should be modified to log a warning when it finds a circular dependency to indicate that a bean is going to be set with a dependency that is not fully initialized. This should be pretty simple as the bean factory knows that it has found a circular dependency and is going ahead with construction / initialization regardless, and it would allow users to take corrective action:

...from AbstractAutowireCapableBeanFactory line 339:

// Eagerly cache singletons to be able to resolve circular references
// even when triggered by lifecycle interfaces like BeanFactoryAware.
if (isSingletonCurrentlyInCreation(beanName)) {
    addSingleton(beanName, bean);
}

Furthermore, the spring documentation should be updated to reflect that this situation is possible - at the moment I cannot find any references to it, which means a code trawl to find the problem, which is significantly hampered by the fact that it happens silently. All you get is corruption in you setter or init method but your application may still start.

Lee

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

I've added an debug-level log message that indicates every time a non-fully-initialized singleton bean is returned from getBean. This in particular applies to other beans that point back to the currently initialized bean. So if there's any failure caused by a non-fully initialized reference, it should be immediately obvious from the log messages that came right before the exception.

Unfortunately, we arguably can't raise the log level beyond debug, since circular references might be entirely deliberate - and we don't want so spam anybody's info/debug log with messages that are not relevant for the scenario. Hence, I'm afraid it has to remain at debug level.

I've also added an "allowCircularReferences" flag to AbstractAutowireCapableBeanFactory, similar to DefaultListableBeanFactory's existing "allowBeanDefinitionOverriding" flag. It is on by default but can be turned off to suppress the eager exposure of singleton references - as a consequence, causing every circular reference to fail immediately. So if want to enforce avoiding them, consider turning this flag off.

Note that those flags are currently only exposed at the BeanFactory level. To use them with an ApplicationContext, you'd have to use a GenericApplicationContext or subclass your ApplicationContext's "createBeanFactory" method.

Juergen

@spring-projects-issues
Copy link
Collaborator Author

Zhihong "John" Wang commented

To solve the problem, we can somehow indicate in the configuration file that bean two's reference to bean one does not require bean one to be fully intialized. e.g.

<bean id="two" class="Two">
<property name="one"><ref local="one" require-fully-initialized="false" /></property>
</bean>
By do this, ApplicationContext knows that bean two's reference to one is not necessary for it's intialization. Thus the ApplicationContext can safely set the reference to one or delay setting of this reference.

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) type: bug A general bug
Projects
None yet
Development

No branches or pull requests

2 participants