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.

git-svn-id: https://src.springframework.org/svn/spring-framework/trunk@3615 50f2f4bb-b051-0410-bef5-90022cba6387

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.

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);
+	}
+
+}