Improve Reactive...Context thread safety

Refactor `ReactiveWebServerApplicationContext` to improve thread safety
by using a single manager object rather than then trying to synchronize
the `WebServer` and `HttpHandler`.

Closes gh-14666

Author: philwebb

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/context/ReactiveWebServerApplicationContext.java

@@ -42,9 +42,7 @@ public class ReactiveWebServerApplicationContext
 		extends GenericReactiveWebApplicationContext
 		implements ConfigurableWebServerApplicationContext {
 
-	private volatile WebServer webServer;
-
-	private volatile DeferredHttpHandler httpHandler;
+	private volatile ServerManager serverManager;
 
 	private String serverNamespace;
 
@@ -87,41 +85,13 @@ protected void onRefresh() {
 	}
 
 	private void createWebServer() {
-		WebServer localServer = this.webServer;
-		if (localServer == null) {
-			DeferredHttpHandler localHandler = new DeferredHttpHandler(
-					this::getHttpHandler);
-			this.webServer = getWebServerFactory().getWebServer(localHandler);
-			this.httpHandler = localHandler;
+		ServerManager serverManager = this.serverManager;
+		if (serverManager == null) {
+			this.serverManager = ServerManager.get(getWebServerFactory());
 		}
 		initPropertySources();
 	}
 
-	@Override
-	protected void finishRefresh() {
-		super.finishRefresh();
-		WebServer localServer = startReactiveWebServer();
-		if (localServer != null) {
-			publishEvent(new ReactiveWebServerInitializedEvent(localServer, this));
-		}
-	}
-
-	@Override
-	protected void onClose() {
-		super.onClose();
-		stopAndReleaseReactiveWebServer();
-	}
-
-	/**
-	 * Returns the {@link WebServer} that was created by the context or {@code null} if
-	 * the server has not yet been created.
-	 * @return the web server
-	 */
-	@Override
-	public WebServer getWebServer() {
-		return this.webServer;
-	}
-
 	/**
 	 * Return the {@link ReactiveWebServerFactory} that should be used to create the
 	 * reactive web server. By default this method searches for a suitable bean in the
@@ -146,6 +116,21 @@ protected ReactiveWebServerFactory getWebServerFactory() {
 		return getBeanFactory().getBean(beanNames[0], ReactiveWebServerFactory.class);
 	}
 
+	@Override
+	protected void finishRefresh() {
+		super.finishRefresh();
+		WebServer webServer = startReactiveWebServer();
+		if (webServer != null) {
+			publishEvent(new ReactiveWebServerInitializedEvent(webServer, this));
+		}
+	}
+
+	private WebServer startReactiveWebServer() {
+		ServerManager serverManager = this.serverManager;
+		ServerManager.start(serverManager, this::getHttpHandler);
+		return ServerManager.getWebServer(serverManager);
+	}
+
 	/**
 	 * Return the {@link HttpHandler} that should be used to process the reactive web
 	 * server. By default this method searches for a suitable bean in the context itself.
@@ -166,32 +151,32 @@ protected HttpHandler getHttpHandler() {
 		return getBeanFactory().getBean(beanNames[0], HttpHandler.class);
 	}
 
-	private WebServer startReactiveWebServer() {
-		WebServer localServer = this.webServer;
-		DeferredHttpHandler localHandler = this.httpHandler;
-		if (localServer != null) {
-			if (localHandler != null) {
-				localHandler.initialize();
-				this.httpHandler = null;
-			}
-			localServer.start();
-		}
-		return localServer;
+	@Override
+	protected void onClose() {
+		super.onClose();
+		stopAndReleaseReactiveWebServer();
 	}
 
 	private void stopAndReleaseReactiveWebServer() {
-		WebServer localServer = this.webServer;
-		if (localServer != null) {
-			try {
-				localServer.stop();
-				this.webServer = null;
-			}
-			catch (Exception ex) {
-				throw new IllegalStateException(ex);
-			}
+		ServerManager serverManager = this.serverManager;
+		try {
+			ServerManager.stop(serverManager);
+		}
+		finally {
+			this.serverManager = null;
 		}
 	}
 
+	/**
+	 * Returns the {@link WebServer} that was created by the context or {@code null} if
+	 * the server has not yet been created.
+	 * @return the web server
+	 */
+	@Override
+	public WebServer getWebServer() {
+		return ServerManager.getWebServer(this.serverManager);
+	}
+
 	@Override
 	public String getServerNamespace() {
 		return this.serverNamespace;
@@ -203,22 +188,18 @@ public void setServerNamespace(String serverNamespace) {
 	}
 
 	/**
-	 * {@link HttpHandler} that defers to a supplied handler which is initialized only
-	 * when the server starts.
+	 * Internal class used to manage the server and the {@link HttpHandler}, taking care
+	 * not to initialize the hander too early.
 	 */
-	static class DeferredHttpHandler implements HttpHandler {
+	static final class ServerManager implements HttpHandler {
 
-		private Supplier<HttpHandler> factory;
+		private final WebServer server;
 
-		private HttpHandler handler;
+		private volatile HttpHandler handler;
 
-		DeferredHttpHandler(Supplier<HttpHandler> factory) {
-			this.factory = factory;
+		private ServerManager(ReactiveWebServerFactory factory) {
 			this.handler = this::handleUninitialized;
-		}
-
-		public void initialize() {
-			this.handler = this.factory.get();
+			this.server = factory.getWebServer(this);
 		}
 
 		private Mono<Void> handleUninitialized(ServerHttpRequest request,
@@ -236,6 +217,33 @@ public HttpHandler getHandler() {
 			return this.handler;
 		}
 
+		public static ServerManager get(ReactiveWebServerFactory factory) {
+			return new ServerManager(factory);
+		}
+
+		public static WebServer getWebServer(ServerManager manager) {
+			return (manager != null) ? manager.server : null;
+		}
+
+		public static void start(ServerManager manager,
+				Supplier<HttpHandler> handlerSupplier) {
+			if (manager != null) {
+				manager.handler = handlerSupplier.get();
+				manager.server.start();
+			}
+		}
+
+		public static void stop(ServerManager manager) {
+			if (manager != null) {
+				try {
+					manager.server.stop();
+				}
+				catch (Exception ex) {
+					throw new IllegalStateException(ex);
+				}
+			}
+		}
+
 	}
 
 }

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/context/AnnotationConfigReactiveWebServerApplicationContextTests.java

@@ -18,7 +18,7 @@
 
 import org.junit.Test;
 
-import org.springframework.boot.web.reactive.context.ReactiveWebServerApplicationContext.DeferredHttpHandler;
+import org.springframework.boot.web.reactive.context.ReactiveWebServerApplicationContext.ServerManager;
 import org.springframework.boot.web.reactive.context.config.ExampleReactiveWebServerApplicationConfiguration;
 import org.springframework.boot.web.reactive.server.MockReactiveWebServerFactory;
 import org.springframework.boot.web.reactive.server.ReactiveWebServerFactory;
@@ -98,8 +98,8 @@ private void verifyContext() {
 				.getBean(MockReactiveWebServerFactory.class);
 		HttpHandler expectedHandler = this.context.getBean(HttpHandler.class);
 		HttpHandler actualHandler = factory.getWebServer().getHttpHandler();
-		if (actualHandler instanceof DeferredHttpHandler) {
-			actualHandler = ((DeferredHttpHandler) actualHandler).getHandler();
+		if (actualHandler instanceof ServerManager) {
+			actualHandler = ((ServerManager) actualHandler).getHandler();
 		}
 		assertThat(actualHandler).isEqualTo(expectedHandler);
 	}