diff --git a/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/BeanFactoryAspectInstanceFactory.java b/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/BeanFactoryAspectInstanceFactory.java index 49e31990f64..02de8995acf 100644 --- a/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/BeanFactoryAspectInstanceFactory.java +++ b/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/BeanFactoryAspectInstanceFactory.java @@ -92,8 +92,19 @@ public class BeanFactoryAspectInstanceFactory implements MetadataAwareAspectInst } public Object getAspectCreationMutex() { - return (this.beanFactory instanceof AbstractBeanFactory ? - ((AbstractBeanFactory) this.beanFactory).getSingletonMutex() : this); + if (this.beanFactory != null) { + if (this.beanFactory.isSingleton(this.name)) { + // Rely on singleton semantics provided by the factory -> no local lock. + return null; + } + else if (this.beanFactory instanceof AbstractBeanFactory) { + // No singleton guarantees from the factory -> let's lock locally but + // reuse the factory's singleton lock, just in case a lazy dependency + // of our advice bean happens to trigger the singleton lock implicitly... + return ((AbstractBeanFactory) this.beanFactory).getSingletonMutex(); + } + } + return this; } /** diff --git a/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/LazySingletonAspectInstanceFactoryDecorator.java b/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/LazySingletonAspectInstanceFactoryDecorator.java index 80788007c5c..df5a628b5ed 100644 --- a/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/LazySingletonAspectInstanceFactoryDecorator.java +++ b/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/LazySingletonAspectInstanceFactoryDecorator.java @@ -48,9 +48,14 @@ public class LazySingletonAspectInstanceFactoryDecorator implements MetadataAwar if (this.maaif instanceof BeanFactoryAspectInstanceFactory) { mutex = ((BeanFactoryAspectInstanceFactory) this.maaif).getAspectCreationMutex(); } - synchronized (mutex) { - if (this.materialized == null) { - this.materialized = this.maaif.getAspectInstance(); + if (mutex == null) { + this.materialized = this.maaif.getAspectInstance(); + } + else { + synchronized (mutex) { + if (this.materialized == null) { + this.materialized = this.maaif.getAspectInstance(); + } } } } diff --git a/spring-aop/src/main/java/org/springframework/aop/support/AbstractBeanFactoryPointcutAdvisor.java b/spring-aop/src/main/java/org/springframework/aop/support/AbstractBeanFactoryPointcutAdvisor.java index c1fda545225..b6eb42d935e 100644 --- a/spring-aop/src/main/java/org/springframework/aop/support/AbstractBeanFactoryPointcutAdvisor.java +++ b/spring-aop/src/main/java/org/springframework/aop/support/AbstractBeanFactoryPointcutAdvisor.java @@ -46,7 +46,7 @@ public abstract class AbstractBeanFactoryPointcutAdvisor extends AbstractPointcu private BeanFactory beanFactory; - private transient Advice advice; + private transient volatile Advice advice; private transient volatile Object adviceMonitor = new Object(); @@ -96,12 +96,28 @@ public abstract class AbstractBeanFactoryPointcutAdvisor extends AbstractPointcu } public Advice getAdvice() { - synchronized (this.adviceMonitor) { - if (this.advice == null && this.adviceBeanName != null) { - Assert.state(this.beanFactory != null, "BeanFactory must be set to resolve 'adviceBeanName'"); - this.advice = this.beanFactory.getBean(this.adviceBeanName, Advice.class); + Advice advice = this.advice; + if (advice != null || this.adviceBeanName == null) { + return advice; + } + + Assert.state(this.beanFactory != null, "BeanFactory must be set to resolve 'adviceBeanName'"); + if (this.beanFactory.isSingleton(this.adviceBeanName)) { + // Rely on singleton semantics provided by the factory. + advice = this.beanFactory.getBean(this.adviceBeanName, Advice.class); + this.advice = advice; + return advice; + } + else { + // No singleton guarantees from the factory -> let's lock locally but + // reuse the factory's singleton lock, just in case a lazy dependency + // of our advice bean happens to trigger the singleton lock implicitly... + synchronized (this.adviceMonitor) { + if (this.advice == null) { + this.advice = this.beanFactory.getBean(this.adviceBeanName, Advice.class); + } + return this.advice; } - return this.advice; } }