From b72cca54030d380ab70af0eed60d82074e2e3522 Mon Sep 17 00:00:00 2001 From: Chris Beams Date: Fri, 27 Aug 2010 10:53:20 +0000 Subject: [PATCH] Fix memory leak in serializable bean factory management (SPR-7502) GenericApplicationContext and AbstractRefreshableApplicationContext implementations now call DefaultListableBeanFactory.setSerializationId() only upon successful refresh() instead of on instantiation of the context, as was previously the case with GAC. DLBF.setSerializationId() adds the beanFactory to the *static* DLBF.serializableFactories map, and while calling close() on the application context removes entries from that map, it does so only if the context is currently active (i.e. refresh() has been called). Also, cancelRefresh() has been overridden in GAC just as it has been in ARAC to accomodate the possibility of a BeansException being thrown. In this case, the beanFactory serializationId will be nulled out and the beanFactory removed from the serializableFactories map. The SerializableBeanFactoryMemoryLeakTests test case provides full coverage of these scenarios. --- .../support/GenericApplicationContext.java | 10 +- ...erializableBeanFactoryMemoryLeakTests.java | 94 +++++++++++++++++++ 2 files changed, 102 insertions(+), 2 deletions(-) create mode 100644 org.springframework.context/src/test/java/org/springframework/context/support/SerializableBeanFactoryMemoryLeakTests.java 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); + } + +}