Wait for component deconstruction to finish when shutting down.

- Wait up to 10 minuts.
- Add separate modes for reconfig and shutdown.
- Add test ctor for setting the delay before deconstruction starts
  in RECONFIG mode. (Tests are TBD.)

3 files changed, 65 insertions, 8 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 835326e0904..f266e3782ef 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
@@ -335,7 +335,7 @@ public final class ConfiguredApplication implements Application {
         return new HandlersConfigurerDi(subscriberFactory,
                                         Container.get(),
                                         configId,
-                                        new Deconstructor(true),
+                                        new Deconstructor(Deconstructor.Mode.RECONFIG),
                                         discInjector,
                                         osgiFramework);
     }
@@ -367,7 +367,7 @@ public final class ConfiguredApplication implements Application {
         }
 
         log.info("Stop: Shutting container down");
-        configurer.shutdown(new Deconstructor(false));
+        configurer.shutdown(new Deconstructor(Deconstructor.Mode.SHUTDOWN));
         slobrokConfigSubscriber.ifPresent(SlobrokConfigSubscriber::shutdown);
         Container.get().shutdown();
 
diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/component/Deconstructor.java b/container-disc/src/main/java/com/yahoo/container/jdisc/component/Deconstructor.java
index 0bb60c8aca8..47e36809a48 100644
--- a/container-disc/src/main/java/com/yahoo/container/jdisc/component/Deconstructor.java
+++ b/container-disc/src/main/java/com/yahoo/container/jdisc/component/Deconstructor.java
@@ -9,6 +9,10 @@ import com.yahoo.container.di.componentgraph.Provider;
 import com.yahoo.jdisc.SharedResource;
 
 import java.util.List;
+
+
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeoutException;
 import java.util.logging.Level;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleException;
@@ -35,13 +39,29 @@ public class Deconstructor implements ComponentDeconstructor {
 
     private static final Logger log = Logger.getLogger(Deconstructor.class.getName());
 
+    private static final Duration SHUTDOWN_DECONSTRUCT_TIMEOUT = Duration.ofMinutes(10);
+
+    public enum Mode {
+        RECONFIG,  // Delay deconstruction to allow old components to finish processing in-flight requests.
+        SHUTDOWN   // The container is shutting down. Start deconstructing immediately, and wait until all components
+                   // are deconstructed, to prevent shutting down while deconstruct is in progress.
+    }
+
+    // TODO: make private again
     final ScheduledExecutorService executor =
-            Executors.newScheduledThreadPool(1, ThreadFactoryFactory.getThreadFactory("component-deconstructor"));
+            Executors.newScheduledThreadPool(2, ThreadFactoryFactory.getThreadFactory("component-deconstructor"));
 
+    private final Mode mode;
     private final Duration delay;
 
-    public Deconstructor(boolean delayDeconstruction) {
-        this.delay = delayDeconstruction ? Duration.ofSeconds(60) : Duration.ZERO;
+    public Deconstructor(Mode mode) {
+        this(mode, (mode == Mode.RECONFIG) ? Duration.ofSeconds(60) : Duration.ZERO);
+    }
+
+    // For testing only
+    Deconstructor(Mode mode, Duration reconfigDeconstructDelay) {
+        this.mode = mode;
+        this.delay = reconfigDeconstructDelay;
     }
 
     @Override
@@ -61,9 +81,24 @@ public class Deconstructor implements ComponentDeconstructor {
                 ((SharedResource) component).release();
             }
         }
-        if (! destructibleComponents.isEmpty() || ! bundles.isEmpty())
-            executor.schedule(new DestructComponentTask(destructibleComponents, bundles),
+        if (!destructibleComponents.isEmpty() || !bundles.isEmpty()) {
+            var task = executor.schedule(new DestructComponentTask(destructibleComponents, bundles),
                               delay.getSeconds(), TimeUnit.SECONDS);
+            if (mode.equals(Mode.SHUTDOWN)) {
+                // Wait for deconstruction to finish
+                try {
+                    log.info("Waiting up to " + SHUTDOWN_DECONSTRUCT_TIMEOUT.toSeconds() + " seconds for all components to deconstruct.");
+                    task.get(SHUTDOWN_DECONSTRUCT_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS);
+                } catch (InterruptedException e) {
+                    log.info("Interrupted while waiting for component deconstruction to finish.");
+                    Thread.currentThread().interrupt();
+                } catch (ExecutionException e) {
+                    log.warning("Component deconstruction threw an exception: " + e.getMessage());
+                } catch (TimeoutException e) {
+                    log.warning("Component deconstruction timed out.");
+                }
+            }
+        }
     }
 
     private static class DestructComponentTask implements Runnable {
diff --git a/container-disc/src/test/java/com/yahoo/container/jdisc/component/DeconstructorTest.java b/container-disc/src/test/java/com/yahoo/container/jdisc/component/DeconstructorTest.java
index 9ae05f5c193..dfc1d32938f 100644
--- a/container-disc/src/test/java/com/yahoo/container/jdisc/component/DeconstructorTest.java
+++ b/container-disc/src/test/java/com/yahoo/container/jdisc/component/DeconstructorTest.java
@@ -24,12 +24,13 @@ public class DeconstructorTest {
 
     @Before
     public void init() {
-        deconstructor = new Deconstructor(false);
+        deconstructor = new Deconstructor(Deconstructor.Mode.SHUTDOWN);
     }
 
     @Test
     public void require_abstract_component_destructed() throws InterruptedException {
         TestAbstractComponent abstractComponent = new TestAbstractComponent();
+
         // Done by executor, so it takes some time even with a 0 delay.
         deconstructor.deconstruct(List.of(abstractComponent), emptyList());
         deconstructor.executor.shutdown();
@@ -47,6 +48,13 @@ public class DeconstructorTest {
     }
 
     @Test
+    public void deconstruct_is_synchronous_in_shutdown_mode() {
+        var slowDeconstructComponent = new SlowDeconstructComponent();
+        deconstructor.deconstruct(List.of(slowDeconstructComponent), emptyList());
+        assertTrue(slowDeconstructComponent.destructed);
+    }
+
+    @Test
     public void require_shared_resource_released() {
         TestSharedResource sharedResource = new TestSharedResource();
         deconstructor.deconstruct(List.of(sharedResource), emptyList());
@@ -68,6 +76,20 @@ public class DeconstructorTest {
         @Override public void deconstruct() { destructed = true; }
     }
 
+    private static class SlowDeconstructComponent extends AbstractComponent {
+        boolean destructed = false;
+        @Override
+        public void deconstruct() {
+            // Add delay to verify that the Deconstructor waits until this is complete before returning.
+            try {
+                Thread.sleep(1000);
+            } catch (InterruptedException e) {
+                throw new RuntimeException("The delayed deconstruct was interrupted.");
+            }
+            destructed = true;
+        }
+    }
+
     private static class TestProvider implements Provider<Void> {
         volatile boolean destructed = false;