Revert "Shutdown reconfiguration thread in a more controlled way [run-systemtest]"

1 files changed, 40 insertions, 45 deletions

diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/ConfiguredApplication.java b/container-disc/src/main/java/com/yahoo/container/jdisc/ConfiguredApplication.java
index 64e94bd5f03..24dcf18b555 100644
--- a/container-disc/src/main/java/com/yahoo/container/jdisc/ConfiguredApplication.java
+++ b/container-disc/src/main/java/com/yahoo/container/jdisc/ConfiguredApplication.java
@@ -94,15 +94,14 @@ public final class ConfiguredApplication implements Application {
                                       new ComponentRegistry<>());
     private final OsgiFramework restrictedOsgiFramework;
     private final Phaser nonTerminatedContainerTracker = new Phaser(1);
-    private final Thread reconfigurerThread;
-    private final Thread portWatcher;
     private HandlersConfigurerDi configurer;
+    private Thread reconfigurerThread;
+    private Thread portWatcher;
     private QrConfig qrConfig;
 
     private Register slobrokRegistrator = null;
     private Supervisor supervisor = null;
     private Acceptor acceptor = null;
-    private volatile boolean shutdownReconfiguration = false;
 
     static {
         LogSetup.initVespaLogging("Container");
@@ -136,8 +135,6 @@ public final class ConfiguredApplication implements Application {
                 : Optional.empty();
         this.restrictedOsgiFramework = new DisableOsgiFramework(new RestrictedBundleContext(osgiFramework.bundleContext()));
         this.shutdownDeadline = new ShutdownDeadline(configId);
-        this.reconfigurerThread = new Thread(this::doReconfigurationLoop, "configured-application-reconfigurer");
-        this.portWatcher = new Thread(this::watchPortChange, "configured-application-port-watcher");
     }
 
     @Override
@@ -149,9 +146,8 @@ public final class ConfiguredApplication implements Application {
         ContainerBuilder builder = createBuilderWithGuiceBindings();
         configurer = createConfigurer(builder.guiceModules().activate());
         initializeAndActivateContainer(builder, () -> {});
-        reconfigurerThread.setDaemon(true);
-        reconfigurerThread.start();
-
+        startReconfigurerThread();
+        portWatcher = new Thread(this::watchPortChange, "configured-application-port-watcher");
         portWatcher.setDaemon(true);
         portWatcher.start();
         if (setupRpc()) {
@@ -290,27 +286,30 @@ public final class ConfiguredApplication implements Application {
     }
 
     @SuppressWarnings("removal") // TODO Vespa 8: remove
-    private void doReconfigurationLoop() {
-        while (!shutdownReconfiguration) {
-            try {
-                ContainerBuilder builder = createBuilderWithGuiceBindings();
-
-                // Block until new config arrives, and it should be applied
-                Runnable cleanupTask = configurer.waitForNextGraphGeneration(builder.guiceModules().activate(), false);
-                initializeAndActivateContainer(builder, cleanupTask);
-            } catch (UncheckedInterruptedException | ConfigInterruptedException e) {
-                break;
-            } catch (Exception | LinkageError e) { // LinkageError: OSGi problems
-                tryReportFailedComponentGraphConstructionMetric(configurer, e);
-                log.log(Level.SEVERE,
-                        "Reconfiguration failed, your application package must be fixed, unless this is a " +
-                                "JNI reload issue: " + Exceptions.toMessageString(e), e);
-            } catch (Error e) {
-                com.yahoo.protect.Process.logAndDie("java.lang.Error on reconfiguration: We are probably in " +
-                        "a bad state and will terminate", e);
+    private void startReconfigurerThread() {
+        reconfigurerThread = new Thread(() -> {
+            while ( ! Thread.interrupted()) {
+                try {
+                    ContainerBuilder builder = createBuilderWithGuiceBindings();
+
+                    // Block until new config arrives, and it should be applied
+                    Runnable cleanupTask = configurer.waitForNextGraphGeneration(builder.guiceModules().activate(), false);
+                    initializeAndActivateContainer(builder, cleanupTask);
+                } catch (UncheckedInterruptedException | ConfigInterruptedException e) {
+                    break;
+                } catch (Exception | LinkageError e) { // LinkageError: OSGi problems
+                    tryReportFailedComponentGraphConstructionMetric(configurer, e);
+                    log.log(Level.SEVERE,
+                            "Reconfiguration failed, your application package must be fixed, unless this is a " +
+                            "JNI reload issue: " + Exceptions.toMessageString(e), e);
+                } catch (Error e) {
+                    com.yahoo.protect.Process.logAndDie("java.lang.Error on reconfiguration: We are probably in " +
+                                                        "a bad state and will terminate", e);
+                }
             }
-        }
-        log.fine("Reconfiguration loop exited");
+            log.fine("Shutting down HandlersConfigurerDi");
+        }, "configured-application-reconfigurer");
+        reconfigurerThread.start();
     }
 
     private static void tryReportFailedComponentGraphConstructionMetric(HandlersConfigurerDi configurer, Throwable error) {
@@ -398,7 +397,7 @@ public final class ConfiguredApplication implements Application {
     }
 
     private void stopServersAndAwaitTermination(String logPrefix) {
-        shutdownReconfigurer();
+        shutdownReconfigurerThread();
         log.info(logPrefix + ": Closing servers");
         startAndStopServers(List.of());
         startAndRemoveClients(List.of());
@@ -408,24 +407,20 @@ public final class ConfiguredApplication implements Application {
         log.info(logPrefix + ": Finished");
     }
 
-    private void shutdownReconfigurer() {
-        if (!reconfigurerThread.isAlive()) {
-            log.info("Reconfiguration thread shutdown already completed");
-            return;
-        }
-        log.info("Shutting down reconfiguration thread");
-        long start = System.currentTimeMillis();
-        shutdownReconfiguration = true;
-        configurer.shutdownConfigRetriever();
+    // TODO Do more graceful shutdown of reconfigurer thread. The interrupt may leave the container in state where
+    //      graceful shutdown is impossible or may hang.
+    private void shutdownReconfigurerThread() {
         try {
-            reconfigurerThread.join();
-            log.info(String.format(
-                    "Reconfiguration shutdown completed in %.3f seconds", (System.currentTimeMillis() - start) / 1000D));
+            //Workaround for component constructors masking InterruptedException.
+            while (reconfigurerThread != null && reconfigurerThread.isAlive()) {
+                reconfigurerThread.interrupt();
+                long millis = 200;
+                reconfigurerThread.join(millis);
+                reconfigurerThread = null;
+            }
         } catch (InterruptedException e) {
-            String message = "Interrupted while waiting for reconfiguration shutdown";
-            log.warning(message);
-            log.log(Level.FINE, e.getMessage(), e);
-            throw new UncheckedInterruptedException(message, true);
+            log.info("Interrupted while joining on HandlersConfigurer reconfigure thread.");
+            Thread.currentThread().interrupt();
         }
     }