Avoid wide locks in order to remove deadlock potential in case of multi-threaded singleton creation/destruction

Issue: SPR-10020
Issue: SPR-8471

Author: jhoeller

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

@@ -576,35 +576,36 @@ public void preInstantiateSingletons() throws BeansException {
 		if (this.logger.isInfoEnabled()) {
 			this.logger.info("Pre-instantiating singletons in " + this);
 		}
+		List<String> beanNames;
 		synchronized (this.beanDefinitionMap) {
 			// Iterate over a copy to allow for init methods which in turn register new bean definitions.
 			// While this may not be part of the regular factory bootstrap, it does otherwise work fine.
-			List<String> beanNames = new ArrayList<String>(this.beanDefinitionNames);
-			for (String beanName : beanNames) {
-				RootBeanDefinition bd = getMergedLocalBeanDefinition(beanName);
-				if (!bd.isAbstract() && bd.isSingleton() && !bd.isLazyInit()) {
-					if (isFactoryBean(beanName)) {
-						final FactoryBean<?> factory = (FactoryBean<?>) getBean(FACTORY_BEAN_PREFIX + beanName);
-						boolean isEagerInit;
-						if (System.getSecurityManager() != null && factory instanceof SmartFactoryBean) {
-							isEagerInit = AccessController.doPrivileged(new PrivilegedAction<Boolean>() {
-								public Boolean run() {
-									return ((SmartFactoryBean<?>) factory).isEagerInit();
-								}
-							}, getAccessControlContext());
-						}
-						else {
-							isEagerInit = (factory instanceof SmartFactoryBean &&
-									((SmartFactoryBean<?>) factory).isEagerInit());
-						}
-						if (isEagerInit) {
-							getBean(beanName);
-						}
+			beanNames = new ArrayList<String>(this.beanDefinitionNames);
+		}
+		for (String beanName : beanNames) {
+			RootBeanDefinition bd = getMergedLocalBeanDefinition(beanName);
+			if (!bd.isAbstract() && bd.isSingleton() && !bd.isLazyInit()) {
+				if (isFactoryBean(beanName)) {
+					final FactoryBean<?> factory = (FactoryBean<?>) getBean(FACTORY_BEAN_PREFIX + beanName);
+					boolean isEagerInit;
+					if (System.getSecurityManager() != null && factory instanceof SmartFactoryBean) {
+						isEagerInit = AccessController.doPrivileged(new PrivilegedAction<Boolean>() {
+							public Boolean run() {
+								return ((SmartFactoryBean<?>) factory).isEagerInit();
+							}
+						}, getAccessControlContext());
 					}
 					else {
+						isEagerInit = (factory instanceof SmartFactoryBean &&
+								((SmartFactoryBean<?>) factory).isEagerInit());
+					}
+					if (isEagerInit) {
 						getBean(beanName);
 					}
 				}
+				else {
+					getBean(beanName);
+				}
 			}
 		}
 	}
@@ -650,9 +651,9 @@ public void registerBeanDefinition(String beanName, BeanDefinition beanDefinitio
 				this.frozenBeanDefinitionNames = null;
 			}
 			this.beanDefinitionMap.put(beanName, beanDefinition);
-
-			resetBeanDefinition(beanName);
 		}
+
+		resetBeanDefinition(beanName);
 	}
 
 	public void removeBeanDefinition(String beanName) throws NoSuchBeanDefinitionException {
@@ -668,9 +669,9 @@ public void removeBeanDefinition(String beanName) throws NoSuchBeanDefinitionExc
 			}
 			this.beanDefinitionNames.remove(beanName);
 			this.frozenBeanDefinitionNames = null;
-
-			resetBeanDefinition(beanName);
 		}
+
+		resetBeanDefinition(beanName);
 	}
 
 	/**
@@ -685,9 +686,7 @@ protected void resetBeanDefinition(String beanName) {
 		// Remove corresponding bean from singleton cache, if any. Shouldn't usually
 		// be necessary, rather just meant for overriding a context's default beans
 		// (e.g. the default StaticMessageSource in a StaticApplicationContext).
-		synchronized (getSingletonMutex()) {
-			destroySingleton(beanName);
-		}
+		destroySingleton(beanName);
 
 		// Remove any assumptions about by-type mappings
 		this.singletonBeanNamesByType.clear();

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

@@ -447,11 +447,12 @@ public void destroySingletons() {
 			this.singletonsCurrentlyInDestruction = true;
 		}
 
+		String[] disposableBeanNames;
 		synchronized (this.disposableBeans) {
-			String[] disposableBeanNames = StringUtils.toStringArray(this.disposableBeans.keySet());
-			for (int i = disposableBeanNames.length - 1; i >= 0; i--) {
-				destroySingleton(disposableBeanNames[i]);
-			}
+			disposableBeanNames = StringUtils.toStringArray(this.disposableBeans.keySet());
+		}
+		for (int i = disposableBeanNames.length - 1; i >= 0; i--) {
+			destroySingleton(disposableBeanNames[i]);
 		}
 
 		this.containedBeanMap.clear();

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2010 the original author or authors.
+ * Copyright 2002-2012 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.
@@ -113,7 +113,9 @@ private BeanFactory getBeanFactory() {
 	 * @see org.springframework.context.ApplicationListener
 	 */
 	protected Collection<ApplicationListener> getApplicationListeners() {
-		return this.defaultRetriever.getApplicationListeners();
+		synchronized (this.defaultRetriever) {
+			return this.defaultRetriever.getApplicationListeners();
+		}
 	}
 
 	/**
@@ -135,26 +137,30 @@ protected Collection<ApplicationListener> getApplicationListeners(ApplicationEve
 		else {
 			retriever = new ListenerRetriever(true);
 			LinkedList<ApplicationListener> allListeners = new LinkedList<ApplicationListener>();
+			Set<ApplicationListener> listeners;
+			Set<String> listenerBeans;
 			synchronized (this.defaultRetriever) {
-				for (ApplicationListener listener : this.defaultRetriever.applicationListeners) {
-					if (supportsEvent(listener, eventType, sourceType)) {
-						retriever.applicationListeners.add(listener);
-						allListeners.add(listener);
-					}
+				listeners = new LinkedHashSet<ApplicationListener>(this.defaultRetriever.applicationListeners);
+				listenerBeans = new LinkedHashSet<String>(this.defaultRetriever.applicationListenerBeans);
+			}
+			for (ApplicationListener listener : listeners) {
+				if (supportsEvent(listener, eventType, sourceType)) {
+					retriever.applicationListeners.add(listener);
+					allListeners.add(listener);
 				}
-				if (!this.defaultRetriever.applicationListenerBeans.isEmpty()) {
-					BeanFactory beanFactory = getBeanFactory();
-					for (String listenerBeanName : this.defaultRetriever.applicationListenerBeans) {
-						ApplicationListener listener = beanFactory.getBean(listenerBeanName, ApplicationListener.class);
-						if (!allListeners.contains(listener) && supportsEvent(listener, eventType, sourceType)) {
-							retriever.applicationListenerBeans.add(listenerBeanName);
-							allListeners.add(listener);
-						}
+			}
+			if (!listenerBeans.isEmpty()) {
+				BeanFactory beanFactory = getBeanFactory();
+				for (String listenerBeanName : listenerBeans) {
+					ApplicationListener listener = beanFactory.getBean(listenerBeanName, ApplicationListener.class);
+					if (!allListeners.contains(listener) && supportsEvent(listener, eventType, sourceType)) {
+						retriever.applicationListenerBeans.add(listenerBeanName);
+						allListeners.add(listener);
 					}
 				}
-				OrderComparator.sort(allListeners);
-				this.retrieverCache.put(cacheKey, retriever);
 			}
+			OrderComparator.sort(allListeners);
+			this.retrieverCache.put(cacheKey, retriever);
 			return allListeners;
 		}
 	}