Invoke bean-derived (Auto)Closeable.close() method directly

Closes gh-27504

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

@@ -61,6 +61,8 @@ import org.springframework.util.StringUtils;
 @SuppressWarnings("serial")
 class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable {
 
+	private static final String DESTROY_METHOD_NAME = "destroy";
+
 	private static final String CLOSE_METHOD_NAME = "close";
 
 	private static final String SHUTDOWN_METHOD_NAME = "shutdown";
@@ -72,12 +74,11 @@ class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable {
 
 	private final String beanName;
 
-	private final boolean invokeDisposableBean;
-
 	private final boolean nonPublicAccessAllowed;
 
-	@Nullable
-	private final AccessControlContext acc;
+	private final boolean invokeDisposableBean;
+
+	private boolean invokeAutoCloseable;
 
 	@Nullable
 	private String destroyMethodName;
@@ -88,6 +89,9 @@ class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable {
 	@Nullable
 	private final List<DestructionAwareBeanPostProcessor> beanPostProcessors;
 
+	@Nullable
+	private final AccessControlContext acc;
+
 
 	/**
 	 * Create a new DisposableBeanAdapter for the given bean.
@@ -103,38 +107,45 @@ class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable {
 		Assert.notNull(bean, "Disposable bean must not be null");
 		this.bean = bean;
 		this.beanName = beanName;
-		this.invokeDisposableBean =
-				(this.bean instanceof DisposableBean && !beanDefinition.isExternallyManagedDestroyMethod("destroy"));
 		this.nonPublicAccessAllowed = beanDefinition.isNonPublicAccessAllowed();
-		this.acc = acc;
+		this.invokeDisposableBean = (bean instanceof DisposableBean &&
+				!beanDefinition.isExternallyManagedDestroyMethod(DESTROY_METHOD_NAME));
+
 		String destroyMethodName = inferDestroyMethodIfNecessary(bean, beanDefinition);
-		if (destroyMethodName != null && !(this.invokeDisposableBean && "destroy".equals(destroyMethodName)) &&
+		if (destroyMethodName != null &&
+				!(this.invokeDisposableBean && DESTROY_METHOD_NAME.equals(destroyMethodName)) &&
 				!beanDefinition.isExternallyManagedDestroyMethod(destroyMethodName)) {
-			this.destroyMethodName = destroyMethodName;
-			Method destroyMethod = determineDestroyMethod(destroyMethodName);
-			if (destroyMethod == null) {
-				if (beanDefinition.isEnforceDestroyMethod()) {
-					throw new BeanDefinitionValidationException("Could not find a destroy method named '" +
-							destroyMethodName + "' on bean with name '" + beanName + "'");
-				}
-			}
-			else {
-				if (destroyMethod.getParameterCount() > 0) {
-					Class<?>[] paramTypes = destroyMethod.getParameterTypes();
-					if (paramTypes.length > 1) {
-						throw new BeanDefinitionValidationException("Method '" + destroyMethodName + "' of bean '" +
-								beanName + "' has more than one parameter - not supported as destroy method");
+
+			this.invokeAutoCloseable = (bean instanceof AutoCloseable && CLOSE_METHOD_NAME.equals(destroyMethodName));
+			if (!this.invokeAutoCloseable) {
+				this.destroyMethodName = destroyMethodName;
+				Method destroyMethod = determineDestroyMethod(destroyMethodName);
+				if (destroyMethod == null) {
+					if (beanDefinition.isEnforceDestroyMethod()) {
+						throw new BeanDefinitionValidationException("Could not find a destroy method named '" +
+								destroyMethodName + "' on bean with name '" + beanName + "'");
 					}
-					else if (paramTypes.length == 1 && boolean.class != paramTypes[0]) {
-						throw new BeanDefinitionValidationException("Method '" + destroyMethodName + "' of bean '" +
-								beanName + "' has a non-boolean parameter - not supported as destroy method");
+				}
+				else {
+					if (destroyMethod.getParameterCount() > 0) {
+						Class<?>[] paramTypes = destroyMethod.getParameterTypes();
+						if (paramTypes.length > 1) {
+							throw new BeanDefinitionValidationException("Method '" + destroyMethodName + "' of bean '" +
+									beanName + "' has more than one parameter - not supported as destroy method");
+						}
+						else if (paramTypes.length == 1 && boolean.class != paramTypes[0]) {
+							throw new BeanDefinitionValidationException("Method '" + destroyMethodName + "' of bean '" +
+									beanName + "' has a non-boolean parameter - not supported as destroy method");
+						}
 					}
+					destroyMethod = ClassUtils.getInterfaceMethodIfPossible(destroyMethod);
 				}
-				destroyMethod = ClassUtils.getInterfaceMethodIfPossible(destroyMethod);
+				this.destroyMethod = destroyMethod;
 			}
-			this.destroyMethod = destroyMethod;
 		}
+
 		this.beanPostProcessors = filterPostProcessors(postProcessors, bean);
+		this.acc = acc;
 	}
 
 	/**
@@ -149,26 +160,27 @@ class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable {
 		Assert.notNull(bean, "Disposable bean must not be null");
 		this.bean = bean;
 		this.beanName = bean.getClass().getName();
-		this.invokeDisposableBean = (this.bean instanceof DisposableBean);
 		this.nonPublicAccessAllowed = true;
-		this.acc = acc;
+		this.invokeDisposableBean = (this.bean instanceof DisposableBean);
 		this.beanPostProcessors = filterPostProcessors(postProcessors, bean);
+		this.acc = acc;
 	}
 
 	/**
 	 * Create a new DisposableBeanAdapter for the given bean.
 	 */
-	private DisposableBeanAdapter(Object bean, String beanName, boolean invokeDisposableBean,
-			boolean nonPublicAccessAllowed, @Nullable String destroyMethodName,
+	private DisposableBeanAdapter(Object bean, String beanName, boolean nonPublicAccessAllowed,
+			boolean invokeDisposableBean, boolean invokeAutoCloseable, @Nullable String destroyMethodName,
 			@Nullable List<DestructionAwareBeanPostProcessor> postProcessors) {
 
 		this.bean = bean;
 		this.beanName = beanName;
-		this.invokeDisposableBean = invokeDisposableBean;
 		this.nonPublicAccessAllowed = nonPublicAccessAllowed;
-		this.acc = null;
+		this.invokeDisposableBean = invokeDisposableBean;
+		this.invokeAutoCloseable = invokeAutoCloseable;
 		this.destroyMethodName = destroyMethodName;
 		this.beanPostProcessors = postProcessors;
+		this.acc = null;
 	}
 
 
@@ -211,7 +223,32 @@ class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable {
 			}
 		}
 
-		if (this.destroyMethod != null) {
+		if (this.invokeAutoCloseable) {
+			if (logger.isTraceEnabled()) {
+				logger.trace("Invoking close() on bean with name '" + this.beanName + "'");
+			}
+			try {
+				if (System.getSecurityManager() != null) {
+					AccessController.doPrivileged((PrivilegedExceptionAction<Object>) () -> {
+						((AutoCloseable) this.bean).close();
+						return null;
+					}, this.acc);
+				}
+				else {
+					((AutoCloseable) this.bean).close();
+				}
+			}
+			catch (Throwable ex) {
+				String msg = "Invocation of close method failed on bean with name '" + this.beanName + "'";
+				if (logger.isDebugEnabled()) {
+					logger.warn(msg, ex);
+				}
+				else {
+					logger.warn(msg + ": " + ex);
+				}
+			}
+		}
+		else if (this.destroyMethod != null) {
 			invokeCustomDestroyMethod(this.destroyMethod);
 		}
 		else if (this.destroyMethodName != null) {
@@ -259,7 +296,7 @@ class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable {
 			args[0] = Boolean.TRUE;
 		}
 		if (logger.isTraceEnabled()) {
-			logger.trace("Invoking destroy method '" + this.destroyMethodName +
+			logger.trace("Invoking custom destroy method '" + this.destroyMethodName +
 					"' on bean with name '" + this.beanName + "'");
 		}
 		try {
@@ -282,7 +319,7 @@ class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable {
 			}
 		}
 		catch (InvocationTargetException ex) {
-			String msg = "Destroy method '" + this.destroyMethodName + "' on bean with name '" +
+			String msg = "Custom destroy method '" + this.destroyMethodName + "' on bean with name '" +
 					this.beanName + "' threw an exception";
 			if (logger.isDebugEnabled()) {
 				logger.warn(msg, ex.getTargetException());
@@ -292,7 +329,7 @@ class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable {
 			}
 		}
 		catch (Throwable ex) {
-			logger.warn("Failed to invoke destroy method '" + this.destroyMethodName +
+			logger.warn("Failed to invoke custom destroy method '" + this.destroyMethodName +
 					"' on bean with name '" + this.beanName + "'", ex);
 		}
 	}
@@ -312,8 +349,9 @@ class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable {
 				}
 			}
 		}
-		return new DisposableBeanAdapter(this.bean, this.beanName, this.invokeDisposableBean,
-				this.nonPublicAccessAllowed, this.destroyMethodName, serializablePostProcessors);
+		return new DisposableBeanAdapter(
+				this.bean, this.beanName, this.nonPublicAccessAllowed, this.invokeDisposableBean,
+				this.invokeAutoCloseable, this.destroyMethodName, serializablePostProcessors);
 	}
 
 
@@ -323,10 +361,7 @@ class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable {
 	 * @param beanDefinition the corresponding bean definition
 	 */
 	public static boolean hasDestroyMethod(Object bean, RootBeanDefinition beanDefinition) {
-		if (bean instanceof DisposableBean || bean instanceof AutoCloseable) {
-			return true;
-		}
-		return inferDestroyMethodIfNecessary(bean, beanDefinition) != null;
+		return (bean instanceof DisposableBean || inferDestroyMethodIfNecessary(bean, beanDefinition) != null);
 	}
 
 
@@ -348,21 +383,27 @@ class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable {
 		String destroyMethodName = beanDefinition.resolvedDestroyMethodName;
 		if (destroyMethodName == null) {
 			destroyMethodName = beanDefinition.getDestroyMethodName();
+			boolean autoCloseable = (bean instanceof AutoCloseable);
 			if (AbstractBeanDefinition.INFER_METHOD.equals(destroyMethodName) ||
-					(destroyMethodName == null && bean instanceof AutoCloseable)) {
-				// Only perform destroy method inference or Closeable detection
-				// in case of the bean not explicitly implementing DisposableBean
+					(destroyMethodName == null && autoCloseable)) {
+				// Only perform destroy method inference in case of the bean
+				// not explicitly implementing the DisposableBean interface
 				destroyMethodName = null;
 				if (!(bean instanceof DisposableBean)) {
-					try {
-						destroyMethodName = bean.getClass().getMethod(CLOSE_METHOD_NAME).getName();
+					if (autoCloseable) {
+						destroyMethodName = CLOSE_METHOD_NAME;
 					}
-					catch (NoSuchMethodException ex) {
+					else {
 						try {
-							destroyMethodName = bean.getClass().getMethod(SHUTDOWN_METHOD_NAME).getName();
+							destroyMethodName = bean.getClass().getMethod(CLOSE_METHOD_NAME).getName();
 						}
-						catch (NoSuchMethodException ex2) {
-							// no candidate destroy method found
+						catch (NoSuchMethodException ex) {
+							try {
+								destroyMethodName = bean.getClass().getMethod(SHUTDOWN_METHOD_NAME).getName();
+							}
+							catch (NoSuchMethodException ex2) {
+								// no candidate destroy method found
+							}
 						}
 					}
 				}

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2021 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.
@@ -26,7 +26,6 @@ import org.springframework.context.support.GenericXmlApplicationContext;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
-
 /**
  * @author Chris Beams
  * @author Juergen Hoeller
@@ -47,6 +46,7 @@ public class DestroyMethodInferenceTests {
 		WithLocalShutdownMethod c7 = ctx.getBean("c7", WithLocalShutdownMethod.class);
 		WithInheritedCloseMethod c8 = ctx.getBean("c8", WithInheritedCloseMethod.class);
 		WithDisposableBean c9 = ctx.getBean("c9", WithDisposableBean.class);
+		WithAutoCloseable c10 = ctx.getBean("c10", WithAutoCloseable.class);
 
 		assertThat(c0.closed).as("c0").isFalse();
 		assertThat(c1.closed).as("c1").isFalse();
@@ -58,6 +58,8 @@ public class DestroyMethodInferenceTests {
 		assertThat(c7.closed).as("c7").isFalse();
 		assertThat(c8.closed).as("c8").isFalse();
 		assertThat(c9.closed).as("c9").isFalse();
+		assertThat(c10.closed).as("c10").isFalse();
+
 		ctx.close();
 		assertThat(c0.closed).as("c0").isTrue();
 		assertThat(c1.closed).as("c1").isTrue();
@@ -69,6 +71,7 @@ public class DestroyMethodInferenceTests {
 		assertThat(c7.closed).as("c7").isTrue();
 		assertThat(c8.closed).as("c8").isFalse();
 		assertThat(c9.closed).as("c9").isTrue();
+		assertThat(c10.closed).as("c10").isTrue();
 	}
 
 	@Test
@@ -80,21 +83,29 @@ public class DestroyMethodInferenceTests {
 		WithLocalCloseMethod x3 = ctx.getBean("x3", WithLocalCloseMethod.class);
 		WithNoCloseMethod x4 = ctx.getBean("x4", WithNoCloseMethod.class);
 		WithInheritedCloseMethod x8 = ctx.getBean("x8", WithInheritedCloseMethod.class);
+		WithDisposableBean x9 = ctx.getBean("x9", WithDisposableBean.class);
+		WithAutoCloseable x10 = ctx.getBean("x10", WithAutoCloseable.class);
 
 		assertThat(x1.closed).isFalse();
 		assertThat(x2.closed).isFalse();
 		assertThat(x3.closed).isFalse();
 		assertThat(x4.closed).isFalse();
+		assertThat(x8.closed).isFalse();
+		assertThat(x9.closed).isFalse();
+		assertThat(x10.closed).isFalse();
+
 		ctx.close();
 		assertThat(x1.closed).isFalse();
 		assertThat(x2.closed).isTrue();
 		assertThat(x3.closed).isTrue();
 		assertThat(x4.closed).isFalse();
 		assertThat(x8.closed).isFalse();
+		assertThat(x9.closed).isTrue();
+		assertThat(x10.closed).isTrue();
 	}
 
 
-	@Configuration
+	@Configuration(proxyBeanMethods = false)
 	static class Config {
 
 		@Bean(destroyMethod = "explicitClose")
@@ -155,6 +166,11 @@ public class DestroyMethodInferenceTests {
 		public WithDisposableBean c9() {
 			return new WithDisposableBean();
 		}
+
+		@Bean
+		public WithAutoCloseable c10() {
+			return new WithAutoCloseable();
+		}
 	}
 
 
@@ -189,28 +205,39 @@ public class DestroyMethodInferenceTests {
 	}
 
 
-	static class WithDisposableBean implements DisposableBean {
+	static class WithNoCloseMethod {
 
 		boolean closed = false;
+	}
 
-		@Override
-		public void destroy() {
+
+	static class WithLocalShutdownMethod {
+
+		boolean closed = false;
+
+		public void shutdown() {
 			closed = true;
 		}
 	}
 
 
-	static class WithNoCloseMethod {
+	static class WithDisposableBean implements DisposableBean {
 
 		boolean closed = false;
+
+		@Override
+		public void destroy() {
+			closed = true;
+		}
 	}
 
 
-	static class WithLocalShutdownMethod {
+	static class WithAutoCloseable implements AutoCloseable {
 
 		boolean closed = false;
 
-		public void shutdown() {
+		@Override
+		public void close() {
 			closed = true;
 		}
 	}

spring-context/src/test/resources/org/springframework/context/annotation/DestroyMethodInferenceTests-context.xml

@@ -15,10 +15,16 @@
 		  class="org.springframework.context.annotation.DestroyMethodInferenceTests.WithInheritedCloseMethod"
 		  destroy-method=""/>
 
+	<bean id="x9"
+		  class="org.springframework.context.annotation.DestroyMethodInferenceTests.WithDisposableBean"
+		  destroy-method=""/>
+
+	<bean id="x10"
+		  class="org.springframework.context.annotation.DestroyMethodInferenceTests.WithAutoCloseable"/>
+
 	<beans default-destroy-method="(inferred)">
 		<bean id="x3"
 		      class="org.springframework.context.annotation.DestroyMethodInferenceTests$WithLocalCloseMethod"/>
-
 		<bean id="x4"
 		      class="org.springframework.context.annotation.DestroyMethodInferenceTests$WithNoCloseMethod"/>
 	</beans>