Thread-safe access to WebSocketServerFactory and WebSocketExtensions

Closes gh-24745

spring-websocket/src/main/java/org/springframework/web/socket/server/jetty/JettyRequestUpgradeStrategy.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2020 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.
@@ -37,6 +37,7 @@ import org.springframework.http.server.ServerHttpRequest;
 import org.springframework.http.server.ServerHttpResponse;
 import org.springframework.http.server.ServletServerHttpRequest;
 import org.springframework.http.server.ServletServerHttpResponse;
+import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 import org.springframework.util.CollectionUtils;
 import org.springframework.web.context.ServletContextAware;
@@ -63,15 +64,18 @@ public class JettyRequestUpgradeStrategy implements RequestUpgradeStrategy, Serv
 	private static final ThreadLocal<WebSocketHandlerContainer> containerHolder =
 			new NamedThreadLocal<>("WebSocketHandlerContainer");
 
-
+	@Nullable
 	private WebSocketPolicy policy;
 
-	private WebSocketServerFactory factory;
+	@Nullable
+	private volatile WebSocketServerFactory factory;
 
+	@Nullable
 	private ServletContext servletContext;
 
 	private volatile boolean running = false;
 
+	@Nullable
 	private volatile List<WebSocketExtension> supportedExtensions;
 
 
@@ -114,17 +118,20 @@ public class JettyRequestUpgradeStrategy implements RequestUpgradeStrategy, Serv
 		if (!isRunning()) {
 			this.running = true;
 			try {
-				if (this.factory == null) {
-					this.factory = new WebSocketServerFactory(servletContext, this.policy);
+				WebSocketServerFactory factory = this.factory;
+				if (factory == null) {
+					Assert.state(this.servletContext != null, "No ServletContext set");
+					factory = new WebSocketServerFactory(this.servletContext, this.policy);
+					this.factory = factory;
 				}
-				this.factory.setCreator((request, response) -> {
-                    WebSocketHandlerContainer container = containerHolder.get();
-                    Assert.state(container != null, "Expected WebSocketHandlerContainer");
-                    response.setAcceptedSubProtocol(container.getSelectedProtocol());
-                    response.setExtensions(container.getExtensionConfigs());
-                    return container.getHandler();
-                });
-				this.factory.start();
+				factory.setCreator((request, response) -> {
+					WebSocketHandlerContainer container = containerHolder.get();
+					Assert.state(container != null, "Expected WebSocketHandlerContainer");
+					response.setAcceptedSubProtocol(container.getSelectedProtocol());
+					response.setExtensions(container.getExtensionConfigs());
+					return container.getHandler();
+				});
+				factory.start();
 			}
 			catch (Throwable ex) {
 				throw new IllegalStateException("Unable to start Jetty WebSocketServerFactory", ex);
@@ -136,9 +143,10 @@ public class JettyRequestUpgradeStrategy implements RequestUpgradeStrategy, Serv
 	public void stop() {
 		if (isRunning()) {
 			this.running = false;
-			if (this.factory != null) {
+			WebSocketServerFactory factory = this.factory;
+			if (factory != null) {
 				try {
-					this.factory.stop();
+					factory.stop();
 				}
 				catch (Throwable ex) {
 					throw new IllegalStateException("Unable to stop Jetty WebSocketServerFactory", ex);
@@ -160,14 +168,18 @@ public class JettyRequestUpgradeStrategy implements RequestUpgradeStrategy, Serv
 
 	@Override
 	public List<WebSocketExtension> getSupportedExtensions(ServerHttpRequest request) {
-		if (this.supportedExtensions == null) {
-			this.supportedExtensions = buildWebSocketExtensions();
+		List<WebSocketExtension> extensions = this.supportedExtensions;
+		if (extensions == null) {
+			extensions = buildWebSocketExtensions();
+			this.supportedExtensions = extensions;
 		}
-		return this.supportedExtensions;
+		return extensions;
 	}
 
 	private List<WebSocketExtension> buildWebSocketExtensions() {
-		Set<String> names = this.factory.getExtensionFactory().getExtensionNames();
+		WebSocketServerFactory factory = this.factory;
+		Assert.state(factory != null, "No WebSocketServerFactory available");
+		Set<String> names = factory.getExtensionFactory().getExtensionNames();
 		List<WebSocketExtension> result = new ArrayList<>(names.size());
 		for (String name : names) {
 			result.add(new WebSocketExtension(name));
@@ -177,7 +189,7 @@ public class JettyRequestUpgradeStrategy implements RequestUpgradeStrategy, Serv
 
 	@Override
 	public void upgrade(ServerHttpRequest request, ServerHttpResponse response,
-			String selectedProtocol, List<WebSocketExtension> selectedExtensions, Principal user,
+			@Nullable String selectedProtocol, List<WebSocketExtension> selectedExtensions, @Nullable Principal user,
 			WebSocketHandler wsHandler, Map<String, Object> attributes) throws HandshakeFailureException {
 
 		Assert.isInstanceOf(ServletServerHttpRequest.class, request, "ServletServerHttpRequest required");
@@ -186,7 +198,9 @@ public class JettyRequestUpgradeStrategy implements RequestUpgradeStrategy, Serv
 		Assert.isInstanceOf(ServletServerHttpResponse.class, response, "ServletServerHttpResponse required");
 		HttpServletResponse servletResponse = ((ServletServerHttpResponse) response).getServletResponse();
 
-		Assert.isTrue(this.factory.isUpgradeRequest(servletRequest, servletResponse), "Not a WebSocket handshake");
+		WebSocketServerFactory factory = this.factory;
+		Assert.state(factory != null, "No WebSocketServerFactory available");
+		Assert.isTrue(factory.isUpgradeRequest(servletRequest, servletResponse), "Not a WebSocket handshake");
 
 		JettyWebSocketSession session = new JettyWebSocketSession(attributes, user);
 		JettyWebSocketHandlerAdapter handlerAdapter = new JettyWebSocketHandlerAdapter(wsHandler, session);
@@ -196,7 +210,7 @@ public class JettyRequestUpgradeStrategy implements RequestUpgradeStrategy, Serv
 
 		try {
 			containerHolder.set(container);
-			this.factory.acceptWebSocket(servletRequest, servletResponse);
+			factory.acceptWebSocket(servletRequest, servletResponse);
 		}
 		catch (IOException ex) {
 			throw new HandshakeFailureException(
@@ -212,12 +226,13 @@ public class JettyRequestUpgradeStrategy implements RequestUpgradeStrategy, Serv
 
 		private final JettyWebSocketHandlerAdapter handler;
 
+		@Nullable
 		private final String selectedProtocol;
 
 		private final List<ExtensionConfig> extensionConfigs;
 
-		public WebSocketHandlerContainer(
-				JettyWebSocketHandlerAdapter handler, String protocol, List<WebSocketExtension> extensions) {
+		public WebSocketHandlerContainer(JettyWebSocketHandlerAdapter handler,
+				@Nullable String protocol, List<WebSocketExtension> extensions) {
 
 			this.handler = handler;
 			this.selectedProtocol = protocol;
@@ -236,6 +251,7 @@ public class JettyRequestUpgradeStrategy implements RequestUpgradeStrategy, Serv
 			return this.handler;
 		}
 
+		@Nullable
 		public String getSelectedProtocol() {
 			return this.selectedProtocol;
 		}

spring-websocket/src/main/java/org/springframework/web/socket/server/jetty/package-info.java

@@ -1,4 +1,9 @@
 /**
  * Server-side support for the Jetty 9+ WebSocket API.
  */
+@NonNullApi
+@NonNullFields
 package org.springframework.web.socket.server.jetty;
+
+import org.springframework.lang.NonNullApi;
+import org.springframework.lang.NonNullFields;