From 9b26e4f1ada2712cfe9c2ab6bc04d9574d999e37 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Thu, 26 Feb 2015 18:35:06 +0100 Subject: [PATCH] Avoid potential deadlocks between event multicaster and singleton registry through shared lock Issue: SPR-12739 (cherry picked from commit 772552b) --- .../support/DefaultSingletonBeanRegistry.java | 8 +- .../AbstractApplicationEventMulticaster.java | 88 ++++++++++--------- 2 files changed, 52 insertions(+), 44 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultSingletonBeanRegistry.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultSingletonBeanRegistry.java index 4ad522a1c7..99bf3c3cc0 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultSingletonBeanRegistry.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultSingletonBeanRegistry.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 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. @@ -317,7 +317,7 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements /** * Callback before singleton creation. - *

Default implementation register the singleton as currently in creation. + *

The default implementation register the singleton as currently in creation. * @param beanName the name of the singleton about to be created * @see #isSingletonCurrentlyInCreation */ @@ -539,13 +539,13 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements } /** - * Expose the singleton mutex to subclasses. + * Exposes the singleton mutex to subclasses and external collaborators. *

Subclasses should synchronize on the given Object if they perform * any sort of extended singleton creation phase. In particular, subclasses * should not 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; } diff --git a/spring-context/src/main/java/org/springframework/context/event/AbstractApplicationEventMulticaster.java b/spring-context/src/main/java/org/springframework/context/event/AbstractApplicationEventMulticaster.java index 87d7c38f59..2957f98f4d 100644 --- a/spring-context/src/main/java/org/springframework/context/event/AbstractApplicationEventMulticaster.java +++ b/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. @@ -27,6 +27,7 @@ import org.springframework.beans.factory.BeanClassLoaderAware; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryAware; 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; @@ -64,42 +65,8 @@ public abstract class AbstractApplicationEventMulticaster private BeanFactory beanFactory; + private Object retrievalMutex = this.defaultRetriever; - public void addApplicationListener(ApplicationListener listener) { - synchronized (this.defaultRetriever) { - this.defaultRetriever.applicationListeners.add(listener); - this.retrieverCache.clear(); - } - } - - public void addApplicationListenerBean(String listenerBeanName) { - synchronized (this.defaultRetriever) { - this.defaultRetriever.applicationListenerBeans.add(listenerBeanName); - this.retrieverCache.clear(); - } - } - - public void removeApplicationListener(ApplicationListener listener) { - synchronized (this.defaultRetriever) { - this.defaultRetriever.applicationListeners.remove(listener); - this.retrieverCache.clear(); - } - } - - public void removeApplicationListenerBean(String listenerBeanName) { - synchronized (this.defaultRetriever) { - this.defaultRetriever.applicationListenerBeans.remove(listenerBeanName); - this.retrieverCache.clear(); - } - } - - public void removeAllListeners() { - synchronized (this.defaultRetriever) { - this.defaultRetriever.applicationListeners.clear(); - this.defaultRetriever.applicationListenerBeans.clear(); - this.retrieverCache.clear(); - } - } public void setBeanClassLoader(ClassLoader classLoader) { this.beanClassLoader = classLoader; @@ -110,6 +77,9 @@ public abstract class AbstractApplicationEventMulticaster if (this.beanClassLoader == null && beanFactory instanceof ConfigurableBeanFactory) { this.beanClassLoader = ((ConfigurableBeanFactory) beanFactory).getBeanClassLoader(); } + if (beanFactory instanceof AbstractBeanFactory) { + this.retrievalMutex = ((AbstractBeanFactory) beanFactory).getSingletonMutex(); + } } private BeanFactory getBeanFactory() { @@ -121,13 +91,50 @@ public abstract class AbstractApplicationEventMulticaster } + public void addApplicationListener(ApplicationListener listener) { + synchronized (this.retrievalMutex) { + this.defaultRetriever.applicationListeners.add(listener); + this.retrieverCache.clear(); + } + } + + public void addApplicationListenerBean(String listenerBeanName) { + synchronized (this.retrievalMutex) { + this.defaultRetriever.applicationListenerBeans.add(listenerBeanName); + this.retrieverCache.clear(); + } + } + + public void removeApplicationListener(ApplicationListener listener) { + synchronized (this.retrievalMutex) { + this.defaultRetriever.applicationListeners.remove(listener); + this.retrieverCache.clear(); + } + } + + public void removeApplicationListenerBean(String listenerBeanName) { + synchronized (this.retrievalMutex) { + this.defaultRetriever.applicationListenerBeans.remove(listenerBeanName); + this.retrieverCache.clear(); + } + } + + public void removeAllListeners() { + synchronized (this.retrievalMutex) { + this.defaultRetriever.applicationListeners.clear(); + this.defaultRetriever.applicationListenerBeans.clear(); + this.retrieverCache.clear(); + } + } + + /** * Return a Collection containing all ApplicationListeners. * @return a Collection of ApplicationListeners * @see org.springframework.context.ApplicationListener */ protected Collection getApplicationListeners() { - synchronized (this.defaultRetriever) { + synchronized (this.retrievalMutex) { return this.defaultRetriever.getApplicationListeners(); } } @@ -156,13 +163,14 @@ public abstract class AbstractApplicationEventMulticaster (ClassUtils.isCacheSafe(eventType, 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 listeners = retrieveApplicationListeners(eventType, sourceType, retriever); + Collection listeners = + retrieveApplicationListeners(eventType, sourceType, retriever); this.retrieverCache.put(cacheKey, retriever); return listeners; } @@ -186,7 +194,7 @@ public abstract class AbstractApplicationEventMulticaster LinkedList allListeners = new LinkedList(); Set listeners; Set listenerBeans; - synchronized (this.defaultRetriever) { + synchronized (this.retrievalMutex) { listeners = new LinkedHashSet(this.defaultRetriever.applicationListeners); listenerBeans = new LinkedHashSet(this.defaultRetriever.applicationListenerBeans); }