diff --git a/org.springframework.context/src/main/java/org/springframework/context/support/GenericApplicationContext.java b/org.springframework.context/src/main/java/org/springframework/context/support/GenericApplicationContext.java index f6db1726836..ce38ecef471 100644 --- a/org.springframework.context/src/main/java/org/springframework/context/support/GenericApplicationContext.java +++ b/org.springframework.context/src/main/java/org/springframework/context/support/GenericApplicationContext.java @@ -18,6 +18,7 @@ package org.springframework.context.support; import java.io.IOException; +import org.springframework.beans.BeansException; import org.springframework.beans.factory.BeanDefinitionStoreException; import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.beans.factory.annotation.QualifierAnnotationAutowireCandidateResolver; @@ -99,7 +100,6 @@ public class GenericApplicationContext extends AbstractApplicationContext implem */ public GenericApplicationContext() { this.beanFactory = new DefaultListableBeanFactory(); - this.beanFactory.setSerializationId(getId()); this.beanFactory.setParameterNameDiscoverer(new LocalVariableTableParameterNameDiscoverer()); this.beanFactory.setAutowireCandidateResolver(new QualifierAnnotationAutowireCandidateResolver()); } @@ -153,7 +153,6 @@ public class GenericApplicationContext extends AbstractApplicationContext implem @Override public void setId(String id) { super.setId(id); - this.beanFactory.setSerializationId(id); } /** @@ -243,9 +242,16 @@ public class GenericApplicationContext extends AbstractApplicationContext implem throw new IllegalStateException( "GenericApplicationContext does not support multiple refresh attempts: just call 'refresh' once"); } + this.beanFactory.setSerializationId(getId()); this.refreshed = true; } + @Override + protected void cancelRefresh(BeansException ex) { + this.beanFactory.setSerializationId(null); + super.cancelRefresh(ex); + } + /** * Not much to do: We hold a single internal BeanFactory that will never * get released. diff --git a/org.springframework.context/src/test/java/org/springframework/context/support/SerializableBeanFactoryMemoryLeakTests.java b/org.springframework.context/src/test/java/org/springframework/context/support/SerializableBeanFactoryMemoryLeakTests.java new file mode 100644 index 00000000000..e644e8426c6 --- /dev/null +++ b/org.springframework.context/src/test/java/org/springframework/context/support/SerializableBeanFactoryMemoryLeakTests.java @@ -0,0 +1,94 @@ +package org.springframework.context.support; + +import static org.hamcrest.CoreMatchers.equalTo; +import static org.junit.Assert.assertThat; +import static org.springframework.beans.factory.support.BeanDefinitionBuilder.rootBeanDefinition; + +import java.lang.reflect.Field; +import java.util.Map; + +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; +import org.springframework.beans.factory.BeanCreationException; +import org.springframework.beans.factory.support.BeanDefinitionRegistry; +import org.springframework.beans.factory.support.DefaultListableBeanFactory; +import org.springframework.context.ConfigurableApplicationContext; + +/** + * Unit tests cornering SPR-7502. + * + * @author Chris Beams + */ +public class SerializableBeanFactoryMemoryLeakTests { + + /** + * Defensively zero-out static factory count - other tests + * may have misbehaved before us. + */ + @BeforeClass + @AfterClass + public static void zeroOutFactoryCount() throws Exception { + getSerializableFactoryMap().clear(); + } + + @Test + public void genericContext() throws Exception { + assertFactoryCountThroughoutLifecycle(new GenericApplicationContext()); + } + + @Test + public void abstractRefreshableContext() throws Exception { + assertFactoryCountThroughoutLifecycle(new ClassPathXmlApplicationContext()); + } + + @Test + public void genericContextWithMisconfiguredBean() throws Exception { + GenericApplicationContext ctx = new GenericApplicationContext(); + registerMisconfiguredBeanDefinition(ctx); + assertFactoryCountThroughoutLifecycle(ctx); + } + + @Test + public void abstractRefreshableContextWithMisconfiguredBean() throws Exception { + ClassPathXmlApplicationContext ctx = new ClassPathXmlApplicationContext() { + @Override + protected void customizeBeanFactory(DefaultListableBeanFactory beanFactory) { + super.customizeBeanFactory(beanFactory); + registerMisconfiguredBeanDefinition(beanFactory); + } + }; + assertFactoryCountThroughoutLifecycle(ctx); + } + + private void assertFactoryCountThroughoutLifecycle(ConfigurableApplicationContext ctx) throws Exception { + assertThat(serializableFactoryCount(), equalTo(0)); + try { + ctx.refresh(); + assertThat(serializableFactoryCount(), equalTo(1)); + ctx.close(); + } catch (BeanCreationException ex) { + // ignore - this is expected on refresh() for failure case tests + } finally { + assertThat(serializableFactoryCount(), equalTo(0)); + } + } + + private void registerMisconfiguredBeanDefinition(BeanDefinitionRegistry registry) { + registry.registerBeanDefinition("misconfigured", + rootBeanDefinition(Object.class).addPropertyValue("nonexistent", "bogus") + .getBeanDefinition()); + } + + private int serializableFactoryCount() throws Exception { + Map map = getSerializableFactoryMap(); + return map.size(); + } + + private static Map getSerializableFactoryMap() throws Exception { + Field field = DefaultListableBeanFactory.class.getDeclaredField("serializableFactories"); + field.setAccessible(true); + return (Map) field.get(DefaultListableBeanFactory.class); + } + +}