Consistent volatile access to running flag in Lifecycle implementations

Issue: SPR-16488

spring-jms/src/main/java/org/springframework/jms/listener/AbstractJmsListeningContainer.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.
@@ -80,7 +80,7 @@ public abstract class AbstractJmsListeningContainer extends JmsDestinationAccess
 
 	private boolean active = false;
 
-	private boolean running = false;
+	private volatile boolean running = false;
 
 	private final List<Object> pausedTasks = new LinkedList<>();
 
@@ -350,9 +350,7 @@ public abstract class AbstractJmsListeningContainer extends JmsDestinationAccess
 	 */
 	@Override
 	public final boolean isRunning() {
-		synchronized (this.lifecycleMonitor) {
-			return (this.running && runningAllowed());
-		}
+		return (this.running && runningAllowed());
 	}
 
 	/**

spring-messaging/src/main/java/org/springframework/messaging/simp/annotation/support/SimpAnnotationMethodMessageHandler.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.
@@ -111,10 +111,10 @@ public class SimpAnnotationMethodMessageHandler extends AbstractMethodMessageHan
 	@Nullable
 	private MessageHeaderInitializer headerInitializer;
 
-	private final Object lifecycleMonitor = new Object();
-
 	private volatile boolean running = false;
 
+	private final Object lifecycleMonitor = new Object();
+
 
 	/**
 	 * Create an instance of SimpAnnotationMethodMessageHandler with the given
@@ -299,9 +299,7 @@ public class SimpAnnotationMethodMessageHandler extends AbstractMethodMessageHan
 
 	@Override
 	public final boolean isRunning() {
-		synchronized (this.lifecycleMonitor) {
-			return this.running;
-		}
+		return this.running;
 	}
 
 

spring-messaging/src/main/java/org/springframework/messaging/simp/broker/AbstractBrokerMessageHandler.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.
@@ -156,9 +156,7 @@ public abstract class AbstractBrokerMessageHandler
 	@Override
 	public void start() {
 		synchronized (this.lifecycleMonitor) {
-			if (logger.isInfoEnabled()) {
-				logger.info("Starting...");
-			}
+			logger.info("Starting...");
 			this.clientInboundChannel.subscribe(this);
 			this.brokerChannel.subscribe(this);
 			if (this.clientInboundChannel instanceof InterceptableChannel) {
@@ -176,9 +174,7 @@ public abstract class AbstractBrokerMessageHandler
 	@Override
 	public void stop() {
 		synchronized (this.lifecycleMonitor) {
-			if (logger.isInfoEnabled()) {
-				logger.info("Stopping...");
-			}
+			logger.info("Stopping...");
 			stopInternal();
 			this.clientInboundChannel.unsubscribe(this);
 			this.brokerChannel.unsubscribe(this);
@@ -209,9 +205,7 @@ public abstract class AbstractBrokerMessageHandler
 	 */
 	@Override
 	public final boolean isRunning() {
-		synchronized (this.lifecycleMonitor) {
-			return this.running;
-		}
+		return this.running;
 	}
 
 	/**

spring-messaging/src/main/java/org/springframework/messaging/simp/user/UserDestinationMessageHandler.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.
@@ -67,10 +67,10 @@ public class UserDestinationMessageHandler implements MessageHandler, SmartLifec
 	@Nullable
 	private MessageHeaderInitializer headerInitializer;
 
-	private final Object lifecycleMonitor = new Object();
-
 	private volatile boolean running = false;
 
+	private final Object lifecycleMonitor = new Object();
+
 
 	/**
 	 * Create an instance with the given client and broker channels subscribing
@@ -185,9 +185,7 @@ public class UserDestinationMessageHandler implements MessageHandler, SmartLifec
 
 	@Override
 	public final boolean isRunning() {
-		synchronized (this.lifecycleMonitor) {
-			return this.running;
-		}
+		return this.running;
 	}
 
 

spring-tx/src/main/java/org/springframework/jca/endpoint/GenericMessageEndpointManager.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.
@@ -161,7 +161,7 @@ public class GenericMessageEndpointManager implements SmartLifecycle, Initializi
 
 	private int phase = Integer.MAX_VALUE;
 
-	private boolean running = false;
+	private volatile boolean running = false;
 
 	private final Object lifecycleMonitor = new Object();
 
@@ -330,9 +330,7 @@ public class GenericMessageEndpointManager implements SmartLifecycle, Initializi
 	 */
 	@Override
 	public boolean isRunning() {
-		synchronized (this.lifecycleMonitor) {
-			return this.running;
-		}
+		return this.running;
 	}
 
 	/**

spring-web/src/test/java/org/springframework/http/server/reactive/bootstrap/AbstractHttpServer.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.
@@ -42,7 +42,7 @@ public abstract class AbstractHttpServer implements HttpServer {
 
 	private Map<String, HttpHandler> handlerMap;
 
-	private boolean running;
+	private volatile boolean running;
 
 	private final Object lifecycleMonitor = new Object();
 
@@ -87,8 +87,8 @@ public abstract class AbstractHttpServer implements HttpServer {
 	}
 
 	protected HttpHandler resolveHttpHandler() {
-		return getHttpHandlerMap() != null ?
-				new ContextPathCompositeHandler(getHttpHandlerMap()) : getHttpHandler();
+		return (getHttpHandlerMap() != null ?
+				new ContextPathCompositeHandler(getHttpHandlerMap()) : getHttpHandler());
 	}
 
 
@@ -111,26 +111,23 @@ public abstract class AbstractHttpServer implements HttpServer {
 
 	// Lifecycle
 
-	@Override
-	public boolean isRunning() {
-		synchronized (this.lifecycleMonitor) {
-			return this.running;
-		}
-	}
-
 	@Override
 	public final void start() {
 		synchronized (this.lifecycleMonitor) {
 			if (!isRunning()) {
 				String serverName = getClass().getSimpleName();
-				logger.debug("Starting " + serverName + "...");
+				if (logger.isDebugEnabled()) {
+					logger.debug("Starting " + serverName + "...");
+				}
 				this.running = true;
 				try {
 					StopWatch stopWatch = new StopWatch();
 					stopWatch.start();
 					startInternal();
 					long millis = stopWatch.getTotalTimeMillis();
-					logger.debug("Server started on port " + getPort() + "(" + millis + " millis).");
+					if (logger.isDebugEnabled()) {
+						logger.debug("Server started on port " + getPort() + "(" + millis + " millis).");
+					}
 				}
 				catch (Throwable ex) {
 					throw new IllegalStateException(ex);
@@ -167,6 +164,12 @@ public abstract class AbstractHttpServer implements HttpServer {
 
 	protected abstract void stopInternal() throws Exception;
 
+	@Override
+	public boolean isRunning() {
+		return this.running;
+	}
+
+
 	private void reset() {
 		this.host = "0.0.0.0";
 		this.port = 0;

spring-webflux/src/main/java/org/springframework/web/reactive/socket/client/JettyWebSocketClient.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.
@@ -54,7 +54,7 @@ public class JettyWebSocketClient extends WebSocketClientSupport implements WebS
 
 	private final boolean externallyManaged;
 
-	private boolean running = false;
+	private volatile boolean running = false;
 
 	private final Object lifecycleMonitor = new Object();
 
@@ -135,9 +135,7 @@ public class JettyWebSocketClient extends WebSocketClientSupport implements WebS
 
 	@Override
 	public boolean isRunning() {
-		synchronized (this.lifecycleMonitor) {
-			return this.running;
-		}
+		return this.running;
 	}
 
 

spring-webflux/src/main/java/org/springframework/web/reactive/socket/server/support/HandshakeWebSocketService.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.
@@ -135,10 +135,6 @@ public class HandshakeWebSocketService implements WebSocketService, Lifecycle {
 		return this.upgradeStrategy;
 	}
 
-	@Override
-	public boolean isRunning() {
-		return this.running;
-	}
 
 	@Override
 	public void start() {
@@ -168,6 +164,11 @@ public class HandshakeWebSocketService implements WebSocketService, Lifecycle {
 		}
 	}
 
+	@Override
+	public boolean isRunning() {
+		return this.running;
+	}
+
 
 	@Override
 	public Mono<Void> handleRequest(ServerWebExchange exchange, WebSocketHandler handler) {

spring-websocket/src/main/java/org/springframework/web/socket/client/ConnectionManagerSupport.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.
@@ -38,15 +38,14 @@ public abstract class ConnectionManagerSupport implements SmartLifecycle {
 
 	protected final Log logger = LogFactory.getLog(getClass());
 
-
 	private final URI uri;
 
 	private boolean autoStartup = false;
 
-	private boolean running = false;
-
 	private int phase = Integer.MAX_VALUE;
 
+	private volatile boolean running = false;
+
 	private final Object lifecycleMonitor = new Object();
 
 
@@ -161,9 +160,7 @@ public abstract class ConnectionManagerSupport implements SmartLifecycle {
 	 */
 	@Override
 	public boolean isRunning() {
-		synchronized (this.lifecycleMonitor) {
-			return this.running;
-		}
+		return this.running;
 	}
 
 

spring-websocket/src/main/java/org/springframework/web/socket/client/jetty/JettyWebSocketClient.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.
@@ -61,8 +61,6 @@ public class JettyWebSocketClient extends AbstractWebSocketClient implements Lif
 
 	private final org.eclipse.jetty.websocket.client.WebSocketClient client;
 
-	private final Object lifecycleMonitor = new Object();
-
 	@Nullable
 	private AsyncListenableTaskExecutor taskExecutor = new SimpleAsyncTaskExecutor();
 
@@ -102,47 +100,33 @@ public class JettyWebSocketClient extends AbstractWebSocketClient implements Lif
 		return this.taskExecutor;
 	}
 
-	@Override
-	public boolean isRunning() {
-		synchronized (this.lifecycleMonitor) {
-			return this.client.isStarted();
-		}
-	}
 
 	@Override
 	public void start() {
-		synchronized (this.lifecycleMonitor) {
-			if (!isRunning()) {
-				try {
-					if (logger.isInfoEnabled()) {
-						logger.info("Starting Jetty WebSocketClient");
-					}
-					this.client.start();
-				}
-				catch (Exception ex) {
-					throw new IllegalStateException("Failed to start Jetty client", ex);
-				}
-			}
+		try {
+			this.client.start();
+		}
+		catch (Exception ex) {
+			throw new IllegalStateException("Failed to start Jetty WebSocketClient", ex);
 		}
 	}
 
 	@Override
 	public void stop() {
-		synchronized (this.lifecycleMonitor) {
-			if (isRunning()) {
-				try {
-					if (logger.isInfoEnabled()) {
-						logger.info("Stopping Jetty WebSocketClient");
-					}
-					this.client.stop();
-				}
-				catch (Exception ex) {
-					logger.error("Error stopping Jetty WebSocketClient", ex);
-				}
-			}
+		try {
+			this.client.stop();
 		}
+		catch (Exception ex) {
+			logger.error("Failed to stop Jetty WebSocketClient", ex);
+		}
+	}
+
+	@Override
+	public boolean isRunning() {
+		return this.client.isStarted();
 	}
 
+
 	@Override
 	public ListenableFuture<WebSocketSession> doHandshake(WebSocketHandler webSocketHandler,
 			String uriTemplate, Object... uriVars) {

spring-websocket/src/main/java/org/springframework/web/socket/messaging/SubProtocolWebSocketHandler.java

@@ -104,10 +104,10 @@ public class SubProtocolWebSocketHandler
 
 	private final Stats stats = new Stats();
 
-	private final Object lifecycleMonitor = new Object();
-
 	private volatile boolean running = false;
 
+	private final Object lifecycleMonitor = new Object();
+
 
 	/**
 	 * Create a new {@code SubProtocolWebSocketHandler} for the given inbound and outbound channels.
@@ -282,9 +282,7 @@ public class SubProtocolWebSocketHandler
 
 	@Override
 	public final boolean isRunning() {
-		synchronized (this.lifecycleMonitor) {
-			return this.running;
-		}
+		return this.running;
 	}
 
 

spring-websocket/src/main/java/org/springframework/web/socket/messaging/WebSocketStompClient.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.
@@ -73,17 +73,16 @@ public class WebSocketStompClient extends StompClientSupport implements SmartLif
 
 	private static final Log logger = LogFactory.getLog(WebSocketStompClient.class);
 
-
 	private final WebSocketClient webSocketClient;
 
 	private int inboundMessageSizeLimit = 64 * 1024;
 
 	private boolean autoStartup = true;
 
-	private boolean running = false;
-
 	private int phase = Integer.MAX_VALUE;
 
+	private volatile boolean running = false;
+
 
 	/**
 	 * Class constructor. Sets {@link #setDefaultHeartbeat} to "0,0" but will

spring-websocket/src/main/java/org/springframework/web/socket/server/support/AbstractHandshakeHandler.java

@@ -185,10 +185,6 @@ public abstract class AbstractHandshakeHandler implements HandshakeHandler, Life
 		return StringUtils.toStringArray(this.supportedProtocols);
 	}
 
-	@Override
-	public boolean isRunning() {
-		return this.running;
-	}
 
 	@Override
 	public void start() {
@@ -218,6 +214,11 @@ public abstract class AbstractHandshakeHandler implements HandshakeHandler, Life
 		}
 	}
 
+	@Override
+	public boolean isRunning() {
+		return this.running;
+	}
+
 
 	@Override
 	public final boolean doHandshake(ServerHttpRequest request, ServerHttpResponse response,

spring-websocket/src/main/java/org/springframework/web/socket/server/support/WebSocketHttpRequestHandler.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.
@@ -21,7 +21,6 @@ import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-
 import javax.servlet.ServletContext;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
@@ -59,7 +58,7 @@ import org.springframework.web.socket.server.HandshakeInterceptor;
  */
 public class WebSocketHttpRequestHandler implements HttpRequestHandler, Lifecycle, ServletContextAware {
 
-	private final Log logger = LogFactory.getLog(WebSocketHttpRequestHandler.class);
+	private static final Log logger = LogFactory.getLog(WebSocketHttpRequestHandler.class);
 
 	private final WebSocketHandler wsHandler;
 
@@ -120,10 +119,6 @@ public class WebSocketHttpRequestHandler implements HttpRequestHandler, Lifecycl
 		}
 	}
 
-	@Override
-	public boolean isRunning() {
-		return this.running;
-	}
 
 	@Override
 	public void start() {
@@ -145,6 +140,11 @@ public class WebSocketHttpRequestHandler implements HttpRequestHandler, Lifecycl
 		}
 	}
 
+	@Override
+	public boolean isRunning() {
+		return this.running;
+	}
+
 
 	@Override
 	public void handleRequest(HttpServletRequest servletRequest, HttpServletResponse servletResponse)

spring-websocket/src/main/java/org/springframework/web/socket/sockjs/client/AbstractXhrTransport.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.
@@ -63,8 +63,7 @@ public abstract class AbstractXhrTransport implements XhrTransport {
 
 	@Override
 	public List<TransportType> getTransportTypes() {
-		return (isXhrStreamingDisabled() ?
-				Collections.singletonList(TransportType.XHR) :
+		return (isXhrStreamingDisabled() ? Collections.singletonList(TransportType.XHR) :
 				Arrays.asList(TransportType.XHR_STREAMING, TransportType.XHR));
 	}
 
@@ -166,10 +165,4 @@ public abstract class AbstractXhrTransport implements XhrTransport {
 	protected abstract ResponseEntity<String> executeSendRequestInternal(
 			URI url, HttpHeaders headers, TextMessage message);
 
-
-	@Override
-	public String toString() {
-		return getClass().getSimpleName();
-	}
-
 }

spring-websocket/src/main/java/org/springframework/web/socket/sockjs/client/JettyXhrTransport.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.
@@ -86,7 +86,7 @@ public class JettyXhrTransport extends AbstractXhrTransport implements Lifecycle
 			}
 		}
 		catch (Exception ex) {
-			throw new SockJsException("Failed to start " + this, ex);
+			throw new SockJsException("Failed to start JettyXhrTransport", ex);
 		}
 	}
 
@@ -98,7 +98,7 @@ public class JettyXhrTransport extends AbstractXhrTransport implements Lifecycle
 			}
 		}
 		catch (Exception ex) {
-			throw new SockJsException("Failed to stop " + this, ex);
+			throw new SockJsException("Failed to stop JettyXhrTransport", ex);
 		}
 	}
 

spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/TransportHandlingSockJsService.java

@@ -81,7 +81,7 @@ public class TransportHandlingSockJsService extends AbstractSockJsService implem
 	@Nullable
 	private ScheduledFuture<?> sessionCleanupTask;
 
-	private boolean running;
+	private volatile boolean running;
 
 
 	/**

spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/WebSocketTransportHandler.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.
@@ -17,7 +17,6 @@
 package org.springframework.web.socket.sockjs.transport.handler;
 
 import java.util.Map;
-
 import javax.servlet.ServletContext;
 
 import org.springframework.context.Lifecycle;
@@ -53,11 +52,11 @@ public class WebSocketTransportHandler extends AbstractTransportHandler
 
 	private final HandshakeHandler handshakeHandler;
 
-	private boolean running;
+	private volatile boolean running;
 
 
 	public WebSocketTransportHandler(HandshakeHandler handshakeHandler) {
-		Assert.notNull(handshakeHandler, "handshakeHandler must not be null");
+		Assert.notNull(handshakeHandler, "HandshakeHandler must not be null");
 		this.handshakeHandler = handshakeHandler;
 	}
 
@@ -78,10 +77,6 @@ public class WebSocketTransportHandler extends AbstractTransportHandler
 		}
 	}
 
-	@Override
-	public boolean isRunning() {
-		return this.running;
-	}
 
 	@Override
 	public void start() {
@@ -103,6 +98,12 @@ public class WebSocketTransportHandler extends AbstractTransportHandler
 		}
 	}
 
+	@Override
+	public boolean isRunning() {
+		return this.running;
+	}
+
+
 	@Override
 	public boolean checkSessionType(SockJsSession session) {
 		return session instanceof WebSocketServerSockJsSession;