Move shutdown timeout logic to separate class

5 files changed, 85 insertions, 47 deletions

diff --git a/container-disc/pom.xml b/container-disc/pom.xml
index 0d602e9e730..1c3115900de 100644
--- a/container-disc/pom.xml
+++ b/container-disc/pom.xml
@@ -21,6 +21,16 @@
       <scope>test</scope>
     </dependency>
     <dependency>
+      <groupId>org.junit.jupiter</groupId>
+      <artifactId>junit-jupiter</artifactId>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.junit.vintage</groupId>
+      <artifactId>junit-vintage-engine</artifactId>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
       <groupId>com.yahoo.vespa</groupId>
       <artifactId>testutil</artifactId>
       <version>${project.version}</version>
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 d686bb9a3dd..1d50686680d 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
@@ -8,7 +8,6 @@ import com.google.inject.Injector;
 import com.yahoo.cloud.config.SlobroksConfig;
 import com.yahoo.component.Vtag;
 import com.yahoo.component.provider.ComponentRegistry;
-import com.yahoo.concurrent.DaemonThreadFactory;
 import com.yahoo.config.ConfigInstance;
 import com.yahoo.config.subscription.ConfigInterruptedException;
 import com.yahoo.container.Container;
@@ -43,7 +42,6 @@ import com.yahoo.log.LogSetup;
 import com.yahoo.messagebus.network.rpc.SlobrokConfigSubscriber;
 import com.yahoo.net.HostName;
 import com.yahoo.vespa.config.ConfigKey;
-import com.yahoo.vespa.defaults.Defaults;
 import com.yahoo.yolean.Exceptions;
 import com.yahoo.yolean.UncheckedInterruptedException;
 
@@ -56,8 +54,6 @@ import java.util.Optional;
 import java.util.Set;
 import java.util.WeakHashMap;
 import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.ScheduledThreadPoolExecutor;
-import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -71,7 +67,6 @@ public final class ConfiguredApplication implements Application {
 
     private static final Logger log = Logger.getLogger(ConfiguredApplication.class.getName());
     private static final Set<ClientProvider> startedClients = Collections.newSetFromMap(new WeakHashMap<>());
-    static final String SANITIZE_FILENAME = "[/,;]";
 
     private static final Set<ServerProvider> startedServers = Collections.newSetFromMap(new IdentityHashMap<>());
     private final SubscriberFactory subscriberFactory;
@@ -86,6 +81,7 @@ public final class ConfiguredApplication implements Application {
     // to config to make sure that container will be registered in slobrok (by {@link com.yahoo.jrt.slobrok.api.Register})
     // if slobrok config changes (typically slobroks moving to other nodes)
     private final Optional<SlobrokConfigSubscriber> slobrokConfigSubscriber;
+    private final ShutdownDeadline shutdownDeadline;
 
     //TODO: FilterChainRepository should instead always be set up in the model.
     private final FilterChainRepository defaultFilterChainRepository =
@@ -96,7 +92,6 @@ public final class ConfiguredApplication implements Application {
                                       new ComponentRegistry<>());
     private final OsgiFramework restrictedOsgiFramework;
     private HandlersConfigurerDi configurer;
-    private ScheduledThreadPoolExecutor shutdownDeadlineExecutor;
     private Thread reconfigurerThread;
     private Thread portWatcher;
     private QrConfig qrConfig;
@@ -136,6 +131,7 @@ public final class ConfiguredApplication implements Application {
                 ? Optional.of(new SlobrokConfigSubscriber(configId))
                 : Optional.empty();
         this.restrictedOsgiFramework = new DisableOsgiFramework(new RestrictedBundleContext(osgiFramework.bundleContext()));
+        this.shutdownDeadline = new ShutdownDeadline(configId);
     }
 
     @Override
@@ -371,9 +367,8 @@ public final class ConfiguredApplication implements Application {
 
     @Override
     public void stop() {
-        startShutdownDeadlineExecutor();
+        shutdownDeadline.schedule((long)(shutdownTimeoutS.get() * 1000), dumpHeapOnShutdownTimeout.get());
         shutdownReconfigurerThread();
-
         log.info("Stop: Closing servers");
         for (ServerProvider server : Container.get().getServerProviderRegistry().allComponents()) {
             if (startedServers.contains(server)) {
@@ -418,31 +413,8 @@ public final class ConfiguredApplication implements Application {
 
     @Override
     public void destroy() {
-        if (shutdownDeadlineExecutor != null) { //stop() is not called when exception happens during start
-            shutdownDeadlineExecutor.shutdownNow();
-        }
-    }
-
-    static String santizeFileName(String s) {
-        return s.trim()
-                .replace('\\', '.')
-                .replaceAll(SANITIZE_FILENAME, ".");
-    }
-
-    // Workaround for ApplicationLoader.stop not being able to shutdown
-    private void startShutdownDeadlineExecutor() {
-        shutdownDeadlineExecutor = new ScheduledThreadPoolExecutor(1, new DaemonThreadFactory("Shutdown deadline timer"));
-        shutdownDeadlineExecutor.setExecuteExistingDelayedTasksAfterShutdownPolicy(false);
-        long delayMillis = (long)(shutdownTimeoutS.get() * 1000.0);
-        shutdownDeadlineExecutor.schedule(() -> {
-            if (dumpHeapOnShutdownTimeout.get()) {
-                String heapDumpName = Defaults.getDefaults().underVespaHome("var/crash/java_pid.") + santizeFileName(configId) + "." + ProcessHandle.current().pid() + ".hprof";
-                com.yahoo.protect.Process.dumpHeap(heapDumpName, true);
-            }
-            com.yahoo.protect.Process.logAndDie(
-                    "Timed out waiting for application shutdown. Please check that all your request handlers " +
-                            "drain their request content channels.", true);
-        }, delayMillis, TimeUnit.MILLISECONDS);
+        shutdownDeadline.cancel();
+        log.info("Destroy: completed");
     }
 
     private static void addHandlerBindings(ContainerBuilder builder,
diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/ShutdownDeadline.java b/container-disc/src/main/java/com/yahoo/container/jdisc/ShutdownDeadline.java
new file mode 100644
index 00000000000..15af216c210
--- /dev/null
+++ b/container-disc/src/main/java/com/yahoo/container/jdisc/ShutdownDeadline.java
@@ -0,0 +1,52 @@
+// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.container.jdisc;
+
+import com.yahoo.concurrent.DaemonThreadFactory;
+import com.yahoo.vespa.defaults.Defaults;
+
+import java.util.concurrent.ScheduledThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+
+import static com.yahoo.protect.Process.dumpHeap;
+import static com.yahoo.protect.Process.logAndDie;
+
+/**
+ * Kills the JVM if the application is unable to shutdown before deadline.
+ *
+ * @author bjorncs
+ */
+class ShutdownDeadline implements AutoCloseable {
+
+    private final String configId;
+    private final ScheduledThreadPoolExecutor executor;
+
+    ShutdownDeadline(String configId) {
+        this.configId = configId;
+        executor = new ScheduledThreadPoolExecutor(1, new DaemonThreadFactory("Shutdown deadline timer"));
+        executor.setExecuteExistingDelayedTasksAfterShutdownPolicy(false);
+    }
+
+    ShutdownDeadline schedule(long millis, boolean heapdumpOnShutdown) {
+        executor.schedule(() -> onDeadline(heapdumpOnShutdown), millis, TimeUnit.MILLISECONDS);
+        return this;
+    }
+
+    void cancel() { executor.shutdownNow(); }
+    @Override public void close() { cancel();  }
+
+    private void onDeadline(boolean heapdumpOnShutdown) {
+        if (heapdumpOnShutdown) dumpHeap(heapdumpFilename(), true);
+        logAndDie("Timed out waiting for application shutdown. Please check that all your request handlers " +
+                "drain their request content channels.", true);
+    }
+
+    private String heapdumpFilename() {
+        return Defaults.getDefaults().underVespaHome("var/crash/java_pid.") + sanitizeFileName(configId) + "."
+                + ProcessHandle.current().pid() + ".hprof";
+    }
+
+    static String sanitizeFileName(String s) {
+        return s.trim().replace('\\', '.').replaceAll("[/,;]", ".");
+    }
+
+}
diff --git a/container-disc/src/test/java/com/yahoo/container/jdisc/ConfiguredApplicationTest.java b/container-disc/src/test/java/com/yahoo/container/jdisc/ConfiguredApplicationTest.java
deleted file mode 100644
index c8cf5c0ce63..00000000000
--- a/container-disc/src/test/java/com/yahoo/container/jdisc/ConfiguredApplicationTest.java
+++ /dev/null
@@ -1,14 +0,0 @@
-// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.container.jdisc;
-
-import org.junit.Test;
-
-import static org.junit.Assert.assertEquals;
-
-public class ConfiguredApplicationTest {
-    @Test
-    public void testConfigId2FileName() {
-        assertEquals("admin.metrics.2088223-v6-1.ostk.bm2.prod.ne1.yahoo.com", ConfiguredApplication.santizeFileName("admin/metrics/2088223-v6-1.ostk.bm2.prod.ne1.yahoo.com"));
-        assertEquals("admin.standalone.cluster-controllers.1", ConfiguredApplication.santizeFileName("admin/standalone/cluster-controllers/1 "));
-    }
-}
diff --git a/container-disc/src/test/java/com/yahoo/container/jdisc/ShutdownDeadlineTest.java b/container-disc/src/test/java/com/yahoo/container/jdisc/ShutdownDeadlineTest.java
new file mode 100644
index 00000000000..244c9f528c5
--- /dev/null
+++ b/container-disc/src/test/java/com/yahoo/container/jdisc/ShutdownDeadlineTest.java
@@ -0,0 +1,18 @@
+package com.yahoo.container.jdisc;// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+
+import org.junit.jupiter.api.Test;
+
+import static com.yahoo.container.jdisc.ShutdownDeadline.sanitizeFileName;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+/**
+ * @author bjorncs
+ */
+class ShutdownDeadlineTest {
+    @Test
+    void testConfigId2FileName() {
+        assertEquals("admin.metrics.2088223-v6-1.ostk.bm2.prod.ne1.yahoo.com", sanitizeFileName("admin/metrics/2088223-v6-1.ostk.bm2.prod.ne1.yahoo.com"));
+        assertEquals("admin.standalone.cluster-controllers.1", sanitizeFileName("admin/standalone/cluster-controllers/1 "));
+    }
+}
\ No newline at end of file