Skip to content

Commit 1ae3eba

Browse files
sbrannensbrannen
committed
Merge from sbrannen/SPR-10785
* SPR-10785: Fix CGLIB memory leak for method injection
2 parents bda8f2b + 8028eae commit 1ae3eba

File tree

2 files changed

+196
-197
lines changed

2 files changed

+196
-197
lines changed

‎spring-beans/src/main/java/org/springframework/beans/factory/support/CglibSubclassingInstantiationStrategy.java

Lines changed: 147 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -22,22 +22,27 @@
2222
import org.apache.commons.logging.Log;
2323
import org.apache.commons.logging.LogFactory;
2424

25+
import org.springframework.beans.BeanInstantiationException;
26+
import org.springframework.beans.BeanUtils;
2527
import org.springframework.beans.factory.BeanFactory;
2628
import org.springframework.cglib.core.SpringNamingPolicy;
2729
import org.springframework.cglib.proxy.Callback;
2830
import org.springframework.cglib.proxy.CallbackFilter;
2931
import org.springframework.cglib.proxy.Enhancer;
32+
import org.springframework.cglib.proxy.Factory;
3033
import org.springframework.cglib.proxy.MethodInterceptor;
3134
import org.springframework.cglib.proxy.MethodProxy;
3235
import org.springframework.cglib.proxy.NoOp;
3336

3437
/**
3538
* Default object instantiation strategy for use in BeanFactories.
36-
* Uses CGLIB to generate subclasses dynamically if methods need to be
37-
* overridden by the container, to implement Method Injection.
39+
*
40+
* <p>Uses CGLIB to generate subclasses dynamically if methods need to be
41+
* overridden by the container to implement <em>Method Injection</em>.
3842
*
3943
* @author Rod Johnson
4044
* @author Juergen Hoeller
45+
* @author Sam Brannen
4146
* @since 1.1
4247
*/
4348
public class CglibSubclassingInstantiationStrategy extends SimpleInstantiationStrategy {
@@ -50,7 +55,7 @@ public class CglibSubclassingInstantiationStrategy extends SimpleInstantiationSt
5055

5156
/**
5257
* Index in the CGLIB callback array for a method that should
53-
* be overridden to provide method lookup.
58+
* be overridden to provide <em>method lookup</em>.
5459
*/
5560
private static final int LOOKUP_OVERRIDE = 1;
5661

@@ -62,18 +67,17 @@ public class CglibSubclassingInstantiationStrategy extends SimpleInstantiationSt
6267

6368

6469
@Override
65-
protected Object instantiateWithMethodInjection(
66-
RootBeanDefinition beanDefinition, String beanName, BeanFactory owner) {
70+
protected Object instantiateWithMethodInjection(RootBeanDefinition beanDefinition, String beanName,
71+
BeanFactory owner) {
6772

68-
// Must generate CGLIB subclass.
69-
return new CglibSubclassCreator(beanDefinition, owner).instantiate(null, null);
73+
return instantiateWithMethodInjection(beanDefinition, beanName, owner, null, null);
7074
}
7175

7276
@Override
73-
protected Object instantiateWithMethodInjection(
74-
RootBeanDefinition beanDefinition, String beanName, BeanFactory owner,
75-
Constructor<?> ctor, Object[] args) {
77+
protected Object instantiateWithMethodInjection(RootBeanDefinition beanDefinition, String beanName,
78+
BeanFactory owner, Constructor<?> ctor, Object[] args) {
7679

80+
// Must generate CGLIB subclass.
7781
return new CglibSubclassCreator(beanDefinition, owner).instantiate(ctor, args);
7882
}
7983

@@ -84,13 +88,15 @@ protected Object instantiateWithMethodInjection(
8488
*/
8589
private static class CglibSubclassCreator {
8690

87-
private static final Log logger = LogFactory.getLog(CglibSubclassCreator.class);
91+
private static final Class<?>[] CALLBACK_TYPES = new Class<?>[] { NoOp.class,
92+
LookupOverrideMethodInterceptor.class, ReplaceOverrideMethodInterceptor.class };
8893

8994
private final RootBeanDefinition beanDefinition;
9095

9196
private final BeanFactory owner;
9297

93-
public CglibSubclassCreator(RootBeanDefinition beanDefinition, BeanFactory owner) {
98+
99+
CglibSubclassCreator(RootBeanDefinition beanDefinition, BeanFactory owner) {
94100
this.beanDefinition = beanDefinition;
95101
this.owner = owner;
96102
}
@@ -101,105 +107,158 @@ public CglibSubclassCreator(RootBeanDefinition beanDefinition, BeanFactory owner
101107
* @param ctor constructor to use. If this is {@code null}, use the
102108
* no-arg constructor (no parameterization, or Setter Injection)
103109
* @param args arguments to use for the constructor.
104-
* Ignored if the ctor parameter is {@code null}.
110+
* Ignored if the {@code ctor} parameter is {@code null}.
105111
* @return new instance of the dynamically generated subclass
106112
*/
107-
public Object instantiate(Constructor<?> ctor, Object[] args) {
113+
Object instantiate(Constructor<?> ctor, Object[] args) {
114+
Class<?> subclass = createEnhancedSubclass(this.beanDefinition);
115+
116+
Object instance;
117+
if (ctor == null) {
118+
instance = BeanUtils.instantiate(subclass);
119+
}
120+
else {
121+
try {
122+
Constructor<?> enhancedSubclassConstructor = subclass.getConstructor(ctor.getParameterTypes());
123+
instance = enhancedSubclassConstructor.newInstance(args);
124+
}
125+
catch (Exception e) {
126+
throw new BeanInstantiationException(this.beanDefinition.getBeanClass(), String.format(
127+
"Failed to invoke construcor for CGLIB enhanced subclass [%s]", subclass.getName()), e);
128+
}
129+
}
130+
131+
// SPR-10785: set callbacks directly on the instance instead of in the
132+
// enhanced class (via the Enhancer) in order to avoid memory leaks.
133+
Factory factory = (Factory) instance;
134+
factory.setCallbacks(new Callback[] { NoOp.INSTANCE,//
135+
new LookupOverrideMethodInterceptor(beanDefinition, owner),//
136+
new ReplaceOverrideMethodInterceptor(beanDefinition, owner) });
137+
138+
return instance;
139+
}
140+
141+
/**
142+
* Create an enhanced subclass of the bean class for the provided bean
143+
* definition, using CGLIB.
144+
*/
145+
private Class<?> createEnhancedSubclass(RootBeanDefinition beanDefinition) {
108146
Enhancer enhancer = new Enhancer();
109-
enhancer.setSuperclass(this.beanDefinition.getBeanClass());
147+
enhancer.setSuperclass(beanDefinition.getBeanClass());
110148
enhancer.setNamingPolicy(SpringNamingPolicy.INSTANCE);
111-
enhancer.setCallbackFilter(new CallbackFilterImpl());
112-
enhancer.setCallbacks(new Callback[] {
113-
NoOp.INSTANCE,
114-
new LookupOverrideMethodInterceptor(),
115-
new ReplaceOverrideMethodInterceptor()
116-
});
117-
118-
return (ctor != null ? enhancer.create(ctor.getParameterTypes(), args) : enhancer.create());
149+
enhancer.setCallbackFilter(new CallbackFilterImpl(beanDefinition));
150+
enhancer.setCallbackTypes(CALLBACK_TYPES);
151+
return enhancer.createClass();
119152
}
153+
}
154+
155+
/**
156+
* Class providing hashCode and equals methods required by CGLIB to
157+
* ensure that CGLIB doesn't generate a distinct class per bean.
158+
* Identity is based on class and bean definition.
159+
*/
160+
private static class CglibIdentitySupport {
120161

162+
private final RootBeanDefinition beanDefinition;
163+
164+
165+
CglibIdentitySupport(RootBeanDefinition beanDefinition) {
166+
this.beanDefinition = beanDefinition;
167+
}
121168

122169
/**
123-
* Class providing hashCode and equals methods required by CGLIB to
124-
* ensure that CGLIB doesn't generate a distinct class per bean.
125-
* Identity is based on class and bean definition.
170+
* Exposed for equals method to allow access to enclosing class field
126171
*/
127-
private class CglibIdentitySupport {
128-
129-
/**
130-
* Exposed for equals method to allow access to enclosing class field
131-
*/
132-
protected RootBeanDefinition getBeanDefinition() {
133-
return beanDefinition;
134-
}
172+
protected RootBeanDefinition getBeanDefinition() {
173+
return beanDefinition;
174+
}
135175

136-
@Override
137-
public boolean equals(Object other) {
138-
return (other.getClass().equals(getClass()) &&
139-
((CglibIdentitySupport) other).getBeanDefinition().equals(beanDefinition));
140-
}
176+
@Override
177+
public boolean equals(Object other) {
178+
return (other.getClass().equals(getClass()) && ((CglibIdentitySupport) other).getBeanDefinition().equals(
179+
beanDefinition));
180+
}
141181

142-
@Override
143-
public int hashCode() {
144-
return beanDefinition.hashCode();
145-
}
182+
@Override
183+
public int hashCode() {
184+
return beanDefinition.hashCode();
146185
}
186+
}
147187

188+
/**
189+
* CGLIB object to filter method interception behavior.
190+
*/
191+
private static class CallbackFilterImpl extends CglibIdentitySupport implements CallbackFilter {
192+
193+
private static final Log logger = LogFactory.getLog(CallbackFilterImpl.class);
148194

149-
/**
150-
* CGLIB MethodInterceptor to override methods, replacing them with an
151-
* implementation that returns a bean looked up in the container.
152-
*/
153-
private class LookupOverrideMethodInterceptor extends CglibIdentitySupport implements MethodInterceptor {
154195

155-
@Override
156-
public Object intercept(Object obj, Method method, Object[] args, MethodProxy mp) throws Throwable {
157-
// Cast is safe, as CallbackFilter filters are used selectively.
158-
LookupOverride lo = (LookupOverride) beanDefinition.getMethodOverrides().getOverride(method);
159-
return owner.getBean(lo.getBeanName());
196+
CallbackFilterImpl(RootBeanDefinition beanDefinition) {
197+
super(beanDefinition);
198+
}
199+
200+
@Override
201+
public int accept(Method method) {
202+
MethodOverride methodOverride = getBeanDefinition().getMethodOverrides().getOverride(method);
203+
if (logger.isTraceEnabled()) {
204+
logger.trace("Override for '" + method.getName() + "' is [" + methodOverride + "]");
205+
}
206+
if (methodOverride == null) {
207+
return PASSTHROUGH;
160208
}
209+
else if (methodOverride instanceof LookupOverride) {
210+
return LOOKUP_OVERRIDE;
211+
}
212+
else if (methodOverride instanceof ReplaceOverride) {
213+
return METHOD_REPLACER;
214+
}
215+
throw new UnsupportedOperationException("Unexpected MethodOverride subclass: "
216+
+ methodOverride.getClass().getName());
161217
}
218+
}
162219

220+
/**
221+
* CGLIB MethodInterceptor to override methods, replacing them with an
222+
* implementation that returns a bean looked up in the container.
223+
*/
224+
private static class LookupOverrideMethodInterceptor extends CglibIdentitySupport implements MethodInterceptor {
163225

164-
/**
165-
* CGLIB MethodInterceptor to override methods, replacing them with a call
166-
* to a generic MethodReplacer.
167-
*/
168-
private class ReplaceOverrideMethodInterceptor extends CglibIdentitySupport implements MethodInterceptor {
169-
170-
@Override
171-
public Object intercept(Object obj, Method method, Object[] args, MethodProxy mp) throws Throwable {
172-
ReplaceOverride ro = (ReplaceOverride) beanDefinition.getMethodOverrides().getOverride(method);
173-
// TODO could cache if a singleton for minor performance optimization
174-
MethodReplacer mr = (MethodReplacer) owner.getBean(ro.getMethodReplacerBeanName());
175-
return mr.reimplement(obj, method, args);
176-
}
226+
private final BeanFactory owner;
227+
228+
229+
LookupOverrideMethodInterceptor(RootBeanDefinition beanDefinition, BeanFactory owner) {
230+
super(beanDefinition);
231+
this.owner = owner;
177232
}
178233

234+
@Override
235+
public Object intercept(Object obj, Method method, Object[] args, MethodProxy mp) throws Throwable {
236+
// Cast is safe, as CallbackFilter filters are used selectively.
237+
LookupOverride lo = (LookupOverride) getBeanDefinition().getMethodOverrides().getOverride(method);
238+
return this.owner.getBean(lo.getBeanName());
239+
}
240+
}
179241

180-
/**
181-
* CGLIB object to filter method interception behavior.
182-
*/
183-
private class CallbackFilterImpl extends CglibIdentitySupport implements CallbackFilter {
242+
/**
243+
* CGLIB MethodInterceptor to override methods, replacing them with a call
244+
* to a generic MethodReplacer.
245+
*/
246+
private static class ReplaceOverrideMethodInterceptor extends CglibIdentitySupport implements MethodInterceptor {
184247

185-
@Override
186-
public int accept(Method method) {
187-
MethodOverride methodOverride = beanDefinition.getMethodOverrides().getOverride(method);
188-
if (logger.isTraceEnabled()) {
189-
logger.trace("Override for '" + method.getName() + "' is [" + methodOverride + "]");
190-
}
191-
if (methodOverride == null) {
192-
return PASSTHROUGH;
193-
}
194-
else if (methodOverride instanceof LookupOverride) {
195-
return LOOKUP_OVERRIDE;
196-
}
197-
else if (methodOverride instanceof ReplaceOverride) {
198-
return METHOD_REPLACER;
199-
}
200-
throw new UnsupportedOperationException(
201-
"Unexpected MethodOverride subclass: " + methodOverride.getClass().getName());
202-
}
248+
private final BeanFactory owner;
249+
250+
251+
ReplaceOverrideMethodInterceptor(RootBeanDefinition beanDefinition, BeanFactory owner) {
252+
super(beanDefinition);
253+
this.owner = owner;
254+
}
255+
256+
@Override
257+
public Object intercept(Object obj, Method method, Object[] args, MethodProxy mp) throws Throwable {
258+
ReplaceOverride ro = (ReplaceOverride) getBeanDefinition().getMethodOverrides().getOverride(method);
259+
// TODO could cache if a singleton for minor performance optimization
260+
MethodReplacer mr = owner.getBean(ro.getMethodReplacerBeanName(), MethodReplacer.class);
261+
return mr.reimplement(obj, method, args);
203262
}
204263
}
205264

0 commit comments

Comments
 (0)