Support safe component deconstruction in jdisc container.

- Add allowDuplicates to all Osgi classes
- Uninstall bundles in Deconstructor
- We no longer refresh bundles because we uninstall old bundles at
  a later point than the new bundles are installed. Hence, the
  user must version app bundles that are dependencies used by
  other app bundles.

3 files changed, 39 insertions, 9 deletions

diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/DisableOsgiFramework.java b/container-disc/src/main/java/com/yahoo/container/jdisc/DisableOsgiFramework.java
index 362fefa405d..b81923c5c0a 100644
--- a/container-disc/src/main/java/com/yahoo/container/jdisc/DisableOsgiFramework.java
+++ b/container-disc/src/main/java/com/yahoo/container/jdisc/DisableOsgiFramework.java
@@ -6,6 +6,7 @@ import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.BundleException;
 
+import java.util.Collection;
 import java.util.List;
 
 /**
@@ -52,6 +53,16 @@ public final class DisableOsgiFramework implements OsgiFramework {
     }
 
     @Override
+    public List<Bundle> getBundles(Bundle requestingBundle) {
+        throw newException();
+    }
+
+    @Override
+    public void allowDuplicateBundles(Collection<Bundle> bundles) {
+        throw newException();
+    }
+
+    @Override
     public void start() throws BundleException {
         throw newException();
     }
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 284e3a69b61..371f29e86a3 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
@@ -7,6 +7,8 @@ import com.yahoo.container.di.ComponentDeconstructor;
 import com.yahoo.container.di.componentgraph.Provider;
 import com.yahoo.jdisc.SharedResource;
 import com.yahoo.log.LogLevel;
+import org.osgi.framework.Bundle;
+import org.osgi.framework.BundleException;
 
 import java.time.Duration;
 import java.util.ArrayList;
@@ -17,6 +19,7 @@ import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
 import java.util.logging.Logger;
 
+import static java.util.logging.Level.SEVERE;
 import static java.util.logging.Level.WARNING;
 
 /**
@@ -37,7 +40,7 @@ public class Deconstructor implements ComponentDeconstructor {
     }
 
     @Override
-    public void deconstruct(Collection<Object> components) {
+    public void deconstruct(Collection<Object> components, Collection<Bundle> bundles) {
         Collection<AbstractComponent> destructibleComponents = new ArrayList<>();
         for (var component : components) {
             if (component instanceof AbstractComponent) {
@@ -57,20 +60,24 @@ public class Deconstructor implements ComponentDeconstructor {
             }
         }
         if (! destructibleComponents.isEmpty())
-            executor.schedule(new DestructComponentTask(destructibleComponents), delay.getSeconds(), TimeUnit.SECONDS);
+            executor.schedule(new DestructComponentTask(destructibleComponents, bundles),
+                              delay.getSeconds(), TimeUnit.SECONDS);
     }
 
     private static class DestructComponentTask implements Runnable {
 
         private final Random random = new Random(System.nanoTime());
         private final Collection<AbstractComponent> components;
+        private final Collection<Bundle> bundles;
 
-        DestructComponentTask(Collection<AbstractComponent> components) {
+        DestructComponentTask(Collection<AbstractComponent> components,
+                              Collection<Bundle> bundles) {
             this.components = components;
+            this.bundles = bundles;
         }
 
         /**
-        * Returns a random delay betweeen 0 and 10 minutes which will be different across identical containers invoking this at the same time.
+        * Returns a random delay between 0 and 10 minutes which will be different across identical containers invoking this at the same time.
         * Used to randomize restart to avoid simultaneous cluster restarts.
         */
         private Duration getRandomizedShutdownDelay() {
@@ -80,7 +87,6 @@ public class Deconstructor implements ComponentDeconstructor {
 
         @Override
         public void run() {
-            log.info("Starting deconstruction of " + components.size() + " components");
             for (var component : components) {
                 log.info("Starting deconstruction of component " + component);
                 try {
@@ -102,7 +108,19 @@ public class Deconstructor implements ComponentDeconstructor {
                     log.log(WARNING, "Non-error not exception throwable thrown when deconstructing component  " + component, e);
                 }
             }
-            log.info("Finished deconstructing " + components.size() + " components");
+            // It should now be safe to uninstall the old bundles.
+            for (var bundle : bundles) {
+                try {
+                    log.info("Uninstalling bundle " + bundle);
+                    bundle.uninstall();
+                } catch (BundleException e) {
+                    log.log(SEVERE, "Could not uninstall bundle " + bundle);
+                }
+            }
+            // NOTE: It could be tempting to refresh packages here, but if active bundles were using any of
+            //       the removed ones, they would switch wiring in the middle of a generation's lifespan.
+            //       This would happen if the dependent active bundle has not been rebuilt with a new version
+            //       of its dependency(ies).
         }
     }
 
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 395b1aa7c44..345f75f7eb6 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
@@ -10,6 +10,7 @@ import org.junit.Test;
 
 import java.util.Collections;
 
+import static java.util.Collections.emptyList;
 import static java.util.Collections.singleton;
 import static org.junit.Assert.assertTrue;
 
@@ -28,7 +29,7 @@ public class DeconstructorTest {
     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(singleton(abstractComponent));
+        deconstructor.deconstruct(singleton(abstractComponent), emptyList());
         int cnt = 0;
         while (! abstractComponent.destructed && (cnt++ < 12000)) {
             Thread.sleep(10);
@@ -39,14 +40,14 @@ public class DeconstructorTest {
     @Test
     public void require_provider_destructed() {
         TestProvider provider = new TestProvider();
-        deconstructor.deconstruct(singleton(provider));
+        deconstructor.deconstruct(singleton(provider), emptyList());
         assertTrue(provider.destructed);
     }
 
     @Test
     public void require_shared_resource_released() {
         TestSharedResource sharedResource = new TestSharedResource();
-        deconstructor.deconstruct(singleton(sharedResource));
+        deconstructor.deconstruct(singleton(sharedResource), emptyList());
         assertTrue(sharedResource.released);
     }