1 files changed, 44 insertions, 27 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() {