Avoid scoped destruction callbacks in case of no post-processor actually applying

Issue: SPR-13744

spring-beans/src/main/java/org/springframework/beans/factory/annotation/InitDestroyAnnotationBeanPostProcessor.java

@@ -136,7 +136,7 @@ public class InitDestroyAnnotationBeanPostProcessor
 			throw new BeanCreationException(beanName, "Invocation of init method failed", ex.getTargetException());
 		}
 		catch (Throwable ex) {
-			throw new BeanCreationException(beanName, "Couldn't invoke init method", ex);
+			throw new BeanCreationException(beanName, "Failed to invoke init method", ex);
 		}
 		return bean;
 	}
@@ -162,10 +162,15 @@ public class InitDestroyAnnotationBeanPostProcessor
 			}
 		}
 		catch (Throwable ex) {
-			logger.error("Couldn't invoke destroy method on bean with name '" + beanName + "'", ex);
+			logger.error("Failed to invoke destroy method on bean with name '" + beanName + "'", ex);
 		}
 	}
 
+	@Override
+	public boolean requiresDestruction(Object bean) {
+		return findLifecycleMetadata(bean.getClass()).hasDestroyMethods();
+	}
+
 
 	private LifecycleMetadata findLifecycleMetadata(Class<?> clazz) {
 		if (this.lifecycleMetadataCache == null) {
@@ -308,11 +313,11 @@ public class InitDestroyAnnotationBeanPostProcessor
 		}
 
 		public void invokeDestroyMethods(Object target, String beanName) throws Throwable {
-			Collection<LifecycleElement> destroyMethodsToIterate =
+			Collection<LifecycleElement> destroyMethodsToUse =
 					(this.checkedDestroyMethods != null ? this.checkedDestroyMethods : this.destroyMethods);
-			if (!destroyMethodsToIterate.isEmpty()) {
+			if (!destroyMethodsToUse.isEmpty()) {
 				boolean debug = logger.isDebugEnabled();
-				for (LifecycleElement element : destroyMethodsToIterate) {
+				for (LifecycleElement element : destroyMethodsToUse) {
 					if (debug) {
 						logger.debug("Invoking destroy method on bean '" + beanName + "': " + element.getMethod());
 					}
@@ -320,6 +325,12 @@ public class InitDestroyAnnotationBeanPostProcessor
 				}
 			}
 		}
+
+		public boolean hasDestroyMethods() {
+			Collection<LifecycleElement> destroyMethodsToUse =
+					(this.checkedDestroyMethods != null ? this.checkedDestroyMethods : this.destroyMethods);
+			return !destroyMethodsToUse.isEmpty();
+		}
 	}
 
 

spring-beans/src/main/java/org/springframework/beans/factory/config/DestructionAwareBeanPostProcessor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 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.
@@ -43,4 +43,24 @@ public interface DestructionAwareBeanPostProcessor extends BeanPostProcessor {
 	 */
 	void postProcessBeforeDestruction(Object bean, String beanName) throws BeansException;
 
+	/**
+	 * Determine whether the given bean instance requires destruction by this
+	 * post-processor.
+	 * <p><b>NOTE:</b> Even as a late addition, this method has been introduced on
+	 * {@code DestructionAwareBeanPostProcessor} itself instead of on a SmartDABPP
+	 * subinterface. This allows existing {@code DestructionAwareBeanPostProcessor}
+	 * implementations to easily provide {@code requiresDestruction} logic while
+	 * retaining compatibility with Spring <4.3, and it is also an easier onramp to
+	 * declaring {@code requiresDestruction} as a Java 8 default method in Spring 5.
+	 * <p>If an implementation of {@code DestructionAwareBeanPostProcessor} does
+	 * not provide a concrete implementation of this method, Spring's invocation
+	 * mechanism silently assumes a method returning {@code true} (the effective
+	 * default before 4.3, and the to-be-default in the Java 8 method in Spring 5).
+	 * @param bean the bean instance to check
+	 * @return {@code true} if {@link #postProcessBeforeDestruction} is supposed to
+	 * be called for this bean instance eventually, or {@code false} if not needed
+	 * @since 4.3
+	 */
+	boolean requiresDestruction(Object bean);
+
 }

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

@@ -1609,7 +1609,8 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp
 	 */
 	protected boolean requiresDestruction(Object bean, RootBeanDefinition mbd) {
 		return (bean != null &&
-				(DisposableBeanAdapter.hasDestroyMethod(bean, mbd) || hasDestructionAwareBeanPostProcessors()));
+				(DisposableBeanAdapter.hasDestroyMethod(bean, mbd) || (hasDestructionAwareBeanPostProcessors() &&
+						DisposableBeanAdapter.hasApplicableProcessors(bean, getBeanPostProcessors()))));
 	}
 
 	/**

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

@@ -139,7 +139,7 @@ class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable {
 				}
 			}
 		}
-		this.beanPostProcessors = filterPostProcessors(postProcessors);
+		this.beanPostProcessors = filterPostProcessors(postProcessors, bean);
 	}
 
 	/**
@@ -155,7 +155,7 @@ class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable {
 		this.invokeDisposableBean = (this.bean instanceof DisposableBean);
 		this.nonPublicAccessAllowed = true;
 		this.acc = acc;
-		this.beanPostProcessors = filterPostProcessors(postProcessors);
+		this.beanPostProcessors = filterPostProcessors(postProcessors, bean);
 	}
 
 	/**
@@ -214,16 +214,26 @@ class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable {
 
 	/**
 	 * Search for all DestructionAwareBeanPostProcessors in the List.
-	 * @param postProcessors the List to search
+	 * @param processors the List to search
 	 * @return the filtered List of DestructionAwareBeanPostProcessors
 	 */
-	private List<DestructionAwareBeanPostProcessor> filterPostProcessors(List<BeanPostProcessor> postProcessors) {
+	private List<DestructionAwareBeanPostProcessor> filterPostProcessors(List<BeanPostProcessor> processors, Object bean) {
 		List<DestructionAwareBeanPostProcessor> filteredPostProcessors = null;
-		if (!CollectionUtils.isEmpty(postProcessors)) {
-			filteredPostProcessors = new ArrayList<DestructionAwareBeanPostProcessor>(postProcessors.size());
-			for (BeanPostProcessor postProcessor : postProcessors) {
-				if (postProcessor instanceof DestructionAwareBeanPostProcessor) {
-					filteredPostProcessors.add((DestructionAwareBeanPostProcessor) postProcessor);
+		if (!CollectionUtils.isEmpty(processors)) {
+			filteredPostProcessors = new ArrayList<DestructionAwareBeanPostProcessor>(processors.size());
+			for (BeanPostProcessor processor : processors) {
+				if (processor instanceof DestructionAwareBeanPostProcessor) {
+					DestructionAwareBeanPostProcessor dabpp = (DestructionAwareBeanPostProcessor) processor;
+					try {
+						if (dabpp.requiresDestruction(bean)) {
+							filteredPostProcessors.add(dabpp);
+						}
+					}
+					catch (AbstractMethodError err) {
+						// A pre-4.3 third-party DestructionAwareBeanPostProcessor...
+						// As of 5.0, we can let requiresDestruction be a Java 8 default method which returns true.
+						filteredPostProcessors.add(dabpp);
+					}
 				}
 			}
 		}
@@ -401,9 +411,36 @@ class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable {
 		}
 		String destroyMethodName = beanDefinition.getDestroyMethodName();
 		if (AbstractBeanDefinition.INFER_METHOD.equals(destroyMethodName)) {
-			return ClassUtils.hasMethod(bean.getClass(), CLOSE_METHOD_NAME);
+			return (ClassUtils.hasMethod(bean.getClass(), CLOSE_METHOD_NAME) ||
+					ClassUtils.hasMethod(bean.getClass(), SHUTDOWN_METHOD_NAME));
 		}
 		return StringUtils.hasLength(destroyMethodName);
 	}
 
+	/**
+	 * Check whether the given bean has destruction-aware post-processors applying to it.
+	 * @param bean the bean instance
+	 * @param postProcessors the post-processor candidates
+	 */
+	public static boolean hasApplicableProcessors(Object bean, List<BeanPostProcessor> postProcessors) {
+		if (!CollectionUtils.isEmpty(postProcessors)) {
+			for (BeanPostProcessor processor : postProcessors) {
+				if (processor instanceof DestructionAwareBeanPostProcessor) {
+					DestructionAwareBeanPostProcessor dabpp = (DestructionAwareBeanPostProcessor) processor;
+					try {
+						if (dabpp.requiresDestruction(bean)) {
+							return true;
+						}
+					}
+					catch (AbstractMethodError err) {
+						// A pre-4.3 third-party DestructionAwareBeanPostProcessor...
+						// As of 5.0, we can let requiresDestruction be a Java 8 default method which returns true.
+						return true;
+					}
+				}
+			}
+		}
+		return false;
+	}
+
 }

spring-context/src/main/java/org/springframework/context/support/PostProcessorRegistrationDelegate.java

@@ -347,7 +347,8 @@ class PostProcessorRegistrationDelegate {
 	 * BeanPostProcessor that detects beans which implement the ApplicationListener interface.
 	 * This catches beans that can't reliably be detected by getBeanNamesForType.
 	 */
-	private static class ApplicationListenerDetector implements MergedBeanDefinitionPostProcessor, DestructionAwareBeanPostProcessor {
+	private static class ApplicationListenerDetector
+			implements DestructionAwareBeanPostProcessor, MergedBeanDefinitionPostProcessor {
 
 		private static final Log logger = LogFactory.getLog(ApplicationListenerDetector.class);
 
@@ -402,6 +403,11 @@ class PostProcessorRegistrationDelegate {
 				multicaster.removeApplicationListenerBean(beanName);
 			}
 		}
+
+		@Override
+		public boolean requiresDestruction(Object bean) {
+			return (bean instanceof ApplicationListener);
+		}
 	}
 
 }

spring-context/src/test/java/org/springframework/context/annotation/CommonAnnotationBeanPostProcessorTests.java

@@ -560,6 +560,11 @@ public class CommonAnnotationBeanPostProcessorTests {
 				assertFalse(((AnnotatedInitDestroyBean) bean).destroyCalled);
 			}
 		}
+
+		@Override
+		public boolean requiresDestruction(Object bean) {
+			return true;
+		}
 	}
 
 

spring-context/src/test/java/org/springframework/context/annotation/configuration/ScopingTests.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.
@@ -31,9 +31,9 @@ import org.springframework.aop.scope.ScopedObject;
 import org.springframework.beans.factory.ObjectFactory;
 import org.springframework.beans.factory.support.DefaultListableBeanFactory;
 import org.springframework.beans.factory.support.RootBeanDefinition;
+import org.springframework.context.annotation.AnnotationConfigApplicationContext;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
-import org.springframework.context.annotation.ConfigurationClassPostProcessor;
 import org.springframework.context.annotation.Scope;
 import org.springframework.context.annotation.ScopedProxyMode;
 import org.springframework.context.support.GenericApplicationContext;
@@ -79,8 +79,7 @@ public class ScopingTests {
 			beanFactory.registerScope(SCOPE, customScope);
 		}
 		beanFactory.registerBeanDefinition("config", new RootBeanDefinition(configClass));
-		GenericApplicationContext ctx = new GenericApplicationContext(beanFactory);
-		ctx.addBeanFactoryPostProcessor(new ConfigurationClassPostProcessor());
+		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(beanFactory);
 		ctx.refresh();
 		return ctx;
 	}
@@ -222,13 +221,6 @@ public class ScopingTests {
 		assertSame(spouse.getName(), spouseFromBF.getName());
 	}
 
-	@Test
-	public void testScopedConfigurationBeanDefinitionCount() throws Exception {
-		// count the beans
-		// 6 @Beans + 1 Configuration + 2 scoped proxy + 1 importRegistry + 1 enhanced config post processor
-		assertEquals(11, ctx.getBeanDefinitionCount());
-	}
-
 
 	static class Foo {
 
@@ -365,7 +357,7 @@ public class ScopingTests {
 
 		@Override
 		public void registerDestructionCallback(String name, Runnable callback) {
-			// do nothing
+			throw new IllegalStateException("Not supposed to be called");
 		}
 
 		@Override

spring-orm/src/main/java/org/springframework/orm/jpa/support/PersistenceAnnotationBeanPostProcessor.java

@@ -375,6 +375,11 @@ public class PersistenceAnnotationBeanPostProcessor
 		EntityManagerFactoryUtils.closeEntityManager(emToClose);
 	}
 
+	@Override
+	public boolean requiresDestruction(Object bean) {
+		return this.extendedEntityManagersToClose.containsKey(bean);
+	}
+
 
 	private InjectionMetadata findPersistenceMetadata(String beanName, final Class<?> clazz, PropertyValues pvs) {
 		// Fall back to class name as cache key, for backwards compatibility with custom callers.