Skip to content

Commit f779c19

Browse files
dridicbeams
authored andcommitted
Fix scoped-proxy memory leak w/ @resource injection
Prior to this change, request-scoped components having @Resource-injected dependencies caused a memory leak in DefaultListableBeanFactory#dependenciesForBeanMap. Consider the following example: @component @scope(value="request", proxyMode=ScopedProxyMode.TARGET_CLASS) public class MyComponent { @resource private HttpServletRequest request; // ... } The bean name for "MyComponent" will end up being 'scopedTarget.myComponent', which will become a key in the #dependenciesForBeanMap structure. On the first request, the injected HttpServletRequest bean will be a proxy and will internally have a bean name of the form "$Proxy10@1a3a2a52". This name will be added to the Set value associated with the 'scopedTarget.myComponent' entry in #dependenciesForBeanMap. On the second request, the process will repeat, but the injected HttpServletRequest will be a different proxy instance, thus having a different identity hex string, e.g. "$Proxy10@5eba06ff". This name will also be added to the Set value associated with the 'scopedTarget.myComponent' entry in #dependenciesForBeanMap, and this is the source of the leak: a new entry is added to the set on each request but should be added only once. This commit fixes the leak by introducing caching to CommonAnnotationBeanPostProcessor#ResourceElement similar to that already present in AutowiredAnnotationBeanPostProcessor#AutowiredFieldElement and #AutowiredMethodElement. Essentially, each ResourceElement instance now tracks whether it has been created, caches the ultimate value to be injected and returns it eagerly if necessary. Besides solving the memory leak, this has the side effect of avoiding unnecessary proxy creation. This fix also explains clearly why injection into request-scoped components using @Autowired never suffered this memory leak: because the correct caching was already in place. Because @resource is considerably less-frequently used than @Autowired, and given that this particular injection arrangement is relatively infrequent, it becomes understandable how this bug has been present without being reported since the introduction of @resource support in Spring 2.5: developers were unlikely to encounter it in the first place; and if they did, the leak was minor enough (adding strings to a Set), that it could potentially go unnoticed indefinitely depending on request volumes and available memory. Issue: SPR-9176
1 parent e85e614 commit f779c19

File tree

1 file changed

+21
-4
lines changed

1 file changed

+21
-4
lines changed

spring-context/src/main/java/org/springframework/context/annotation/CommonAnnotationBeanPostProcessor.java

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2011 the original author or authors.
2+
* Copyright 2002-2012 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -35,6 +35,7 @@
3535
import java.util.Map;
3636
import java.util.Set;
3737
import java.util.concurrent.ConcurrentHashMap;
38+
3839
import javax.annotation.PostConstruct;
3940
import javax.annotation.PreDestroy;
4041
import javax.annotation.Resource;
@@ -509,9 +510,12 @@ public final DependencyDescriptor getDependencyDescriptor() {
509510
*/
510511
private class ResourceElement extends LookupElement {
511512

512-
@SuppressWarnings("unused")
513513
protected boolean shareable = true;
514514

515+
private volatile boolean cached = false;
516+
517+
private volatile Object cachedFieldValue;
518+
515519
public ResourceElement(Member member, PropertyDescriptor pd) {
516520
super(member, pd);
517521
}
@@ -546,7 +550,20 @@ else if (beanFactory instanceof ConfigurableBeanFactory){
546550

547551
@Override
548552
protected Object getResourceToInject(Object target, String requestingBeanName) {
549-
return getResource(this, requestingBeanName);
553+
Object value = null;
554+
if (this.cached && this.shareable) {
555+
value = this.cachedFieldValue;
556+
}
557+
synchronized (this) {
558+
if (!this.cached) {
559+
value = getResource(this, requestingBeanName);
560+
if (value != null && this.shareable) {
561+
this.cachedFieldValue = value;
562+
this.cached = true;
563+
}
564+
}
565+
}
566+
return value;
550567
}
551568
}
552569

@@ -724,4 +741,4 @@ public Class<?> getDependencyType() {
724741
}
725742
}
726743

727-
}
744+
}

0 commit comments

Comments
 (0)