INT-4486: Properly implement stop(Runnable)

artembilan · garyrussell · commit 1ce656e4fe12 · 2018-06-11T17:52:23.000-04:00
JIRA: https://jira.spring.io/browse/INT-4486 The `SmartLifecycle.stop(Runnable callback)` must always call the `callback` in the end independently of the internal state * Revise all the `SmartLifecycle` implementations for the proper `callback` handling **Cherry-pick to 5.0.x and 4.3.x**
diff --git a/spring-integration-amqp/src/main/java/org/springframework/integration/amqp/channel/AbstractSubscribableAmqpChannel.java b/spring-integration-amqp/src/main/java/org/springframework/integration/amqp/channel/AbstractSubscribableAmqpChannel.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -234,6 +234,9 @@ public void stop(Runnable callback) {
 			this.container.stop(callback);
 			this.declared = false;
 		}
+		else {
+			callback.run();
+		}
 	}
 
 	@Override
diff --git a/spring-integration-amqp/src/main/java/org/springframework/integration/amqp/config/AmqpChannelFactoryBean.java b/spring-integration-amqp/src/main/java/org/springframework/integration/amqp/config/AmqpChannelFactoryBean.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -66,10 +66,11 @@
  * @author Mark Fisher
  * @author Gary Russell
  * @author Artem Bilan
+ *
  * @since 2.1
  */
-public class AmqpChannelFactoryBean extends AbstractFactoryBean<AbstractAmqpChannel> implements SmartLifecycle,
-		DisposableBean, BeanNameAware {
+public class AmqpChannelFactoryBean extends AbstractFactoryBean<AbstractAmqpChannel>
+		implements SmartLifecycle, DisposableBean, BeanNameAware {
 
 	private volatile AbstractAmqpChannel channel;
 
@@ -516,13 +517,14 @@ public void stop(Runnable callback) {
 		if (this.channel instanceof SmartLifecycle) {
 			((SmartLifecycle) this.channel).stop(callback);
 		}
+		else {
+			callback.run();
+		}
 	}
 
 	@Override
 	protected void destroyInstance(AbstractAmqpChannel instance) throws Exception {
-		if (instance instanceof DisposableBean) {
-			((DisposableBean) this.channel).destroy();
-		}
+		this.channel.destroy();
 	}
 
 }
diff --git a/spring-integration-core/src/main/java/org/springframework/integration/config/ConsumerEndpointFactoryBean.java b/spring-integration-core/src/main/java/org/springframework/integration/config/ConsumerEndpointFactoryBean.java
@@ -362,6 +362,9 @@ public void stop(Runnable callback) {
 		if (this.endpoint != null) {
 			this.endpoint.stop(callback);
 		}
+		else {
+			callback.run();
+		}
 	}
 
 	@Override
diff --git a/spring-integration-core/src/main/java/org/springframework/integration/config/SourcePollingChannelAdapterFactoryBean.java b/spring-integration-core/src/main/java/org/springframework/integration/config/SourcePollingChannelAdapterFactoryBean.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -247,6 +247,9 @@ public void stop(Runnable callback) {
 		if (this.adapter != null) {
 			this.adapter.stop(callback);
 		}
+		else {
+			callback.run();
+		}
 	}
 
 	@Override
diff --git a/spring-integration-core/src/main/java/org/springframework/integration/dsl/IntegrationFlowAdapter.java b/spring-integration-core/src/main/java/org/springframework/integration/dsl/IntegrationFlowAdapter.java
@@ -95,6 +95,9 @@ public void stop(Runnable callback) {
 		if (this.running.getAndSet(false)) {
 			this.targetIntegrationFlow.stop(callback);
 		}
+		else {
+			callback.run();
+		}
 	}
 
 	@Override
diff --git a/spring-integration-core/src/main/java/org/springframework/integration/endpoint/AbstractEndpoint.java b/spring-integration-core/src/main/java/org/springframework/integration/endpoint/AbstractEndpoint.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -199,6 +199,9 @@ public final void stop(Runnable callback) {
 					logger.info("stopped " + this);
 				}
 			}
+			else {
+				callback.run();
+			}
 		}
 		finally {
 			this.lifecycleLock.unlock();
diff --git a/spring-integration-file/src/main/java/org/springframework/integration/file/config/FileTailInboundChannelAdapterFactoryBean.java b/spring-integration-file/src/main/java/org/springframework/integration/file/config/FileTailInboundChannelAdapterFactoryBean.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -36,6 +36,7 @@
  * @author Gary Russell
  * @author Artem Bilan
  * @author Ali Shahbour
+ *
  * @since 3.0
  *
  */
@@ -192,6 +193,9 @@ public void stop(Runnable callback) {
 		if (this.adapter != null) {
 			this.adapter.stop(callback);
 		}
+		else {
+			callback.run();
+		}
 	}
 
 	@Override
diff --git a/spring-integration-jms/src/main/java/org/springframework/integration/jms/SubscribableJmsChannel.java b/spring-integration-jms/src/main/java/org/springframework/integration/jms/SubscribableJmsChannel.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2018 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,9 +43,12 @@
 /**
  * @author Mark Fisher
  * @author Gary Russell
+ * @author Artem Bilan
+ *
  * @since 2.0
  */
-public class SubscribableJmsChannel extends AbstractJmsChannel implements SubscribableChannel, SmartLifecycle, DisposableBean {
+public class SubscribableJmsChannel extends AbstractJmsChannel
+		implements SubscribableChannel, SmartLifecycle, DisposableBean {
 
 	private final AbstractMessageListenerContainer container;
 
@@ -72,13 +75,15 @@ public void setMaxSubscribers(int maxSubscribers) {
 
 	@Override
 	public boolean subscribe(MessageHandler handler) {
-		Assert.state(this.dispatcher != null, "'MessageDispatcher' must not be null. This channel might not have been initialized");
+		Assert.state(this.dispatcher != null,
+				"'MessageDispatcher' must not be null. This channel might not have been initialized");
 		return this.dispatcher.addHandler(handler);
 	}
 
 	@Override
 	public boolean unsubscribe(MessageHandler handler) {
-		Assert.state(this.dispatcher != null, "'MessageDispatcher' must not be null. This channel might not have been initialized");
+		Assert.state(this.dispatcher != null,
+				"'MessageDispatcher' must not be null. This channel might not have been initialized");
 		return this.dispatcher.removeHandler(handler);
 	}
 
@@ -126,7 +131,7 @@ private void configureDispatcher(boolean isPubSub) {
 
 	@Override
 	public boolean isAutoStartup() {
-		return (this.container != null) ? this.container.isAutoStartup() : false;
+		return (this.container != null) && this.container.isAutoStartup();
 	}
 
 	@Override
@@ -136,7 +141,7 @@ public int getPhase() {
 
 	@Override
 	public boolean isRunning() {
-		return (this.container != null) ? this.container.isRunning() : false;
+		return (this.container != null) && this.container.isRunning();
 	}
 
 	@Override
@@ -158,6 +163,9 @@ public void stop(Runnable callback) {
 		if (this.container != null) {
 			this.container.stop(callback);
 		}
+		else {
+			callback.run();
+		}
 	}
 
 	@Override
@@ -226,6 +234,7 @@ else if (this.logger.isWarnEnabled()) {
 				throw new MessagingException("failed to handle incoming JMS Message", e);
 			}
 		}
+
 	}
 
 }
diff --git a/spring-integration-jms/src/main/java/org/springframework/integration/jms/config/JmsChannelFactoryBean.java b/spring-integration-jms/src/main/java/org/springframework/integration/jms/config/JmsChannelFactoryBean.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -548,6 +548,9 @@ public void stop(Runnable callback) {
 		if (this.channel instanceof SubscribableJmsChannel) {
 			((SubscribableJmsChannel) this.channel).stop(callback);
 		}
+		else {
+			callback.run();
+		}
 	}
 
 	@Override
diff --git a/spring-integration-redis/src/main/java/org/springframework/integration/redis/channel/SubscribableRedisChannel.java b/spring-integration-redis/src/main/java/org/springframework/integration/redis/channel/SubscribableRedisChannel.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -51,6 +51,7 @@
  * @author Oleg Zhurakousky
  * @author Gary Russell
  * @author Artem Bilan
+ *
  * @since 2.0
  */
 @SuppressWarnings("rawtypes")
@@ -105,7 +106,6 @@ public void setSerializer(RedisSerializer<?> serializer) {
 	/**
 	 * Specify the maximum number of subscribers supported by the
 	 * channel's dispatcher.
-	 *
 	 * @param maxSubscribers The maximum number of subscribers allowed.
 	 */
 	public void setMaxSubscribers(int maxSubscribers) {
@@ -168,45 +168,37 @@ public void onInit() throws Exception {
 
 	@Override
 	public boolean isAutoStartup() {
-		return (this.container != null) && this.container.isAutoStartup();
+		return this.container.isAutoStartup();
 	}
 
 	@Override
 	public int getPhase() {
-		return (this.container != null) ? this.container.getPhase() : 0;
+		return this.container.getPhase();
 	}
 
 	@Override
 	public boolean isRunning() {
-		return (this.container != null) && this.container.isRunning();
+		return this.container.isRunning();
 	}
 
 	@Override
 	public void start() {
-		if (this.container != null) {
-			this.container.start();
-		}
+		this.container.start();
 	}
 
 	@Override
 	public void stop() {
-		if (this.container != null) {
-			this.container.stop();
-		}
+		this.container.stop();
 	}
 
 	@Override
 	public void stop(Runnable callback) {
-		if (this.container != null) {
-			this.container.stop(callback);
-		}
+		this.container.stop(callback);
 	}
 
 	@Override
 	public void destroy() throws Exception {
-		if (this.container != null) {
-			this.container.destroy();
-		}
+		this.container.destroy();
 	}
 
 	private class MessageListenerDelegate {
@@ -230,6 +222,7 @@ public void handleMessage(Object payload) {
 						+ "' (" + SubscribableRedisChannel.this.getFullChannelName() + ").", e);
 			}
 		}
+
 	}
 
 }
diff --git a/spring-integration-syslog/src/main/java/org/springframework/integration/syslog/config/SyslogReceivingChannelAdapterFactoryBean.java b/spring-integration-syslog/src/main/java/org/springframework/integration/syslog/config/SyslogReceivingChannelAdapterFactoryBean.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -32,7 +32,10 @@
 
 /**
  * Factory bean to create syslog inbound adapters (UDP or TCP).
+ *
  * @author Gary Russell
+ * @author Artem Bilan
+ *
  * @since 3.0
  *
  */
@@ -163,6 +166,9 @@ public void stop(Runnable callback) {
 		if (this.adapter != null) {
 			this.adapter.stop(callback);
 		}
+		else {
+			callback.run();
+		}
 	}
 
 	@Override

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`/*`
`2`		`- * Copyright 2002-2017 the original author or authors.`
	`2`	`+ * Copyright 2002-2018 the original author or authors.`
`3`	`3`	`*`
`4`	`4`	`* Licensed under the Apache License, Version 2.0 (the "License");`
`5`	`5`	`* you may not use this file except in compliance with the License.`
`@@ -234,6 +234,9 @@ public void stop(Runnable callback) {`
`234`	`234`	`this.container.stop(callback);`
`235`	`235`	`this.declared = false;`
`236`	`236`	`}`
	`237`	`+ else {`
	`238`	`+ callback.run();`
	`239`	`+ }`
`237`	`240`	`}`
`238`	`241`
`239`	`242`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -362,6 +362,9 @@ public void stop(Runnable callback) {`
`362`	`362`	`if (this.endpoint != null) {`
`363`	`363`	`this.endpoint.stop(callback);`
`364`	`364`	`}`
	`365`	`+ else {`
	`366`	`+ callback.run();`
	`367`	`+ }`
`365`	`368`	`}`
`366`	`369`
`367`	`370`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -95,6 +95,9 @@ public void stop(Runnable callback) {`
`95`	`95`	`if (this.running.getAndSet(false)) {`
`96`	`96`	`this.targetIntegrationFlow.stop(callback);`
`97`	`97`	`}`
	`98`	`+ else {`
	`99`	`+ callback.run();`
	`100`	`+ }`
`98`	`101`	`}`
`99`	`102`
`100`	`103`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`/*`
`2`		`- * Copyright 2002-2014 the original author or authors.`
	`2`	`+ * Copyright 2002-2018 the original author or authors.`
`3`	`3`	`*`
`4`	`4`	`* Licensed under the Apache License, Version 2.0 (the "License");`
`5`	`5`	`* you may not use this file except in compliance with the License.`
`@@ -36,6 +36,7 @@`
`36`	`36`	`* @author Gary Russell`
`37`	`37`	`* @author Artem Bilan`
`38`	`38`	`* @author Ali Shahbour`
	`39`	`+ *`
`39`	`40`	`* @since 3.0`
`40`	`41`	`*`
`41`	`42`	`*/`
`@@ -192,6 +193,9 @@ public void stop(Runnable callback) {`
`192`	`193`	`if (this.adapter != null) {`
`193`	`194`	`this.adapter.stop(callback);`
`194`	`195`	`}`
	`196`	`+ else {`
	`197`	`+ callback.run();`
	`198`	`+ }`
`195`	`199`	`}`
`196`	`200`
`197`	`201`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`/*`
`2`		`- * Copyright 2002-2016 the original author or authors.`
	`2`	`+ * Copyright 2002-2018 the original author or authors.`
`3`	`3`	`*`
`4`	`4`	`* Licensed under the Apache License, Version 2.0 (the "License");`
`5`	`5`	`* you may not use this file except in compliance with the License.`
`@@ -43,9 +43,12 @@`
`43`	`43`	`/**`
`44`	`44`	`* @author Mark Fisher`
`45`	`45`	`* @author Gary Russell`
	`46`	`+ * @author Artem Bilan`
	`47`	`+ *`
`46`	`48`	`* @since 2.0`
`47`	`49`	`*/`
`48`		`-public class SubscribableJmsChannel extends AbstractJmsChannel implements SubscribableChannel, SmartLifecycle, DisposableBean {`
	`50`	`+public class SubscribableJmsChannel extends AbstractJmsChannel`
	`51`	`+ implements SubscribableChannel, SmartLifecycle, DisposableBean {`
`49`	`52`
`50`	`53`	`private final AbstractMessageListenerContainer container;`
`51`	`54`
`@@ -72,13 +75,15 @@ public void setMaxSubscribers(int maxSubscribers) {`
`72`	`75`
`73`	`76`	`@Override`
`74`	`77`	`public boolean subscribe(MessageHandler handler) {`
`75`		`- Assert.state(this.dispatcher != null, "'MessageDispatcher' must not be null. This channel might not have been initialized");`
	`78`	`+ Assert.state(this.dispatcher != null,`
	`79`	`+ "'MessageDispatcher' must not be null. This channel might not have been initialized");`
`76`	`80`	`return this.dispatcher.addHandler(handler);`
`77`	`81`	`}`
`78`	`82`
`79`	`83`	`@Override`
`80`	`84`	`public boolean unsubscribe(MessageHandler handler) {`
`81`		`- Assert.state(this.dispatcher != null, "'MessageDispatcher' must not be null. This channel might not have been initialized");`
	`85`	`+ Assert.state(this.dispatcher != null,`
	`86`	`+ "'MessageDispatcher' must not be null. This channel might not have been initialized");`
`82`	`87`	`return this.dispatcher.removeHandler(handler);`
`83`	`88`	`}`
`84`	`89`
`@@ -126,7 +131,7 @@ private void configureDispatcher(boolean isPubSub) {`
`126`	`131`
`127`	`132`	`@Override`
`128`	`133`	`public boolean isAutoStartup() {`
`129`		`- return (this.container != null) ? this.container.isAutoStartup() : false;`
	`134`	`+ return (this.container != null) && this.container.isAutoStartup();`
`130`	`135`	`}`
`131`	`136`
`132`	`137`	`@Override`
`@@ -136,7 +141,7 @@ public int getPhase() {`
`136`	`141`
`137`	`142`	`@Override`
`138`	`143`	`public boolean isRunning() {`
`139`		`- return (this.container != null) ? this.container.isRunning() : false;`
	`144`	`+ return (this.container != null) && this.container.isRunning();`
`140`	`145`	`}`
`141`	`146`
`142`	`147`	`@Override`
`@@ -158,6 +163,9 @@ public void stop(Runnable callback) {`
`158`	`163`	`if (this.container != null) {`
`159`	`164`	`this.container.stop(callback);`
`160`	`165`	`}`
	`166`	`+ else {`
	`167`	`+ callback.run();`
	`168`	`+ }`
`161`	`169`	`}`
`162`	`170`
`163`	`171`	`@Override`
`@@ -226,6 +234,7 @@ else if (this.logger.isWarnEnabled()) {`
`226`	`234`	`throw new MessagingException("failed to handle incoming JMS Message", e);`
`227`	`235`	`}`
`228`	`236`	`}`
	`237`	`+`
`229`	`238`	`}`
`230`	`239`
`231`	`240`	`}`