From c8ff562d1f98de5603cf26b19bab00614c2aaa64 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Sun, 7 Dec 2014 16:30:31 +0100 Subject: [PATCH] DefaultSingletonBeanRegistry's isDependent defensively checks for circular recursion Issue: SPR-10787 (cherry picked from commit 15d3b88) --- .../support/DefaultSingletonBeanRegistry.java | 18 +++++++- .../DefaultSingletonBeanRegistryTests.java} | 45 +++++++++++++------ 2 files changed, 48 insertions(+), 15 deletions(-) rename spring-beans/src/test/java/org/springframework/beans/factory/{SharedBeanRegistryTests.java => support/DefaultSingletonBeanRegistryTests.java} (65%) diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultSingletonBeanRegistry.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultSingletonBeanRegistry.java index c5218e88d5d..987fb79742d 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultSingletonBeanRegistry.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultSingletonBeanRegistry.java @@ -18,6 +18,7 @@ package org.springframework.beans.factory.support; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.Iterator; import java.util.LinkedHashMap; import java.util.LinkedHashSet; @@ -415,9 +416,18 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements * dependent on the given bean or on any of its transitive dependencies. * @param beanName the name of the bean to check * @param dependentBeanName the name of the dependent bean + * @since 4.0 */ protected boolean isDependent(String beanName, String dependentBeanName) { - Set dependentBeans = this.dependentBeanMap.get(beanName); + return isDependent(beanName, dependentBeanName, null); + } + + private boolean isDependent(String beanName, String dependentBeanName, Set alreadySeen) { + String canonicalName = canonicalName(beanName); + if (alreadySeen != null && alreadySeen.contains(beanName)) { + return false; + } + Set dependentBeans = this.dependentBeanMap.get(canonicalName); if (dependentBeans == null) { return false; } @@ -425,7 +435,11 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements return true; } for (String transitiveDependency : dependentBeans) { - if (isDependent(transitiveDependency, dependentBeanName)) { + if (alreadySeen == null) { + alreadySeen = new HashSet(); + } + alreadySeen.add(beanName); + if (isDependent(transitiveDependency, dependentBeanName, alreadySeen)) { return true; } } diff --git a/spring-beans/src/test/java/org/springframework/beans/factory/SharedBeanRegistryTests.java b/spring-beans/src/test/java/org/springframework/beans/factory/support/DefaultSingletonBeanRegistryTests.java similarity index 65% rename from spring-beans/src/test/java/org/springframework/beans/factory/SharedBeanRegistryTests.java rename to spring-beans/src/test/java/org/springframework/beans/factory/support/DefaultSingletonBeanRegistryTests.java index 78015d18e8d..1b49c287abc 100644 --- a/spring-beans/src/test/java/org/springframework/beans/factory/SharedBeanRegistryTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/factory/support/DefaultSingletonBeanRegistryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2014 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -14,24 +14,23 @@ * limitations under the License. */ -package org.springframework.beans.factory; - -import static org.junit.Assert.*; - -import java.util.Arrays; +package org.springframework.beans.factory.support; import org.junit.Test; + import org.springframework.beans.BeansException; -import org.springframework.beans.factory.support.DefaultSingletonBeanRegistry; +import org.springframework.beans.factory.ObjectFactory; import org.springframework.tests.sample.beans.DerivedTestBean; import org.springframework.tests.sample.beans.TestBean; +import static org.junit.Assert.*; + /** * @author Juergen Hoeller * @author Chris Beams * @since 04.07.2006 */ -public final class SharedBeanRegistryTests { +public class DefaultSingletonBeanRegistryTests { @Test public void testSingletons() { @@ -52,9 +51,10 @@ public final class SharedBeanRegistryTests { assertSame(tb, beanRegistry.getSingleton("tb")); assertSame(tb2, beanRegistry.getSingleton("tb2")); assertEquals(2, beanRegistry.getSingletonCount()); - assertEquals(2, beanRegistry.getSingletonNames().length); - assertTrue(Arrays.asList(beanRegistry.getSingletonNames()).contains("tb")); - assertTrue(Arrays.asList(beanRegistry.getSingletonNames()).contains("tb2")); + String[] names = beanRegistry.getSingletonNames(); + assertEquals(2, names.length); + assertEquals("tb", names[0]); + assertEquals("tb2", names[1]); beanRegistry.destroySingletons(); assertEquals(0, beanRegistry.getSingletonCount()); @@ -72,8 +72,9 @@ public final class SharedBeanRegistryTests { assertSame(tb, beanRegistry.getSingleton("tb")); assertEquals(1, beanRegistry.getSingletonCount()); - assertEquals(1, beanRegistry.getSingletonNames().length); - assertTrue(Arrays.asList(beanRegistry.getSingletonNames()).contains("tb")); + String[] names = beanRegistry.getSingletonNames(); + assertEquals(1, names.length); + assertEquals("tb", names[0]); assertFalse(tb.wasDestroyed()); beanRegistry.destroySingletons(); @@ -82,4 +83,22 @@ public final class SharedBeanRegistryTests { assertTrue(tb.wasDestroyed()); } + @Test + public void testDependentRegistration() { + DefaultSingletonBeanRegistry beanRegistry = new DefaultSingletonBeanRegistry(); + + beanRegistry.registerDependentBean("a", "b"); + beanRegistry.registerDependentBean("b", "c"); + beanRegistry.registerDependentBean("c", "b"); + assertTrue(beanRegistry.isDependent("a", "b")); + assertTrue(beanRegistry.isDependent("b", "c")); + assertTrue(beanRegistry.isDependent("c", "b")); + assertTrue(beanRegistry.isDependent("a", "c")); + assertFalse(beanRegistry.isDependent("c", "a")); + assertFalse(beanRegistry.isDependent("b", "a")); + assertFalse(beanRegistry.isDependent("a", "a")); + assertTrue(beanRegistry.isDependent("b", "b")); + assertTrue(beanRegistry.isDependent("c", "c")); + } + }