Avoid potential deadlocks between event multicaster and singleton registry through shared lock

Issue: SPR-12739
(cherry picked from commit 81102de)

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -607,13 +607,13 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements
 	}
 
 	/**
-	 * Expose the singleton mutex to subclasses.
+	 * Exposes the singleton mutex to subclasses and external collaborators.
 	 * <p>Subclasses should synchronize on the given Object if they perform
 	 * any sort of extended singleton creation phase. In particular, subclasses
 	 * should <i>not</i> have their own mutexes involved in singleton creation,
 	 * to avoid the potential for deadlocks in lazy-init situations.
 	 */
-	protected final Object getSingletonMutex() {
+	public final Object getSingletonMutex() {
 		return this.singletonObjects;
 	}
 

spring-context/src/main/java/org/springframework/context/event/AbstractApplicationEventMulticaster.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -28,6 +28,7 @@ import org.springframework.beans.factory.BeanFactory;
 import org.springframework.beans.factory.BeanFactoryAware;
 import org.springframework.beans.factory.NoSuchBeanDefinitionException;
 import org.springframework.beans.factory.config.ConfigurableBeanFactory;
+import org.springframework.beans.factory.support.AbstractBeanFactory;
 import org.springframework.context.ApplicationEvent;
 import org.springframework.context.ApplicationListener;
 import org.springframework.core.OrderComparator;
@@ -65,10 +66,37 @@ public abstract class AbstractApplicationEventMulticaster
 
 	private BeanFactory beanFactory;
 
+	private Object retrievalMutex = this.defaultRetriever;
+
+
+	@Override
+	public void setBeanClassLoader(ClassLoader classLoader) {
+		this.beanClassLoader = classLoader;
+	}
+
+	@Override
+	public void setBeanFactory(BeanFactory beanFactory) {
+		this.beanFactory = beanFactory;
+		if (this.beanClassLoader == null && beanFactory instanceof ConfigurableBeanFactory) {
+			this.beanClassLoader = ((ConfigurableBeanFactory) beanFactory).getBeanClassLoader();
+		}
+		if (beanFactory instanceof AbstractBeanFactory) {
+			this.retrievalMutex = ((AbstractBeanFactory) beanFactory).getSingletonMutex();
+		}
+	}
+
+	private BeanFactory getBeanFactory() {
+		if (this.beanFactory == null) {
+			throw new IllegalStateException("ApplicationEventMulticaster cannot retrieve listener beans " +
+					"because it is not associated with a BeanFactory");
+		}
+		return this.beanFactory;
+	}
+
 
 	@Override
 	public void addApplicationListener(ApplicationListener<?> listener) {
-		synchronized (this.defaultRetriever) {
+		synchronized (this.retrievalMutex) {
 			this.defaultRetriever.applicationListeners.add(listener);
 			this.retrieverCache.clear();
 		}
@@ -76,7 +104,7 @@ public abstract class AbstractApplicationEventMulticaster
 
 	@Override
 	public void addApplicationListenerBean(String listenerBeanName) {
-		synchronized (this.defaultRetriever) {
+		synchronized (this.retrievalMutex) {
 			this.defaultRetriever.applicationListenerBeans.add(listenerBeanName);
 			this.retrieverCache.clear();
 		}
@@ -84,7 +112,7 @@ public abstract class AbstractApplicationEventMulticaster
 
 	@Override
 	public void removeApplicationListener(ApplicationListener<?> listener) {
-		synchronized (this.defaultRetriever) {
+		synchronized (this.retrievalMutex) {
 			this.defaultRetriever.applicationListeners.remove(listener);
 			this.retrieverCache.clear();
 		}
@@ -92,7 +120,7 @@ public abstract class AbstractApplicationEventMulticaster
 
 	@Override
 	public void removeApplicationListenerBean(String listenerBeanName) {
-		synchronized (this.defaultRetriever) {
+		synchronized (this.retrievalMutex) {
 			this.defaultRetriever.applicationListenerBeans.remove(listenerBeanName);
 			this.retrieverCache.clear();
 		}
@@ -100,34 +128,13 @@ public abstract class AbstractApplicationEventMulticaster
 
 	@Override
 	public void removeAllListeners() {
-		synchronized (this.defaultRetriever) {
+		synchronized (this.retrievalMutex) {
 			this.defaultRetriever.applicationListeners.clear();
 			this.defaultRetriever.applicationListenerBeans.clear();
 			this.retrieverCache.clear();
 		}
 	}
 
-	@Override
-	public void setBeanClassLoader(ClassLoader classLoader) {
-		this.beanClassLoader = classLoader;
-	}
-
-	@Override
-	public void setBeanFactory(BeanFactory beanFactory) {
-		this.beanFactory = beanFactory;
-		if (this.beanClassLoader == null && beanFactory instanceof ConfigurableBeanFactory) {
-			this.beanClassLoader = ((ConfigurableBeanFactory) beanFactory).getBeanClassLoader();
-		}
-	}
-
-	private BeanFactory getBeanFactory() {
-		if (this.beanFactory == null) {
-			throw new IllegalStateException("ApplicationEventMulticaster cannot retrieve listener beans " +
-					"because it is not associated with a BeanFactory");
-		}
-		return this.beanFactory;
-	}
-
 
 	/**
 	 * Return a Collection containing all ApplicationListeners.
@@ -135,7 +142,7 @@ public abstract class AbstractApplicationEventMulticaster
 	 * @see org.springframework.context.ApplicationListener
 	 */
 	protected Collection<ApplicationListener<?>> getApplicationListeners() {
-		synchronized (this.defaultRetriever) {
+		synchronized (this.retrievalMutex) {
 			return this.defaultRetriever.getApplicationListeners();
 		}
 	}
@@ -163,13 +170,14 @@ public abstract class AbstractApplicationEventMulticaster
 				(ClassUtils.isCacheSafe(event.getClass(), this.beanClassLoader) &&
 						(sourceType == null || ClassUtils.isCacheSafe(sourceType, this.beanClassLoader)))) {
 			// Fully synchronized building and caching of a ListenerRetriever
-			synchronized (this.defaultRetriever) {
+			synchronized (this.retrievalMutex) {
 				retriever = this.retrieverCache.get(cacheKey);
 				if (retriever != null) {
 					return retriever.getApplicationListeners();
 				}
 				retriever = new ListenerRetriever(true);
-				Collection<ApplicationListener<?>> listeners = retrieveApplicationListeners(event, sourceType, retriever);
+				Collection<ApplicationListener<?>> listeners =
+						retrieveApplicationListeners(event, sourceType, retriever);
 				this.retrieverCache.put(cacheKey, retriever);
 				return listeners;
 			}
@@ -193,7 +201,7 @@ public abstract class AbstractApplicationEventMulticaster
 		LinkedList<ApplicationListener<?>> allListeners = new LinkedList<ApplicationListener<?>>();
 		Set<ApplicationListener<?>> listeners;
 		Set<String> listenerBeans;
-		synchronized (this.defaultRetriever) {
+		synchronized (this.retrievalMutex) {
 			listeners = new LinkedHashSet<ApplicationListener<?>>(this.defaultRetriever.applicationListeners);
 			listenerBeans = new LinkedHashSet<String>(this.defaultRetriever.applicationListenerBeans);
 		}