From 702587bec0b671e755a5257d2b0a454b7dc49cb8 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Mon, 9 Jul 2018 12:13:49 +0100 Subject: [PATCH] GenericScope can delegate actual bean creation to lower level GenericScope has some requirements to synchronize the bean creation and destruction phases, which exposes it to potential deadlocks (e.g. as reported in #383). The fact that the bean factory is also taking locks is taken advantage of to ensure that the same lock is taken on bean creation in all cases. So all threads have to wait for the same lock, and there is no danger of interleaving. --- .../cloud/context/scope/GenericScope.java | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/context/scope/GenericScope.java b/spring-cloud-context/src/main/java/org/springframework/cloud/context/scope/GenericScope.java index 8946d353..2c9ed58b 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/context/scope/GenericScope.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/context/scope/GenericScope.java @@ -49,6 +49,7 @@ import org.springframework.beans.factory.config.Scope; import org.springframework.beans.factory.support.BeanDefinitionRegistry; import org.springframework.beans.factory.support.BeanDefinitionRegistryPostProcessor; import org.springframework.beans.factory.support.DefaultListableBeanFactory; +import org.springframework.beans.factory.support.DefaultSingletonBeanRegistry; import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.expression.Expression; import org.springframework.expression.ExpressionParser; @@ -360,7 +361,7 @@ public class GenericScope implements Scope, BeanFactoryPostProcessor, * @author Dave Syer * */ - private static class BeanLifecycleWrapper { + private class BeanLifecycleWrapper { private Object bean; @@ -385,9 +386,23 @@ public class GenericScope implements Scope, BeanFactoryPostProcessor, public Object getBean() { if (this.bean == null) { - synchronized (this.name) { - if (this.bean == null) { - this.bean = this.objectFactory.getObject(); + if (beanFactory instanceof DefaultSingletonBeanRegistry + && this.callback == null) { + // DefaultSingletonBeanRegistry takes a lock in here that we need to + // have in case two threads want to create a bean at the same time. + this.bean = ((DefaultSingletonBeanRegistry) beanFactory) + .getSingleton(this.name, this.objectFactory); + // The callback is null at this point so nothing will happen, but we + // don't want the bean registered as a singleton, otherwise it won't + // be scoped. + ((DefaultSingletonBeanRegistry) beanFactory) + .destroySingleton(this.name); + } + else { + synchronized (this.name) { + if (this.bean == null) { + this.bean = this.objectFactory.getObject(); + } } } } @@ -497,7 +512,8 @@ public class GenericScope implements Scope, BeanFactoryPostProcessor, } return invocation.proceed(); } - // see gh-349. Throw the original exception rather than the UndeclaredThrowableException + // see gh-349. Throw the original exception rather than the + // UndeclaredThrowableException catch (UndeclaredThrowableException e) { throw e.getUndeclaredThrowable(); }