From c8061393fbe3aa20bb51606c60d01a772d3df536 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Thu, 11 Oct 2012 20:45:19 -0700 Subject: [PATCH] Prevent memory leaks with @Configuration beans Refactor ConfigurationClassEnhancer to allow cglib caching of generated classes. Prior to this commit each enhanced @Configuration class would consume permgen space when created. The CallbackFilter and Callback Types are now defined as static final members so that they can be shared by all enhancers. Only the callbackInstances remain specific to a @Configuration class and these are not used by cglib as part of the cache key. Issue: SPR-9851 --- .../ConfigurationClassEnhancer.java | 105 +++++++++--------- 1 file changed, 51 insertions(+), 54 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassEnhancer.java b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassEnhancer.java index 85bda35602..4f569067b8 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassEnhancer.java +++ b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassEnhancer.java @@ -17,8 +17,6 @@ package org.springframework.context.annotation; import java.lang.reflect.Method; -import java.util.ArrayList; -import java.util.List; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -54,11 +52,28 @@ class ConfigurationClassEnhancer { private static final Log logger = LogFactory.getLog(ConfigurationClassEnhancer.class); - private final List callbackInstances = new ArrayList(); + private static final Class[] CALLBACK_TYPES = { BeanMethodInterceptor.class, + DisposableBeanMethodInterceptor.class, NoOp.class }; - private final List> callbackTypes = new ArrayList>(); + private static final CallbackFilter CALLBACK_FILTER = new CallbackFilter() { - private final CallbackFilter callbackFilter; + public int accept(Method candidateMethod) { + // Set up the callback filter to return the index of the BeanMethodInterceptor when + // handling a @Bean-annotated method; otherwise, return index of the NoOp callback. + if (BeanAnnotationHelper.isBeanAnnotated(candidateMethod)) { + return 0; + } + if (DisposableBeanMethodInterceptor.isDestroyMethod(candidateMethod)) { + return 1; + } + return 2; + } + }; + + private static final Callback DISPOSABLE_BEAN_METHOD_INTERCEPTOR = new DisposableBeanMethodInterceptor(); + + + private final Callback[] callbackInstances; /** @@ -66,28 +81,11 @@ class ConfigurationClassEnhancer { */ public ConfigurationClassEnhancer(ConfigurableBeanFactory beanFactory) { Assert.notNull(beanFactory, "BeanFactory must not be null"); - - this.callbackInstances.add(new BeanMethodInterceptor(beanFactory)); - this.callbackInstances.add(new DisposableBeanMethodInterceptor()); - this.callbackInstances.add(NoOp.INSTANCE); - - for (Callback callback : this.callbackInstances) { - this.callbackTypes.add(callback.getClass()); - } - - // Set up the callback filter to return the index of the BeanMethodInterceptor when - // handling a @Bean-annotated method; otherwise, return index of the NoOp callback. - callbackFilter = new CallbackFilter() { - public int accept(Method candidateMethod) { - if (BeanAnnotationHelper.isBeanAnnotated(candidateMethod)) { - return 0; - } - if (DisposableBeanMethodInterceptor.isDestroyMethod(candidateMethod)) { - return 1; - } - return 2; - } - }; + // Callback instances must be ordered in the same way as CALLBACK_TYPES and CALLBACK_FILTER + this.callbackInstances = new Callback[] { + new BeanMethodInterceptor(beanFactory), + DISPOSABLE_BEAN_METHOD_INTERCEPTOR, + NoOp.INSTANCE }; } /** @@ -135,15 +133,11 @@ class ConfigurationClassEnhancer { */ private Enhancer newEnhancer(Class superclass) { Enhancer enhancer = new Enhancer(); - // Because callbackFilter and callbackTypes are dynamically populated - // there's no opportunity for caching. This does not appear to be causing - // any performance problem. - enhancer.setUseCache(false); enhancer.setSuperclass(superclass); enhancer.setInterfaces(new Class[] {EnhancedConfiguration.class}); enhancer.setUseFactory(false); - enhancer.setCallbackFilter(this.callbackFilter); - enhancer.setCallbackTypes(this.callbackTypes.toArray(new Class[this.callbackTypes.size()])); + enhancer.setCallbackFilter(CALLBACK_FILTER); + enhancer.setCallbackTypes(CALLBACK_TYPES); return enhancer; } @@ -154,7 +148,7 @@ class ConfigurationClassEnhancer { private Class createClass(Enhancer enhancer) { Class subclass = enhancer.createClass(); // registering callbacks statically (as opposed to threadlocal) is critical for usage in an OSGi env (SPR-5932) - Enhancer.registerStaticCallbacks(subclass, this.callbackInstances.toArray(new Callback[this.callbackInstances.size()])); + Enhancer.registerStaticCallbacks(subclass, this.callbackInstances); return subclass; } @@ -166,7 +160,7 @@ class ConfigurationClassEnhancer { * @see BeanMethodInterceptor#enhanceFactoryBean(Class, String) */ private static class GetObjectMethodInterceptor implements MethodInterceptor { - + private final ConfigurableBeanFactory beanFactory; private final String beanName; @@ -178,7 +172,7 @@ class ConfigurationClassEnhancer { public Object intercept(Object obj, Method method, Object[] args, MethodProxy proxy) throws Throwable { return beanFactory.getBean(beanName); } - + } @@ -216,8 +210,19 @@ class ConfigurationClassEnhancer { */ private static class BeanMethodInterceptor implements MethodInterceptor { + private static final Class[] CALLBACK_TYPES = { + GetObjectMethodInterceptor.class, NoOp.class }; + + private static final CallbackFilter CALLBACK_FITLER = new CallbackFilter() { + public int accept(Method method) { + return method.getName().equals("getObject") ? 0 : 1; + } + }; + + private final ConfigurableBeanFactory beanFactory; + public BeanMethodInterceptor(ConfigurableBeanFactory beanFactory) { this.beanFactory = beanFactory; } @@ -246,7 +251,7 @@ class ConfigurationClassEnhancer { // to handle the case of an inter-bean method reference, we must explicitly check the // container for already cached instances - + // first, check to see if the requested bean is a FactoryBean. If so, create a subclass // proxy that intercepts calls to getObject() and returns any cached bean instance. // this ensures that the semantics of calling a FactoryBean from within @Bean methods @@ -328,26 +333,18 @@ class ConfigurationClassEnhancer { */ private Object enhanceFactoryBean(Class fbClass, String beanName) throws InstantiationException, IllegalAccessException { Enhancer enhancer = new Enhancer(); - enhancer.setUseCache(false); enhancer.setSuperclass(fbClass); enhancer.setUseFactory(false); - enhancer.setCallbackFilter(new CallbackFilter() { - public int accept(Method method) { - return method.getName().equals("getObject") ? 0 : 1; - } - }); - List callbackInstances = new ArrayList(); - callbackInstances.add(new GetObjectMethodInterceptor(this.beanFactory, beanName)); - callbackInstances.add(NoOp.INSTANCE); - - List> callbackTypes = new ArrayList>(); - for (Callback callback : callbackInstances) { - callbackTypes.add(callback.getClass()); - } - - enhancer.setCallbackTypes(callbackTypes.toArray(new Class[callbackTypes.size()])); + enhancer.setCallbackFilter(CALLBACK_FITLER); + // Callback instances must be ordered in the same way as CALLBACK_TYPES and CALLBACK_FILTER + Callback[] callbackInstances = new Callback[] { + new GetObjectMethodInterceptor(this.beanFactory, beanName), + NoOp.INSTANCE + }; + + enhancer.setCallbackTypes(CALLBACK_TYPES); Class fbSubclass = enhancer.createClass(); - Enhancer.registerCallbacks(fbSubclass, callbackInstances.toArray(new Callback[callbackInstances.size()])); + Enhancer.registerCallbacks(fbSubclass, callbackInstances); return fbSubclass.newInstance(); }