Reapply "Gjoranv/allow duplicate bundles"

This reverts commit 2ef1e922a1d845b3cd79e9fb329925e7e9896919.

8 files changed, 181 insertions, 64 deletions

diff --git a/container-core/src/main/java/com/yahoo/container/core/config/BundleLoader.java b/container-core/src/main/java/com/yahoo/container/core/config/BundleLoader.java
index 2b3a272fadc..4b8f21469d1 100644
--- a/container-core/src/main/java/com/yahoo/container/core/config/BundleLoader.java
+++ b/container-core/src/main/java/com/yahoo/container/core/config/BundleLoader.java
@@ -10,7 +10,6 @@ import org.osgi.framework.Bundle;
 import org.osgi.framework.wiring.BundleRevision;
 
 import java.io.File;
-import java.util.Arrays;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -26,11 +25,18 @@ import static com.yahoo.container.core.BundleLoaderProperties.DISK_BUNDLE_PREFIX
  * Manages the set of installed 3rd-party component bundles.
  *
  * @author Tony Vaagenes
+ * @author gjoranv
  */
 public class BundleLoader {
 
-    private final List<Bundle> initialBundles;
-
+    /* Map of file refs of active bundles (not scheduled for uninstall) to a list of all bundles that were installed
+     * (pre-install directive) by the bundle pointed to by the file ref (including itself).
+     *
+     * Used to:
+     * 1. Avoid installing already installed bundles. Just an optimization, installing the same bundle location is a NOP
+     * 2. Start bundles (all are started every time)
+     * 3. Calculate the set of bundles to uninstall
+     */
     private final Map<FileReference, List<Bundle>> reference2Bundles = new LinkedHashMap<>();
 
     private final Logger log = Logger.getLogger(BundleLoader.class.getName());
@@ -38,7 +44,6 @@ public class BundleLoader {
 
     public BundleLoader(Osgi osgi) {
         this.osgi = osgi;
-        initialBundles = Arrays.asList(osgi.getBundles());
     }
 
     private List<Bundle> obtainBundles(FileReference reference, FileAcquirer fileAcquirer) throws InterruptedException {
@@ -46,17 +51,19 @@ public class BundleLoader {
         return osgi.install(file.getAbsolutePath());
     }
 
-    /** Returns the number of bundles installed by this call. */
-    private int install(List<FileReference> references) {
+    private void install(List<FileReference> references) {
         Set<FileReference> bundlesToInstall = new HashSet<>(references);
+
+        // This is just an optimization, as installing a bundle with the same location id returns the already installed bundle.
+        // It's ok that fileRefs pending uninstall are removed from the map, because they are never in the new set of bundles..
         bundlesToInstall.removeAll(reference2Bundles.keySet());
 
         PredicateSplit<FileReference> bundlesToInstall_isDisk = partition(bundlesToInstall, BundleLoader::isDiskBundle);
         installBundlesFromDisk(bundlesToInstall_isDisk.trueValues);
         installBundlesFromFileDistribution(bundlesToInstall_isDisk.falseValues);
 
+        // TODO: Remove. Bundles are also started in use()
         startBundles();
-        return bundlesToInstall.size();
     }
 
     private static boolean isDiskBundle(FileReference fileReference) {
@@ -97,6 +104,7 @@ public class BundleLoader {
         }
 
         List<Bundle> bundles = osgi.install(file.getAbsolutePath());
+
         reference2Bundles.put(reference, bundles);
     }
 
@@ -113,13 +121,16 @@ public class BundleLoader {
         }
     }
 
-    // All bundles must have been started first to ensure correct package resolution.
+    /**
+     * Resolves and starts (calls the Bundles BundleActivator) all bundles. Bundle resolution must take place
+     * after all bundles are installed to ensure that the framework can resolve dependencies between bundles.
+     */
     private void startBundles() {
         for (List<Bundle> bundles : reference2Bundles.values()) {
             for (Bundle bundle : bundles) {
                 try {
                     if ( ! isFragment(bundle))
-                        bundle.start();
+                        bundle.start();  // NOP for already ACTIVE bundles
                 } catch(Exception e) {
                     throw new RuntimeException("Could not start bundle '" + bundle.getSymbolicName() + "'", e);
                 }
@@ -135,38 +146,44 @@ public class BundleLoader {
         return (bundleRevision.getTypes() & BundleRevision.TYPE_FRAGMENT) != 0;
     }
 
-    /** Returns the number of uninstalled bundles */
-    private int retainOnly(List<FileReference> newReferences) {
-        Set<Bundle> bundlesToRemove = new HashSet<>(Arrays.asList(osgi.getBundles()));
+    /**
+     * Returns the bundles to schedule for uninstall after their components have been deconstructed
+     * and removes the same bundles from the map of active bundles.
+     */
+    private Set<Bundle> getBundlesToUninstall(List<FileReference> newReferences) {
+        Set<Bundle> bundlesToRemove = new HashSet<>(osgi.getCurrentBundles());
 
         for (FileReference fileReferenceToKeep: newReferences) {
             if (reference2Bundles.containsKey(fileReferenceToKeep))
                 bundlesToRemove.removeAll(reference2Bundles.get(fileReferenceToKeep));
         }
 
-        bundlesToRemove.removeAll(initialBundles);
-        for (Bundle bundle : bundlesToRemove) {
-            log.info("Removing bundle '" + bundle.toString() + "'");
-            osgi.uninstall(bundle);
-        }
+        bundlesToRemove.removeAll(osgi.getInitialBundles());
+        removeInactiveFileReferences(newReferences);
 
+        return bundlesToRemove;
+    }
+
+    private void removeInactiveFileReferences(List<FileReference> newReferences) {
+        // Clean up the map of active bundles
         Set<FileReference> fileReferencesToRemove = new HashSet<>(reference2Bundles.keySet());
         fileReferencesToRemove.removeAll(newReferences);
-
-        for (FileReference fileReferenceToRemove : fileReferencesToRemove) {
-            reference2Bundles.remove(fileReferenceToRemove);
-        }
-        return bundlesToRemove.size();
+        fileReferencesToRemove.forEach(reference2Bundles::remove);
     }
 
-    public synchronized int use(List<FileReference> bundles) {
-        int removedBundles = retainOnly(bundles);
-        int installedBundles = install(bundles);
+    /**
+     * Installs the given set of bundles and returns the set of bundles that is no longer used
+     * by the application, and should therefore be scheduled for uninstall.
+     */
+    public synchronized Set<Bundle> use(List<FileReference> newBundles) {
+        Set<Bundle> bundlesToUninstall = getBundlesToUninstall(newBundles);
+        osgi.allowDuplicateBundles(bundlesToUninstall);
+        log.info(() -> bundlesToUninstall.isEmpty() ? "Adding bundles to allowed duplicates: " + bundlesToUninstall : "");
+        install(newBundles);
         startBundles();
 
-        log.info(removedBundles + " bundles were removed, and " + installedBundles + " bundles were installed.");
         log.info(installedBundlesMessage());
-        return removedBundles + installedBundles;
+        return bundlesToUninstall;
     }
 
     private String installedBundlesMessage() {
diff --git a/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java b/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java
index dd8b2605820..ef132694e10 100644
--- a/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java
+++ b/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java
@@ -9,6 +9,7 @@ import com.yahoo.component.ComponentSpecification;
 import com.yahoo.component.provider.ComponentRegistry;
 import com.yahoo.concurrent.ThreadFactoryFactory;
 import com.yahoo.config.FileReference;
+import com.yahoo.container.core.config.testutil.MockOsgiWrapper;
 import com.yahoo.container.di.ComponentDeconstructor;
 import com.yahoo.container.di.Container;
 import com.yahoo.container.di.componentgraph.core.ComponentGraph;
@@ -20,10 +21,9 @@ import com.yahoo.jdisc.application.OsgiFramework;
 import com.yahoo.jdisc.handler.RequestHandler;
 import com.yahoo.jdisc.service.ClientProvider;
 import com.yahoo.jdisc.service.ServerProvider;
-import com.yahoo.language.Linguistics;
-import com.yahoo.language.simple.SimpleLinguistics;
 import com.yahoo.log.LogLevel;
 import com.yahoo.osgi.OsgiImpl;
+import com.yahoo.osgi.OsgiWrapper;
 import com.yahoo.statistics.Statistics;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.wiring.BundleWiring;
@@ -81,19 +81,30 @@ public class HandlersConfigurerDi {
                                 Injector discInjector,
                                 OsgiFramework osgiFramework) {
 
-        this.vespaContainer = vespaContainer;
-        osgiWrapper = new OsgiWrapper(osgiFramework, new BundleLoader(new OsgiImpl(osgiFramework)));
+        this(subscriberFactory, vespaContainer, configId, deconstructor, discInjector,
+             new ContainerAndDiOsgi(osgiFramework, new BundleLoader(new OsgiImpl(osgiFramework))));
+    }
+
+    // Only public for testing
+    public HandlersConfigurerDi(SubscriberFactory subscriberFactory,
+                                com.yahoo.container.Container vespaContainer,
+                                String configId,
+                                ComponentDeconstructor deconstructor,
+                                Injector discInjector,
+                                OsgiWrapper osgiWrapper) {
 
+        this.vespaContainer = vespaContainer;
+        this.osgiWrapper = osgiWrapper;
         container = new Container(subscriberFactory, configId, deconstructor, osgiWrapper);
         getNewComponentGraph(discInjector, false);
     }
 
-    private static class OsgiWrapper extends OsgiImpl implements com.yahoo.container.di.Osgi {
+    private static class ContainerAndDiOsgi extends OsgiImpl implements OsgiWrapper {
 
         private final OsgiFramework osgiFramework;
         private final BundleLoader bundleLoader;
 
-        public OsgiWrapper(OsgiFramework osgiFramework, BundleLoader bundleLoader) {
+        public ContainerAndDiOsgi(OsgiFramework osgiFramework, BundleLoader bundleLoader) {
             super(osgiFramework);
             this.osgiFramework = osgiFramework;
             this.bundleLoader = bundleLoader;
@@ -121,14 +132,9 @@ public class HandlersConfigurerDi {
         }
 
         @Override
-        public void useBundles(Collection<FileReference> bundles) {
+        public Set<Bundle> useBundles(Collection<FileReference> bundles) {
             log.info("Installing bundles from the latest application");
-
-            int bundlesRemovedOrInstalled = bundleLoader.use(new ArrayList<>(bundles));
-
-            if (bundlesRemovedOrInstalled > 0) {
-                refreshPackages();
-            }
+            return bundleLoader.use(new ArrayList<>(bundles));
         }
     }
 
diff --git a/container-core/src/main/java/com/yahoo/container/core/config/testutil/HandlersConfigurerTestWrapper.java b/container-core/src/main/java/com/yahoo/container/core/config/testutil/HandlersConfigurerTestWrapper.java
index 684a45aeac1..9f49b016b68 100644
--- a/container-core/src/main/java/com/yahoo/container/core/config/testutil/HandlersConfigurerTestWrapper.java
+++ b/container-core/src/main/java/com/yahoo/container/core/config/testutil/HandlersConfigurerTestWrapper.java
@@ -19,7 +19,6 @@ import com.yahoo.osgi.MockOsgi;
 
 import java.io.File;
 import java.io.IOException;
-import java.util.Collection;
 import java.util.LinkedHashSet;
 import java.util.Random;
 import java.util.Set;
@@ -90,7 +89,7 @@ public class HandlersConfigurerTestWrapper {
 
     public HandlersConfigurerTestWrapper(Container container, String configId) {
         createFiles(configId);
-        MockOsgi mockOsgi = new MockOsgi();
+        MockOsgiWrapper mockOsgiWrapper = new MockOsgiWrapper();
         ComponentDeconstructor testDeconstructor = getTestDeconstructor();
         configurer = new HandlersConfigurerDi(
                 new CloudSubscriberFactory(configSources),
@@ -98,16 +97,17 @@ public class HandlersConfigurerTestWrapper {
                 configId,
                 testDeconstructor,
                 guiceInjector(),
-                mockOsgi);
+                mockOsgiWrapper);
         this.container = container;
     }
 
     private ComponentDeconstructor getTestDeconstructor() {
-        return components -> components.forEach(component -> {
+        return (components, bundles) -> components.forEach(component -> {
             if (component instanceof AbstractComponent) {
                 AbstractComponent abstractComponent = (AbstractComponent) component;
                 if (abstractComponent.isDeconstructable()) abstractComponent.deconstruct();
             }
+            if (! bundles.isEmpty()) throw new IllegalArgumentException("This test should not use bundles");
         });
     }
 
diff --git a/container-core/src/main/java/com/yahoo/container/core/config/testutil/MockOsgiWrapper.java b/container-core/src/main/java/com/yahoo/container/core/config/testutil/MockOsgiWrapper.java
new file mode 100644
index 00000000000..a1f564f5682
--- /dev/null
+++ b/container-core/src/main/java/com/yahoo/container/core/config/testutil/MockOsgiWrapper.java
@@ -0,0 +1,44 @@
+package com.yahoo.container.core.config.testutil;
+
+import com.yahoo.component.ComponentSpecification;
+import com.yahoo.osgi.OsgiWrapper;
+import org.osgi.framework.Bundle;
+
+import java.util.Collection;
+import java.util.List;
+
+import static java.util.Collections.emptyList;
+
+/**
+ * @author gjoranv
+ */
+public class MockOsgiWrapper implements OsgiWrapper {
+
+    @Override
+    public List<Bundle> getInitialBundles() {
+        return emptyList();
+    }
+
+    @Override
+    public Bundle[] getBundles() {
+        return new Bundle[0];
+    }
+
+    @Override
+    public List<Bundle> getCurrentBundles() {
+        return emptyList();
+    }
+
+    @Override
+    public Bundle getBundle(ComponentSpecification bundleId) {
+        return null;
+    }
+
+    @Override
+    public List<Bundle> install(String absolutePath) {
+        return emptyList();
+    }
+
+    @Override
+    public void allowDuplicateBundles(Collection<Bundle> bundles) {  }
+}
diff --git a/container-core/src/main/java/com/yahoo/osgi/MockOsgi.java b/container-core/src/main/java/com/yahoo/osgi/MockOsgi.java
index 45ad02d2cef..d809c493565 100644
--- a/container-core/src/main/java/com/yahoo/osgi/MockOsgi.java
+++ b/container-core/src/main/java/com/yahoo/osgi/MockOsgi.java
@@ -16,26 +16,28 @@ import java.util.List;
 public class MockOsgi extends NonWorkingOsgiFramework implements Osgi {
 
     @Override
-    public Bundle[] getBundles() {
-        return new Bundle[0];
+    public List<Bundle> getInitialBundles() {
+        return Collections.emptyList();
     }
 
     @Override
-    public Bundle getBundle(ComponentSpecification bundleId) {
-        return null;
+    public Bundle[] getBundles() {
+        return new Bundle[0];
     }
 
     @Override
-    public List<Bundle> install(String absolutePath) {
+    public List<Bundle> getCurrentBundles() {
         return Collections.emptyList();
     }
 
     @Override
-    public void uninstall(Bundle bundle) {
+    public Bundle getBundle(ComponentSpecification bundleId) {
+        return null;
     }
 
     @Override
-    public void refreshPackages() {
+    public List<Bundle> install(String absolutePath) {
+        return Collections.emptyList();
     }
 
 }
diff --git a/container-core/src/main/java/com/yahoo/osgi/Osgi.java b/container-core/src/main/java/com/yahoo/osgi/Osgi.java
index c94eaf43deb..8f0acf41f30 100644
--- a/container-core/src/main/java/com/yahoo/osgi/Osgi.java
+++ b/container-core/src/main/java/com/yahoo/osgi/Osgi.java
@@ -4,6 +4,7 @@ package com.yahoo.osgi;
 import com.yahoo.component.ComponentSpecification;
 import org.osgi.framework.Bundle;
 
+import java.util.Collection;
 import java.util.List;
 
 /**
@@ -11,14 +12,17 @@ import java.util.List;
  */
 public interface Osgi {
 
+    List<Bundle> getInitialBundles();
+
     Bundle[] getBundles();
 
+    /** Returns all bundles that have not been scheduled for uninstall. */
+    List<Bundle> getCurrentBundles();
+
     Bundle getBundle(ComponentSpecification bundleId);
 
     List<Bundle> install(String absolutePath);
 
-    void uninstall(Bundle bundle);
-
-    void refreshPackages();
+    void allowDuplicateBundles(Collection<Bundle> bundles);
 
 }
diff --git a/container-core/src/main/java/com/yahoo/osgi/OsgiImpl.java b/container-core/src/main/java/com/yahoo/osgi/OsgiImpl.java
index 8b2f20a1c13..ed93d15c975 100644
--- a/container-core/src/main/java/com/yahoo/osgi/OsgiImpl.java
+++ b/container-core/src/main/java/com/yahoo/osgi/OsgiImpl.java
@@ -7,19 +7,43 @@ import com.yahoo.container.bundle.BundleInstantiationSpecification;
 import com.yahoo.jdisc.application.OsgiFramework;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleException;
+import org.osgi.framework.launch.Framework;
 
+import java.util.Collection;
 import java.util.List;
+import java.util.logging.Logger;
 
 /**
  * @author Tony Vaagenes
  * @author bratseth
  */
 public class OsgiImpl implements Osgi {
+    private static final Logger log = Logger.getLogger(OsgiImpl.class.getName());
 
     private final OsgiFramework jdiscOsgi;
 
+    // The initial bundles are never scheduled for uninstall
+    private final List<Bundle> initialBundles;
+
+    // An initial bundle that is not the framework, and can hence be used to look up current bundles
+    private final Bundle alwaysCurrentBundle;
+
     public OsgiImpl(OsgiFramework jdiscOsgi) {
         this.jdiscOsgi = jdiscOsgi;
+
+        this.initialBundles = jdiscOsgi.bundles();
+        if (initialBundles.isEmpty())
+            throw new IllegalStateException("No initial bundles!");
+
+        alwaysCurrentBundle = firstNonFrameworkBundle(initialBundles);
+        if (alwaysCurrentBundle == null)
+            throw new IllegalStateException("The initial bundles only contained the framework bundle!");
+        log.info("Using " + alwaysCurrentBundle + " to lookup current bundles.");
+    }
+
+    @Override
+    public List<Bundle> getInitialBundles() {
+        return initialBundles;
     }
 
     @Override
@@ -28,6 +52,10 @@ public class OsgiImpl implements Osgi {
         return bundles.toArray(new Bundle[bundles.size()]);
     }
 
+    @Override
+    public List<Bundle> getCurrentBundles() {
+        return jdiscOsgi.getBundles(alwaysCurrentBundle);
+    }
 
     public Class<Object> resolveClass(BundleInstantiationSpecification spec) {
         Bundle bundle = getBundle(spec.bundle);
@@ -86,8 +114,9 @@ public class OsgiImpl implements Osgi {
      * @return the bundle match having the highest version, or null if there was no matches
      */
     public Bundle getBundle(ComponentSpecification id) {
+        log.fine(() -> "Getting bundle for component " + id + ". Set of current bundles: " + getCurrentBundles());
         Bundle highestMatch = null;
-        for (Bundle bundle : getBundles()) {
+        for (Bundle bundle : getCurrentBundles()) {
             assert bundle.getSymbolicName() != null : "ensureHasBundleSymbolicName not called during installation";
 
             if ( ! bundle.getSymbolicName().equals(id.getName())) continue;
@@ -122,17 +151,16 @@ public class OsgiImpl implements Osgi {
     }
 
     @Override
-    public void uninstall(Bundle bundle) {
-        try {
-            bundle.uninstall();
-        } catch (BundleException e) {
-            throw new RuntimeException(e);
-        }
+    public void allowDuplicateBundles(Collection<Bundle> bundles) {
+        jdiscOsgi.allowDuplicateBundles(bundles);
     }
 
-    @Override
-    public void refreshPackages() {
-        jdiscOsgi.refreshPackages();
+    private static Bundle firstNonFrameworkBundle(List<Bundle> bundles) {
+        for (Bundle b : bundles) {
+            if (! (b instanceof Framework))
+                return b;
+        }
+        return null;
     }
 
 }
diff --git a/container-core/src/main/java/com/yahoo/osgi/OsgiWrapper.java b/container-core/src/main/java/com/yahoo/osgi/OsgiWrapper.java
new file mode 100644
index 00000000000..58e19e52ee3
--- /dev/null
+++ b/container-core/src/main/java/com/yahoo/osgi/OsgiWrapper.java
@@ -0,0 +1,16 @@
+package com.yahoo.osgi;
+
+import com.yahoo.component.ComponentSpecification;
+import org.osgi.framework.Bundle;
+
+/**
+ * @author gjoranv
+ */
+public interface OsgiWrapper extends Osgi, com.yahoo.container.di.Osgi {
+
+    @Override
+    default Bundle getBundle(ComponentSpecification bundleId) {
+        return null;
+    }
+
+}