Make @Configuration class enhancement idempotent

The registration of more than one ConfigurationClassPostProcessor
results in the double-enhancement of @Configuration classes, i.e. a
two-deep CGLIB subclass hierarchy is created.

As a side-effect of changes introduced in 3.1 M2 fixing SPR-8080, this
behavior now results in an infinite loop at CGLIB callback processing
time, leading to a StackOverflowException which is then suppressed by
the container, and ultimately results in the user being presented with
an unintuitive "Bean 'x' is not already in creation" exception.

This fix introduces a marker interface 'EnhancedConfiguration' to be
implemented by all generated @Configuration subclasses. The
configuration class enhancer can then behave in an idempotent fashion
by checking to see whether a candidate @Configuration class is already
assignable to this type i.e. already enhanced and ignore it if so.

Naturally, users should avoid registering more than one
ConfigurationClassPostProcessor, but this is not always possible. As
with the case in point, SPR-8824 originates from problems with
spring-data-neo4j, which explicitly registers its own
ConfigurationClassPostProcessor. The user has little control over this
arrangement, so it is important that the framework is defensive as
described above.

Issue: SPR-8824

org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassEnhancer.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2010 the original author or authors.
+ * Copyright 2002-2011 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.
@@ -89,13 +89,23 @@ class ConfigurationClassEnhancer {
 		};
 	}
 
-
 	/**
 	 * Loads the specified class and generates a CGLIB subclass of it equipped with
 	 * container-aware callbacks capable of respecting scoping and other bean semantics.
-	 * @return fully-qualified name of the enhanced subclass
+	 * @return the enhanced subclass
 	 */
 	public Class<?> enhance(Class<?> configClass) {
+		if (EnhancedConfiguration.class.isAssignableFrom(configClass)) {
+			if (logger.isDebugEnabled()) {
+				logger.debug(String.format("Ignoring request to enhance %s as it has " +
+						"already been enhanced. This usually indicates that more than one " +
+						"ConfigurationClassPostProcessor has been registered (e.g. via " +
+						"<context:annotation-config>). This is harmless, but you may " +
+						"want check your configuration and remove one CCPP if possible",
+						configClass.getName()));
+			}
+			return configClass;
+		}
 		Class<?> enhancedClass = createClass(newEnhancer(configClass));
 		if (logger.isDebugEnabled()) {
 			logger.debug(String.format("Successfully enhanced %s; enhanced class name is: %s",
@@ -104,6 +114,21 @@ class ConfigurationClassEnhancer {
 		return enhancedClass;
 	}
 
+	/**
+	 * Marker interface to be implemented by all @Configuration CGLIB subclasses.
+	 * Facilitates idempotent behavior for {@link ConfigurationClassEnhancer#enhance(Class)}
+	 * through checking to see if candidate classes are already assignable to it, e.g.
+	 * have already been enhanced.
+	 * <p>Also extends {@link DisposableBean}, as all enhanced
+	 * {@code @Configuration} classes must de-register static CGLIB callbacks on
+	 * destruction, which is handled by the (private) {@code DisposableBeanMethodInterceptor}.
+	 * <p>Note that this interface is intended for framework-internal use only, however
+	 * must remain public in order to allow access to subclasses generated from other 
+	 * packages (i.e. user code).
+	 */
+	public interface EnhancedConfiguration extends DisposableBean {
+	}
+
 	/**
 	 * Creates a new CGLIB {@link Enhancer} instance.
 	 */
@@ -114,7 +139,7 @@ class ConfigurationClassEnhancer {
 		// any performance problem.
 		enhancer.setUseCache(false);
 		enhancer.setSuperclass(superclass);
-		enhancer.setInterfaces(new Class[] {DisposableBean.class});
+		enhancer.setInterfaces(new Class[] {EnhancedConfiguration.class});
 		enhancer.setUseFactory(false);
 		enhancer.setCallbackFilter(this.callbackFilter);
 		enhancer.setCallbackTypes(this.callbackTypes.toArray(new Class[this.callbackTypes.size()]));
@@ -131,8 +156,8 @@ class ConfigurationClassEnhancer {
 		Enhancer.registerStaticCallbacks(subclass, this.callbackInstances.toArray(new Callback[this.callbackInstances.size()]));
 		return subclass;
 	}
-	
-	
+
+
 	/**
 	 * Intercepts calls to {@link FactoryBean#getObject()}, delegating to calling
 	 * {@link BeanFactory#getBean(String)} in order to respect caching / scoping.
@@ -160,6 +185,7 @@ class ConfigurationClassEnhancer {
 	 * Intercepts the invocation of any {@link DisposableBean#destroy()} on @Configuration
 	 * class instances for the purpose of de-registering CGLIB callbacks. This helps avoid
 	 * garbage collection issues See SPR-7901.
+	 * @see EnhancedConfiguration
 	 */
 	private static class DisposableBeanMethodInterceptor implements MethodInterceptor {
 

org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassPostProcessor.java

@@ -301,11 +301,13 @@ public class ConfigurationClassPostProcessor implements BeanDefinitionRegistryPo
 			try {
 				Class<?> configClass = beanDef.resolveBeanClass(this.beanClassLoader);
 				Class<?> enhancedClass = enhancer.enhance(configClass);
-				if (logger.isDebugEnabled()) {
-					logger.debug(String.format("Replacing bean definition '%s' existing class name '%s' " +
-							"with enhanced class name '%s'", entry.getKey(), configClass.getName(), enhancedClass.getName()));
+				if (configClass != enhancedClass) {
+					if (logger.isDebugEnabled()) {
+						logger.debug(String.format("Replacing bean definition '%s' existing class name '%s' " +
+								"with enhanced class name '%s'", entry.getKey(), configClass.getName(), enhancedClass.getName()));
+					}
+					beanDef.setBeanClass(enhancedClass);
 				}
-				beanDef.setBeanClass(enhancedClass);
 			}
 			catch (Throwable ex) {
 				throw new IllegalStateException("Cannot load configuration class: " + beanDef.getBeanClassName(), ex);

org.springframework.context/src/test/java/org/springframework/context/annotation/configuration/DuplicateConfigurationClassPostProcessorTests-context.xml

@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+
+<beans xmlns="http://www.springframework.org/schema/beans"
+	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+	xmlns:neo4j="http://www.springframework.org/schema/data/neo4j"
+	xmlns:context="http://www.springframework.org/schema/context"
+	xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-3.0.xsd
+		http://www.springframework.org/schema/data/neo4j http://www.springframework.org/schema/data/neo4j/spring-neo4j-2.0.xsd
+		http://www.springframework.org/schema/context http://www.springframework.org/schema/context/spring-context-3.1.xsd">
+
+	<bean id="a" class="org.springframework.context.annotation.ConfigurationClassPostProcessor"/>
+	<bean id="b" class="org.springframework.context.annotation.ConfigurationClassPostProcessor"/>
+	<bean id="c" class="org.springframework.context.annotation.DuplicateConfigurationClassPostProcessorTests$Config"/>
+</beans>
+

org.springframework.context/src/test/java/org/springframework/context/annotation/configuration/DuplicateConfigurationClassPostProcessorTests.java

@@ -0,0 +1,37 @@
+package org.springframework.context.annotation.configuration;
+
+import org.junit.Test;
+import org.springframework.beans.factory.support.RootBeanDefinition;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.context.annotation.ConfigurationClassPostProcessor;
+import org.springframework.context.support.GenericApplicationContext;
+
+/**
+ * Corners the bug originally reported by SPR-8824, where the presence of two
+ * {@link ConfigurationClassPostProcessor} beans in combination with a @Configuration
+ * class having at least one @Bean method causes a "Singleton 'foo' isn't currently in
+ * creation" exception.
+ *
+ * @author Chris Beams
+ * @since 3.1
+ */
+public class DuplicateConfigurationClassPostProcessorTests {
+
+	@Test
+	public void test() {
+		GenericApplicationContext ctx = new GenericApplicationContext();
+		ctx.registerBeanDefinition("a", new RootBeanDefinition(ConfigurationClassPostProcessor.class));
+		ctx.registerBeanDefinition("b", new RootBeanDefinition(ConfigurationClassPostProcessor.class));
+		ctx.registerBeanDefinition("myConfig", new RootBeanDefinition(Config.class));
+		ctx.refresh();
+	}
+
+	@Configuration
+	static class Config {
+		@Bean
+		public String string() {
+			return "bean";
+		}
+	}
+}