Stop ActiveMQ pooled connection factory when context is closed

Previously, ActiveMQ's pooled connection factory was not closed as
part of the application context being closed. This would leave
non-daemon threads running which could cause shutdown to hang unless
the JVM itself was shutting down (in which case a shutdown hook would
stop the pool).

This commit configures each pooled connection factory bean with a
custom destroy method so that the pool is stopped as part of the
application context being closed. To allow the destroy method to only
be declared when the connection factory is pooled, the bean method
has been split into two; one for pooled and one for non-pooled. This
is a partial backport of the changes made in bedf2ed.

Closes gh-4748

Author: wilkinsona

spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jms/activemq/ActiveMQConnectionFactoryConfiguration.java

@@ -16,18 +16,16 @@
 
 package org.springframework.boot.autoconfigure.jms.activemq;
 
-import java.lang.reflect.Method;
-
 import javax.jms.ConnectionFactory;
 
 import org.apache.activemq.ActiveMQConnectionFactory;
 import org.apache.activemq.pool.PooledConnectionFactory;
 
+import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
+import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
-import org.springframework.util.Assert;
-import org.springframework.util.ReflectionUtils;
 
 /**
  * Configuration for ActiveMQ {@link ConnectionFactory}.
@@ -43,31 +41,24 @@
 class ActiveMQConnectionFactoryConfiguration {
 
 	@Bean
-	public ConnectionFactory jmsConnectionFactory(ActiveMQProperties properties) {
-		ActiveMQConnectionFactory connectionFactory = new ActiveMQConnectionFactoryFactory(
-				properties).createConnectionFactory(ActiveMQConnectionFactory.class);
-		if (properties.isPooled()) {
-			PooledConnectionFactory pool = new PooledConnectionFactory();
-			Method setConnectionFactory = findConnectionFactorySetter();
-			Assert.state(setConnectionFactory != null,
-					"No supported " + "setConnectionFactory method was found");
-			ReflectionUtils.invokeMethod(setConnectionFactory, pool, connectionFactory);
-			return pool;
-		}
-		return connectionFactory;
+	@ConditionalOnProperty(prefix = "spring.activemq", name = "pooled", havingValue = "false", matchIfMissing = true)
+	public ActiveMQConnectionFactory jmsConnectionFactory(ActiveMQProperties properties) {
+		return new ActiveMQConnectionFactoryFactory(properties)
+				.createConnectionFactory(ActiveMQConnectionFactory.class);
 	}
 
-	private Method findConnectionFactorySetter() {
-		Method setter = findConnectionFactorySetter(ConnectionFactory.class);
-		if (setter == null) {
-			setter = findConnectionFactorySetter(Object.class);
+	@ConditionalOnClass(PooledConnectionFactory.class)
+	static class PooledConnectionFactoryConfiguration {
+
+		@Bean(destroyMethod = "stop")
+		@ConditionalOnProperty(prefix = "spring.activemq", name = "pooled", havingValue = "true", matchIfMissing = false)
+		public PooledConnectionFactory pooledJmsConnectionFactory(
+				ActiveMQProperties properties) {
+			return new PooledConnectionFactory(
+					new ActiveMQConnectionFactoryFactory(properties)
+							.createConnectionFactory(ActiveMQConnectionFactory.class));
 		}
-		return setter;
-	}
 
-	private Method findConnectionFactorySetter(Class<?> param) {
-		return ReflectionUtils.findMethod(PooledConnectionFactory.class,
-				"setConnectionFactory", param);
 	}
 
 }

spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jms/activemq/ActiveMQXAConnectionFactoryConfiguration.java

@@ -26,6 +26,7 @@
 import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
+import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.boot.jta.XAConnectionFactoryWrapper;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
@@ -53,15 +54,25 @@ public ConnectionFactory jmsConnectionFactory(ActiveMQProperties properties,
 	}
 
 	@Bean
-	public ConnectionFactory nonXaJmsConnectionFactory(ActiveMQProperties properties) {
-		ActiveMQConnectionFactory connectionFactory = new ActiveMQConnectionFactoryFactory(
-				properties).createConnectionFactory(ActiveMQConnectionFactory.class);
-		if (properties.isPooled()) {
-			PooledConnectionFactory pool = new PooledConnectionFactory();
-			pool.setConnectionFactory(connectionFactory);
-			return pool;
+	@ConditionalOnProperty(prefix = "spring.activemq", name = "pooled", havingValue = "false", matchIfMissing = true)
+	public ActiveMQConnectionFactory nonXaJmsConnectionFactory(
+			ActiveMQProperties properties) {
+		return new ActiveMQConnectionFactoryFactory(properties)
+				.createConnectionFactory(ActiveMQConnectionFactory.class);
+	}
+
+	@ConditionalOnClass(PooledConnectionFactory.class)
+	@ConditionalOnProperty(prefix = "spring.activemq", name = "pooled", havingValue = "true", matchIfMissing = false)
+	static class PooledConnectionFactoryConfiguration {
+
+		@Bean(destroyMethod = "stop")
+		public PooledConnectionFactory pooledNonXaJmsConnectionFactory(
+				ActiveMQProperties properties) {
+			return new PooledConnectionFactory(
+					new ActiveMQConnectionFactoryFactory(properties)
+							.createConnectionFactory(ActiveMQConnectionFactory.class));
 		}
-		return connectionFactory;
+
 	}
 
 }

spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jms/activemq/ActiveMQAutoConfigurationTests.java

@@ -17,6 +17,7 @@
 package org.springframework.boot.autoconfigure.jms.activemq;
 
 import javax.jms.ConnectionFactory;
+import javax.jms.JMSException;
 
 import org.apache.activemq.ActiveMQConnectionFactory;
 import org.apache.activemq.pool.PooledConnectionFactory;
@@ -29,6 +30,8 @@
 import org.springframework.context.annotation.Configuration;
 
 import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.nullValue;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
@@ -62,11 +65,14 @@ public void configurationBacksOffWhenCustomConnectionFactoryExists() {
 	}
 
 	@Test
-	public void pooledConnectionFactoryConfiguration() {
+	public void pooledConnectionFactoryConfiguration() throws JMSException {
 		load(EmptyConfiguration.class, "spring.activemq.pooled:true");
 		ConnectionFactory connectionFactory = this.context
 				.getBean(ConnectionFactory.class);
 		assertThat(connectionFactory, instanceOf(PooledConnectionFactory.class));
+		this.context.close();
+		assertThat(((PooledConnectionFactory) connectionFactory).createConnection(),
+				is(nullValue()));
 	}
 
 	private void load(Class<?> config, String... environment) {