Close other HTTP connections on reload

This will prevent old persistent connections hanging onto the old shutdown application. Fixes #18776
quarkusio · Jul 28, 2021 · ef81669 · ef81669
1 parent 6ff45d3
commit ef81669
Show file tree

Hide file tree

Showing 4 changed files with 93 additions and 16 deletions.
diff --git a/...src/main/java/io/quarkus/vertx/http/deployment/devmode/console/ConfigEditorProcessor.java b/...src/main/java/io/quarkus/vertx/http/deployment/devmode/console/ConfigEditorProcessor.java
@@ -21,6 +21,7 @@
 import io.quarkus.runtime.configuration.ProfileManager;
 import io.quarkus.vertx.http.runtime.devmode.ConfigDescription;
 import io.quarkus.vertx.http.runtime.devmode.ConfigDescriptionsSupplier;
+import io.quarkus.vertx.http.runtime.devmode.VertxHttpHotReplacementSetup;
 import io.vertx.core.MultiMap;
 import io.vertx.ext.web.RoutingContext;
 
@@ -87,8 +88,12 @@ protected void handlePost(RoutingContext event, MultiMap form) throws Exception
                         writer.newLine();
                     }
                 }
-
-                DevConsoleManager.getHotReplacementContext().doScan(true);
+                VertxHttpHotReplacementSetup.setDoingHttpInitiatedReload(true);
+                try {
+                    DevConsoleManager.getHotReplacementContext().doScan(true);
+                } finally {
+                    VertxHttpHotReplacementSetup.setDoingHttpInitiatedReload(false);
+                }
                 flashMessage(event, "Configuration updated");
             }
         });

diff --git a/...ons/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/VertxHttpRecorder.java b/...ons/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/VertxHttpRecorder.java
@@ -60,6 +60,7 @@
 import io.quarkus.vertx.core.runtime.config.VertxConfiguration;
 import io.quarkus.vertx.http.runtime.HttpConfiguration.InsecureRequests;
 import io.quarkus.vertx.http.runtime.devmode.RemoteSyncHandler;
+import io.quarkus.vertx.http.runtime.devmode.VertxHttpHotReplacementSetup;
 import io.quarkus.vertx.http.runtime.filters.Filter;
 import io.quarkus.vertx.http.runtime.filters.Filters;
 import io.quarkus.vertx.http.runtime.filters.GracefulShutdownFilter;
@@ -168,6 +169,7 @@ public static void shutDownDevMode() {
         }
         rootHandler = null;
         hotReplacementHandler = null;
+
     }
 
     public static void startServerAfterFailedStart() {
@@ -250,6 +252,13 @@ public void startServer(Supplier<Vertx> vertx, ShutdownContext shutdown,
                         websocketSubProtocols, auxiliaryApplication);
                 if (launchMode != LaunchMode.DEVELOPMENT) {
                     shutdown.addShutdownTask(closeTask);
+                } else {
+                    shutdown.addShutdownTask(new Runnable() {
+                        @Override
+                        public void run() {
+                            VertxHttpHotReplacementSetup.handleDevModeRestart();
+                        }
+                    });
                 }
             }
         }

diff --git a/...ime/src/main/java/io/quarkus/vertx/http/runtime/devmode/VertxHttpHotReplacementSetup.java b/...ime/src/main/java/io/quarkus/vertx/http/runtime/devmode/VertxHttpHotReplacementSetup.java
@@ -1,5 +1,10 @@
 package io.quarkus.vertx.http.runtime.devmode;
 
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+
 import io.quarkus.dev.ErrorPageGenerators;
 import io.quarkus.dev.spi.HotReplacementContext;
 import io.quarkus.dev.spi.HotReplacementSetup;
@@ -39,7 +44,45 @@ public void handleFailedInitialStart() {
         VertxHttpRecorder.startServerAfterFailedStart();
     }
 
+    private static volatile Set<ConnectionBase> openConnections;
+    private static volatile boolean doingHttpInitiatedReload;
+
+    public static void handleDevModeRestart() {
+        if (doingHttpInitiatedReload) {
+            return;
+        }
+        Set<ConnectionBase> connections = new HashSet<>(openConnections);
+        for (ConnectionBase con : connections) {
+            con.close();
+        }
+    }
+
+    public static boolean isDoingHttpInitiatedReload() {
+        return doingHttpInitiatedReload;
+    }
+
+    public static void setDoingHttpInitiatedReload(boolean doingHttpInitiatedReload) {
+        VertxHttpHotReplacementSetup.doingHttpInitiatedReload = doingHttpInitiatedReload;
+    }
+
     void handleHotReplacementRequest(RoutingContext routingContext) {
+        if (openConnections == null) {
+            synchronized (VertxHttpHotReplacementSetup.class) {
+                if (openConnections == null) {
+                    openConnections = Collections.newSetFromMap(new ConcurrentHashMap<>());
+                }
+            }
+        }
+        ConnectionBase connectionBase = (ConnectionBase) routingContext.request().connection();
+        if (openConnections.add(connectionBase)) {
+            connectionBase.closeFuture().onComplete(new Handler<AsyncResult<Void>>() {
+                @Override
+                public void handle(AsyncResult<Void> event) {
+                    openConnections.remove(connectionBase);
+                }
+            });
+        }
+
         if ((nextUpdate > System.currentTimeMillis() && !hotReplacementContext.isTest())
                 || routingContext.request().headers().contains(HEADER_NAME)) {
             if (hotReplacementContext.getDeploymentProblem() != null) {
@@ -50,27 +93,42 @@ void handleHotReplacementRequest(RoutingContext routingContext) {
             return;
         }
         ClassLoader current = Thread.currentThread().getContextClassLoader();
-        ConnectionBase connectionBase = (ConnectionBase) routingContext.request().connection();
         connectionBase.getContext().executeBlocking(new Handler<Promise<Boolean>>() {
             @Override
             public void handle(Promise<Boolean> event) {
                 //the blocking pool may have a stale TCCL
                 Thread.currentThread().setContextClassLoader(current);
                 boolean restart = false;
-                synchronized (this) {
-                    if (nextUpdate < System.currentTimeMillis() || hotReplacementContext.isTest()) {
-                        nextUpdate = System.currentTimeMillis() + HOT_REPLACEMENT_INTERVAL;
-                        try {
-                            restart = hotReplacementContext.doScan(true);
-                        } catch (Exception e) {
-                            event.fail(new IllegalStateException("Unable to perform live reload scanning", e));
-                            return;
+                try {
+                    doingHttpInitiatedReload = true;
+                    synchronized (this) {
+                        if (nextUpdate < System.currentTimeMillis() || hotReplacementContext.isTest()) {
+                            nextUpdate = System.currentTimeMillis() + HOT_REPLACEMENT_INTERVAL;
+                            try {
+                                restart = hotReplacementContext.doScan(true);
+                            } catch (Exception e) {
+                                event.fail(new IllegalStateException("Unable to perform live reload scanning", e));
+                                return;
+                            }
                         }
                     }
-                }
-                if (hotReplacementContext.getDeploymentProblem() != null) {
-                    event.fail(hotReplacementContext.getDeploymentProblem());
-                    return;
+                    if (hotReplacementContext.getDeploymentProblem() != null) {
+                        event.fail(hotReplacementContext.getDeploymentProblem());
+                        return;
+                    }
+                    if (restart) {
+                        //close all connections on close, except for this one
+                        //this prevents long running requests such as SSE or websockets
+                        //from holding onto the old deployment
+                        Set<ConnectionBase> connections = new HashSet<>(openConnections);
+                        for (ConnectionBase con : connections) {
+                            if (con != connectionBase) {
+                                con.close();
+                            }
+                        }
+                    }
+                } finally {
+                    doingHttpInitiatedReload = false;
                 }
                 event.complete(restart);
             }

diff --git a/...nt-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/app/CuratedApplication.java b/...nt-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/app/CuratedApplication.java
@@ -21,6 +21,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.BiConsumer;
 import java.util.function.Consumer;
 import java.util.function.Function;
@@ -62,6 +63,8 @@ public class CuratedApplication implements Serializable, AutoCloseable {
 
     final AppModel appModel;
 
+    final AtomicInteger runtimeClassLoaderCount = new AtomicInteger();
+
     CuratedApplication(QuarkusBootstrap quarkusBootstrap, CurationResult curationResult,
             ConfiguredClassLoading configuredClassLoading) {
         this.quarkusBootstrap = quarkusBootstrap;
@@ -330,7 +333,9 @@ public QuarkusClassLoader createRuntimeClassLoader(Map<String, byte[]> resources
     public QuarkusClassLoader createRuntimeClassLoader(ClassLoader base, Map<String, byte[]> resources,
             Map<String, byte[]> transformedClasses) {
         QuarkusClassLoader.Builder builder = QuarkusClassLoader
-                .builder("Quarkus Runtime ClassLoader: " + quarkusBootstrap.getMode(),
+                .builder(
+                        "Quarkus Runtime ClassLoader: " + quarkusBootstrap.getMode() + " restart no:"
+                                + runtimeClassLoaderCount.getAndIncrement(),
                         getBaseRuntimeClassLoader(), false)
                 .setAssertionsEnabled(quarkusBootstrap.isAssertionsEnabled())
                 .setAggregateParentResources(true);