root
/
spring-framework
mirror of https://github.com/spring-projects/spring-framework.git
1
0
Fork 0
Code Issues Actions Packages Projects Releases Wiki Activity

Improve annotation processing thread-safety 

Commit http://bit.ly/nXumTs ensured that component methods and fields
marked with 'common annotations' such as @Resource, @PostConstruct and
@PreDestroy are invoked/assigned once and only once, even if multiple
instances of the CommonAnnotationBeanPostProcessor are processing the
same bean factory.

The implementation works against the InjectionMetadata API, adding and
removing these members from sets that track whether they are already
'externally managed', i.e. that another CABPP has already handled them,
thus avoiding redundant processing.

Prior to this change, the #remove operations against these sets were
not synchronized. In a single-threaded context this is fine thanks to
logic in AbstractAutowireCapableBeanFactory#doCreateBean that checks to
see whether a given bean definition has already been post processed.
However, as reported by SPR-8598, certain cases involving multiple
threads and annotated prototype-scoped beans can cause concurrent
modification exceptions during the #remove operation (ostensibly because
another thread is attempting to do the same removal at the same time,
though this has yet to be reproduced in isolation).

Now the sets originally introduced by the commit above are decorated
with Collections#synchronizedSet and any iterations over those sets
are synchronized properly. This change should have low performance
impact as such processing happens at container startup time (save for
non-singleton lookups at runtime), and there should be little
contention in any case.

Issue: SPR-8598
This commit is contained in:
Chris Beams 2011-09-12 23:05:01 +00:00
parent c8add61a72
commit a456a1a0e3
2 changed files with 32 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
org.springframework.beans/src/main/java/org/springframework/beans/factory/annotation/InitDestroyAnnotationBeanPostProcessor.java
View File

@ -24,6 +24,7 @@ import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.util.Collection;
import java.util.Collections;
import java.util.Iterator;
import java.util.LinkedHashSet;
import java.util.LinkedList;
@ -246,7 +247,7 @@ public class InitDestroyAnnotationBeanPostProcessor
public LifecycleMetadata(Class<?> targetClass, Collection<LifecycleElement> initMethods,
Collection<LifecycleElement> destroyMethods) {
this.initMethods = new LinkedHashSet<LifecycleElement>();
this.initMethods = Collections.synchronizedSet(new LinkedHashSet<LifecycleElement>());
for (LifecycleElement element : initMethods) {
if (logger.isDebugEnabled()) {
logger.debug("Found init method on class [" + targetClass.getName() + "]: " + element);
@ -254,7 +255,7 @@ public class InitDestroyAnnotationBeanPostProcessor
this.initMethods.add(element);
}
this.destroyMethods = new LinkedHashSet<LifecycleElement>();
this.destroyMethods = Collections.synchronizedSet(new LinkedHashSet<LifecycleElement>());
for (LifecycleElement element : destroyMethods) {
if (logger.isDebugEnabled()) {
logger.debug("Found destroy method on class [" + targetClass.getName() + "]: " + element);
@ -264,22 +265,26 @@ public class InitDestroyAnnotationBeanPostProcessor
}
public void checkConfigMembers(RootBeanDefinition beanDefinition) {
for (Iterator<LifecycleElement> it = this.initMethods.iterator(); it.hasNext();) {
String methodIdentifier = it.next().getIdentifier();
if (!beanDefinition.isExternallyManagedInitMethod(methodIdentifier)) {
beanDefinition.registerExternallyManagedInitMethod(methodIdentifier);
}
else {
it.remove();
synchronized(this.initMethods) {
for (Iterator<LifecycleElement> it = this.initMethods.iterator(); it.hasNext();) {
String methodIdentifier = it.next().getIdentifier();
if (!beanDefinition.isExternallyManagedInitMethod(methodIdentifier)) {
beanDefinition.registerExternallyManagedInitMethod(methodIdentifier);
}
else {
it.remove();
}
}
}
for (Iterator<LifecycleElement> it = this.destroyMethods.iterator(); it.hasNext();) {
String methodIdentifier = it.next().getIdentifier();
if (!beanDefinition.isExternallyManagedDestroyMethod(methodIdentifier)) {
beanDefinition.registerExternallyManagedDestroyMethod(methodIdentifier);
}
else {
it.remove();
synchronized(this.destroyMethods) {
for (Iterator<LifecycleElement> it = this.destroyMethods.iterator(); it.hasNext();) {
String methodIdentifier = it.next().getIdentifier();
if (!beanDefinition.isExternallyManagedDestroyMethod(methodIdentifier)) {
beanDefinition.registerExternallyManagedDestroyMethod(methodIdentifier);
}
else {
it.remove();
}
}
}
}

19
org.springframework.beans/src/main/java/org/springframework/beans/factory/annotation/InjectionMetadata.java
View File

@ -22,6 +22,7 @@ import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Member;
import java.lang.reflect.Method;
import java.util.Collection;
import java.util.Collections;
import java.util.Iterator;
import java.util.LinkedHashSet;
import java.util.Set;
@ -53,7 +54,7 @@ public class InjectionMetadata {
public InjectionMetadata(Class targetClass, Collection<InjectedElement> elements) {
this.injectedElements = new LinkedHashSet<InjectedElement>();
this.injectedElements = Collections.synchronizedSet(new LinkedHashSet<InjectedElement>());
for (InjectedElement element : elements) {
if (logger.isDebugEnabled()) {
logger.debug("Found injected element on class [" + targetClass.getName() + "]: " + element);
@ -63,13 +64,15 @@ public class InjectionMetadata {
}
public void checkConfigMembers(RootBeanDefinition beanDefinition) {
for (Iterator<InjectedElement> it = this.injectedElements.iterator(); it.hasNext();) {
Member member = it.next().getMember();
if (!beanDefinition.isExternallyManagedConfigMember(member)) {
beanDefinition.registerExternallyManagedConfigMember(member);
}
else {
it.remove();
synchronized(this.injectedElements) {
for (Iterator<InjectedElement> it = this.injectedElements.iterator(); it.hasNext();) {
Member member = it.next().getMember();
if (!beanDefinition.isExternallyManagedConfigMember(member)) {
beanDefinition.registerExternallyManagedConfigMember(member);
}
else {
it.remove();
}
}
}
}