Uploaded image for project: 'Spring Framework'
  1. Spring Framework
  2. SPR-7966

Improve failure mode when depends-on cycles exist

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Complete
    • Affects Version/s: 3.0.5
    • Fix Version/s: 4.0 RC2
    • Component/s: Core
    • Last commented by a User:
      false

      Description

      I had an application context containing a bogus cyclic dependency like this:

      <bean id="bean1" depends-on="bean2" .../>
      <bean id="bean2" depends-on="bean1" .../>

      Obviously this is misconfigured. But the error message you get from Spring is a StackOverflowError:

      	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.initializeBean(AbstractAutowireCapableBeanFactory.java:1420)
      	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:519)
      	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:456)
      	at org.springframework.beans.factory.support.AbstractBeanFactory$1.getObject(AbstractBeanFactory.java:291)
      	at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:222)
      	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:288)
      	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:190)
      	at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingletons(DefaultListableBeanFactory.java:580)
      	at org.springframework.context.support.AbstractApplicationContext.finishBeanFactoryInitialization(AbstractApplicationContext.java:895)
      	at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:425)
      	at org.springframework.context.support.ClassPathXmlApplicationContext.<init>(ClassPathXmlApplicationContext.java:197)
      	at org.springframework.context.support.ClassPathXmlApplicationContext.<init>(ClassPathXmlApplicationContext.java:172)
      	at org.springframework.context.support.ClassPathXmlApplicationContext.<init>(ClassPathXmlApplicationContext.java:158)
              ... application-specifc stack frames ...
      Caused by: java.lang.StackOverflowError
      	at java.util.HashMap.put(HashMap.java:389)
      	at java.util.HashSet.add(HashSet.java:217)
      	at java.util.Collections$SynchronizedCollection.add(Collections.java:1593)
      	at org.springframework.beans.factory.support.AbstractBeanFactory.markBeanAsCreated(AbstractBeanFactory.java:1363)
      	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:271)
      	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:190)
      	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:281)
      	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:190)
      	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:281)
      	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:190)
      	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:281)
      	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:190)
              ...

      This failure mode could be improved to better indicate the source of the problem.

      Doing so could be implemented easily e.g. by keeping track as the dependency graph is explored of the members in the current dependency path in a ThreadLocal<HashMap>.

        Issue Links

          Activity

          Hide
          letters4u Soungmin Joo added a comment -

          DefaultSingletonBeanRegistry.class seems already have member variable 'dependenciesForBeanMap'
          to keep 'depends-on' relations of beans. And AbstractBeanFactory seems verifiy 'depends-on' definition
          of a bean through following code.

          // Guarantee initialization of beans that the current bean depends on.			
          String[] dependsOn = mbd.getDependsOn();			
          if (dependsOn != null) {				
              for (String dependsOnBean : dependsOn) {					
                  getBean(dependsOnBean);					
                  registerDependentBean(dependsOnBean, beanName);				
              }			
          }

          StackOverflowError occurs from inside of getBean() method because above code invokes
          infinitely changeing value of 'dependsOnBean' variable to 'bean1' and 'bean2'.

          The next method 'registerDependentBean' saves dependency relation into ConcurrentHashMap.

          Because 'getBean' method is invoked prior to 'registerDependentBean' method,
          depends-on relations cannot be accessed inside of 'getBean' method.

          So I have tried to change above code into following code which creates BeanCreationException
          in case of circular 'depends-on'. And I'm preparing for pull-request according to this issue.

          Is this approach OK?

          String[] dependsOn = mbd.getDependsOn();			
          if (dependsOn != null) {				
              for (String dependsOnBean : dependsOn) {					
                  registerDependentBean(dependsOnBean, beanName);        
                  checkCircularDependsOn(dependsOnBean, beanName);
                  registerDependenciesForBean(dependsOnBean, beanName);
                  getBean(dependsOnBean);								
              }			
          }

          private void checkCircularDependsOn(String dependsOnBean, String beanName) {
              String[] dependenciesForBean = this.getDependenciesForBean(dependsOnBean);
              for(int i = 0; i < dependenciesForBean.length; i++) {
                  if(dependenciesForBean[i].equals(beanName)) {
                      throw new BeanCreationException(beanName,
                              "Beans with name '" + dependsOnBean + "' and '" + beanName + 
                              "' are dependent on each other; " + 
                              "consider removing 'depends-on' attribute from one bean definition.");
                  }
              }
          }

          Show
          letters4u Soungmin Joo added a comment - DefaultSingletonBeanRegistry.class seems already have member variable 'dependenciesForBeanMap' to keep 'depends-on' relations of beans. And AbstractBeanFactory seems verifiy 'depends-on' definition of a bean through following code. // Guarantee initialization of beans that the current bean depends on. String[] dependsOn = mbd.getDependsOn(); if (dependsOn != null ) { for (String dependsOnBean : dependsOn) { getBean(dependsOnBean); registerDependentBean(dependsOnBean, beanName); } } StackOverflowError occurs from inside of getBean() method because above code invokes infinitely changeing value of 'dependsOnBean' variable to 'bean1' and 'bean2'. The next method 'registerDependentBean' saves dependency relation into ConcurrentHashMap. Because 'getBean' method is invoked prior to 'registerDependentBean' method, depends-on relations cannot be accessed inside of 'getBean' method. So I have tried to change above code into following code which creates BeanCreationException in case of circular 'depends-on'. And I'm preparing for pull-request according to this issue. Is this approach OK? String[] dependsOn = mbd.getDependsOn(); if (dependsOn != null ) { for (String dependsOnBean : dependsOn) { registerDependentBean(dependsOnBean, beanName); checkCircularDependsOn(dependsOnBean, beanName); registerDependenciesForBean(dependsOnBean, beanName); getBean(dependsOnBean); } } private void checkCircularDependsOn(String dependsOnBean, String beanName) { String[] dependenciesForBean = this .getDependenciesForBean(dependsOnBean); for ( int i = 0 ; i < dependenciesForBean.length; i++) { if (dependenciesForBean[i].equals(beanName)) { throw new BeanCreationException(beanName, "Beans with name '" + dependsOnBean + "' and '" + beanName + "' are dependent on each other; " + "consider removing 'depends-on' attribute from one bean definition." ); } } }

            People

            • Assignee:
              juergen.hoeller Juergen Hoeller
              Reporter:
              archie172 Archie Cobbs
              Last updater:
              Juergen Hoeller
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Days since last comment:
                4 years, 37 weeks, 5 days ago

                Time Tracking

                Estimated:
                Original Estimate - 1h
                1h
                Remaining:
                Remaining Estimate - 1h
                1h
                Logged:
                Time Spent - Not Specified
                Not Specified