diff options
19 files changed, 250 insertions, 66 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..c796b773232 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 @@ -26,11 +26,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 +45,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 { @@ -47,16 +53,19 @@ public class BundleLoader { } /** 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 has no effect that pendingUninstall fileRefs are removed from the map, as they are NOT 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 +106,7 @@ public class BundleLoader { } List<Bundle> bundles = osgi.install(file.getAbsolutePath()); + reference2Bundles.put(reference, bundles); } @@ -113,13 +123,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 +148,39 @@ 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. + */ + private Set<Bundle> retainOnly(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()); + // Clean up the map of active bundles Set<FileReference> fileReferencesToRemove = new HashSet<>(reference2Bundles.keySet()); fileReferencesToRemove.removeAll(newReferences); + fileReferencesToRemove.forEach(reference2Bundles::remove); - for (FileReference fileReferenceToRemove : fileReferencesToRemove) { - reference2Bundles.remove(fileReferenceToRemove); - } - return bundlesToRemove.size(); + return bundlesToRemove; } - 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> bundles) { + Set<Bundle> bundlesToUninstall = retainOnly(bundles); + osgi.allowDuplicateBundles(bundlesToUninstall); + install(bundles); 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..2ca76535b34 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 @@ -30,6 +30,7 @@ import org.osgi.framework.wiring.BundleWiring; import java.util.ArrayList; import java.util.Collection; +import java.util.List; import java.util.concurrent.Executor; import java.util.concurrent.Executors; import java.util.logging.Logger; @@ -121,14 +122,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..bc06aad76ec 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; @@ -103,11 +102,12 @@ public class HandlersConfigurerTestWrapper { } 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/osgi/MockOsgi.java b/container-core/src/main/java/com/yahoo/osgi/MockOsgi.java index 45ad02d2cef..5cfcde7a8a3 100644 --- a/container-core/src/main/java/com/yahoo/osgi/MockOsgi.java +++ b/container-core/src/main/java/com/yahoo/osgi/MockOsgi.java @@ -16,11 +16,21 @@ import java.util.List; public class MockOsgi extends NonWorkingOsgiFramework implements Osgi { @Override + public List<Bundle> getInitialBundles() { + return Collections.emptyList(); + } + + @Override public Bundle[] getBundles() { return new Bundle[0]; } @Override + public List<Bundle> getCurrentBundles() { + return Collections.emptyList(); + } + + @Override public Bundle getBundle(ComponentSpecification bundleId) { return null; } 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..fa7e88dbfe6 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,23 @@ 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 allowDuplicateBundles(Collection<Bundle> bundles); + + // TODO: remove void uninstall(Bundle bundle); + // TODO: remove void refreshPackages(); } 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..9a74aeda8cf 100644 --- a/container-core/src/main/java/com/yahoo/osgi/OsgiImpl.java +++ b/container-core/src/main/java/com/yahoo/osgi/OsgiImpl.java @@ -5,9 +5,13 @@ import com.yahoo.component.ComponentSpecification; import com.yahoo.component.Version; import com.yahoo.container.bundle.BundleInstantiationSpecification; import com.yahoo.jdisc.application.OsgiFramework; +import com.yahoo.jdisc.test.NonWorkingOsgiFramework; import org.osgi.framework.Bundle; import org.osgi.framework.BundleException; +import org.osgi.framework.launch.Framework; +import java.util.Collection; +import java.util.Collections; import java.util.List; /** @@ -18,8 +22,33 @@ public class OsgiImpl implements Osgi { 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; + + if (jdiscOsgi instanceof NonWorkingOsgiFramework) { + initialBundles = Collections.emptyList(); + alwaysCurrentBundle = null; + } else { + + 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!"); + } + } + + @Override + public List<Bundle> getInitialBundles() { + return initialBundles; } @Override @@ -28,6 +57,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); @@ -87,7 +120,7 @@ public class OsgiImpl implements Osgi { */ public Bundle getBundle(ComponentSpecification id) { 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,6 +155,11 @@ public class OsgiImpl implements Osgi { } @Override + public void allowDuplicateBundles(Collection<Bundle> bundles) { + jdiscOsgi.allowDuplicateBundles(bundles); + } + + @Override public void uninstall(Bundle bundle) { try { bundle.uninstall(); @@ -135,4 +173,12 @@ public class OsgiImpl implements Osgi { 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-di/src/main/java/com/yahoo/container/di/ComponentDeconstructor.java b/container-di/src/main/java/com/yahoo/container/di/ComponentDeconstructor.java index 09f72c9d86d..61497cf71bc 100644 --- a/container-di/src/main/java/com/yahoo/container/di/ComponentDeconstructor.java +++ b/container-di/src/main/java/com/yahoo/container/di/ComponentDeconstructor.java @@ -1,6 +1,8 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.container.di; +import org.osgi.framework.Bundle; + import java.util.Collection; /** @@ -8,5 +10,7 @@ import java.util.Collection; * @author Tony Vaagenes */ public interface ComponentDeconstructor { - void deconstruct(Collection<Object> components); + + void deconstruct(Collection<Object> components, Collection<Bundle> bundles); + } diff --git a/container-di/src/main/java/com/yahoo/container/di/Container.java b/container-di/src/main/java/com/yahoo/container/di/Container.java index f82fd22f40d..9a9245f4ba2 100644 --- a/container-di/src/main/java/com/yahoo/container/di/Container.java +++ b/container-di/src/main/java/com/yahoo/container/di/Container.java @@ -18,9 +18,13 @@ import com.yahoo.container.di.componentgraph.core.Node; import com.yahoo.container.di.config.RestApiContext; import com.yahoo.container.di.config.SubscriberFactory; import com.yahoo.vespa.config.ConfigKey; +import org.osgi.framework.Bundle; +import java.util.Collection; +import java.util.Collections; import java.util.HashSet; import java.util.IdentityHashMap; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.logging.Level; @@ -65,19 +69,22 @@ public class Container { }); } - private void deconstructObsoleteComponents(ComponentGraph oldGraph, ComponentGraph newGraph) { + private void deconstructObsoleteComponents(ComponentGraph oldGraph, + ComponentGraph newGraph, + Collection<Bundle> obsoleteBundles) { IdentityHashMap<Object, Object> oldComponents = new IdentityHashMap<>(); oldGraph.allConstructedComponentsAndProviders().forEach(c -> oldComponents.put(c, null)); newGraph.allConstructedComponentsAndProviders().forEach(oldComponents::remove); - componentDeconstructor.deconstruct(oldComponents.keySet()); + componentDeconstructor.deconstruct(oldComponents.keySet(), obsoleteBundles); } public ComponentGraph getNewComponentGraph(ComponentGraph oldGraph, Injector fallbackInjector, boolean restartOnRedeploy) { try { - ComponentGraph newGraph = getConfigAndCreateGraph(oldGraph, fallbackInjector, restartOnRedeploy); + Collection<Bundle> obsoleteBundles = new HashSet<>(); + ComponentGraph newGraph = getConfigAndCreateGraph(oldGraph, fallbackInjector, restartOnRedeploy, obsoleteBundles); newGraph.reuseNodes(oldGraph); constructComponents(newGraph); - deconstructObsoleteComponents(oldGraph, newGraph); + deconstructObsoleteComponents(oldGraph, newGraph, obsoleteBundles); return newGraph; } catch (Throwable t) { // TODO: Wrap ComponentConstructorException in an Error when generation==0 (+ unit test that Error is thrown) @@ -122,8 +129,13 @@ public class Container { } } - private ComponentGraph getConfigAndCreateGraph(ComponentGraph graph, Injector fallbackInjector, boolean restartOnRedeploy) { + private ComponentGraph getConfigAndCreateGraph(ComponentGraph graph, + Injector fallbackInjector, + boolean restartOnRedeploy, + Collection<Bundle> obsoleteBundles) // NOTE: Return value + { ConfigSnapshot snapshot; + while (true) { snapshot = configurer.getConfigs(graph.configKeys(), leastGeneration, restartOnRedeploy); @@ -131,7 +143,6 @@ public class Container { graph.configKeys(), graph.generation(), snapshot)); if (snapshot instanceof BootstrapConfigs) { - // TODO: remove require when proven unnecessary if (getBootstrapGeneration() <= previousConfigGeneration) { throw new IllegalStateException(String.format( "Got bootstrap configs out of sequence for old config generation %d.\n" + "Previous config generation is %d", @@ -142,8 +153,12 @@ public class Container { "Got new bootstrap generation\n" + "bootstrap generation = %d\n" + "components generation: %d\n" + "previous generation: %d\n", getBootstrapGeneration(), getComponentsGeneration(), previousConfigGeneration)); - installBundles(snapshot.configs()); + + Collection<Bundle> bundlesToRemove = installBundles(snapshot.configs()); + obsoleteBundles.addAll(bundlesToRemove); + graph = createComponentsGraph(snapshot.configs(), getBootstrapGeneration(), fallbackInjector); + // Continues loop } else if (snapshot instanceof ConfigRetriever.ComponentsConfigs) { @@ -184,9 +199,9 @@ public class Container { } } - private void installBundles(Map<ConfigKey<? extends ConfigInstance>, ConfigInstance> configsIncludingBootstrapConfigs) { + private Set<Bundle> installBundles(Map<ConfigKey<? extends ConfigInstance>, ConfigInstance> configsIncludingBootstrapConfigs) { BundlesConfig bundlesConfig = getConfig(bundlesConfigKey, configsIncludingBootstrapConfigs); - osgi.useBundles(bundlesConfig.bundle()); + return osgi.useBundles(bundlesConfig.bundle()); } private ComponentGraph createComponentsGraph(Map<ConfigKey<? extends ConfigInstance>, ConfigInstance> configsIncludingBootstrapConfigs, @@ -244,7 +259,8 @@ public class Container { } private void deconstructAllComponents(ComponentGraph graph, ComponentDeconstructor deconstructor) { - deconstructor.deconstruct(graph.allConstructedComponentsAndProviders()); + // This is only used for shutdown, so no need to uninstall any bundles. + deconstructor.deconstruct(graph.allConstructedComponentsAndProviders(), Collections.emptyList()); } public static <T extends ConfigInstance> T getConfig(ConfigKey<T> key, diff --git a/container-di/src/main/java/com/yahoo/container/di/Osgi.java b/container-di/src/main/java/com/yahoo/container/di/Osgi.java index 7095180dfc5..ab7da7665b6 100644 --- a/container-di/src/main/java/com/yahoo/container/di/Osgi.java +++ b/container-di/src/main/java/com/yahoo/container/di/Osgi.java @@ -13,6 +13,8 @@ import java.util.Collections; import java.util.Set; import java.util.stream.Collectors; +import static java.util.Collections.emptySet; + /** * @author gjoranv * @author Tony Vaagenes @@ -23,8 +25,13 @@ public interface Osgi { return new BundleClasses(new MockBundle(), Collections.emptySet()); } - default void useBundles(Collection<FileReference> bundles) { + /** + * Returns the set of bundles that is not used by the current application generation, + * and therefore should be scheduled for uninstalling. + */ + default Set<Bundle> useBundles(Collection<FileReference> bundles) { System.out.println("useBundles " + bundles.stream().map(Object::toString).collect(Collectors.joining(", "))); + return emptySet(); } default Class<?> resolveClass(BundleInstantiationSpecification spec) { diff --git a/container-di/src/test/java/com/yahoo/container/di/ContainerTest.java b/container-di/src/test/java/com/yahoo/container/di/ContainerTest.java index 9c4891c7db2..90bda0ef8a8 100644 --- a/container-di/src/test/java/com/yahoo/container/di/ContainerTest.java +++ b/container-di/src/test/java/com/yahoo/container/di/ContainerTest.java @@ -14,6 +14,8 @@ import com.yahoo.container.di.componentgraph.core.Node; import com.yahoo.container.di.config.RestApiContext; import org.junit.Ignore; import org.junit.Test; +import org.osgi.framework.Bundle; +import org.osgi.framework.BundleException; import java.util.Collection; import java.util.concurrent.ExecutionException; @@ -283,13 +285,16 @@ public class ContainerTest extends ContainerTestBase { public void providers_are_destructed() { writeBootstrapConfigs("id1", DestructableProvider.class); - ComponentDeconstructor deconstructor = components -> components.forEach(component -> { - if (component instanceof AbstractComponent) { - ((AbstractComponent) component).deconstruct(); - } else if (component instanceof Provider) { - ((Provider<?>) component).deconstruct(); - } - }); + ComponentDeconstructor deconstructor = (components, bundles) -> { + components.forEach(component -> { + if (component instanceof AbstractComponent) { + ((AbstractComponent) component).deconstruct(); + } else if (component instanceof Provider) { + ((Provider<?>) component).deconstruct(); + } + }); + if (! bundles.isEmpty()) throw new IllegalArgumentException("This test should not use bundles"); + }; Container container = newContainer(dirConfigSource, deconstructor); @@ -373,13 +378,14 @@ public class ContainerTest extends ContainerTestBase { public static class TestDeconstructor implements ComponentDeconstructor { @Override - public void deconstruct(Collection<Object> components) { + public void deconstruct(Collection<Object> components, Collection<Bundle> bundles) { components.forEach(component -> { if (component instanceof DestructableComponent) { DestructableComponent vespaComponent = (DestructableComponent) component; vespaComponent.deconstruct(); } }); + if (! bundles.isEmpty()) throw new IllegalArgumentException("This test should not use bundles"); } } diff --git a/container-di/src/test/java/com/yahoo/container/di/ContainerTestBase.java b/container-di/src/test/java/com/yahoo/container/di/ContainerTestBase.java index 79cb080dfa4..18236a6bde9 100644 --- a/container-di/src/test/java/com/yahoo/container/di/ContainerTestBase.java +++ b/container-di/src/test/java/com/yahoo/container/di/ContainerTestBase.java @@ -5,7 +5,6 @@ import com.google.inject.Guice; import com.yahoo.component.ComponentSpecification; import com.yahoo.config.FileReference; import com.yahoo.container.bundle.BundleInstantiationSpecification; -import com.yahoo.container.di.CloudSubscriberFactory; import com.yahoo.container.di.ContainerTest.ComponentTakingConfig; import com.yahoo.container.di.componentgraph.core.ComponentGraph; import com.yahoo.container.di.osgi.BundleClasses; @@ -16,6 +15,8 @@ import org.osgi.framework.Bundle; import java.util.Collection; import java.util.Set; +import static java.util.Collections.emptySet; + /** * @author Tony Vaagenes * @author gjoranv @@ -60,7 +61,8 @@ public class ContainerTestBase { } @Override - public void useBundles(Collection<FileReference> bundles) { + public Set<Bundle> useBundles(Collection<FileReference> bundles) { + return emptySet(); } @Override 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); } diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/application/OsgiFramework.java b/jdisc_core/src/main/java/com/yahoo/jdisc/application/OsgiFramework.java index f5346a21a4f..4dc59accee4 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/application/OsgiFramework.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/application/OsgiFramework.java @@ -5,6 +5,7 @@ import org.osgi.framework.Bundle; import org.osgi.framework.BundleContext; import org.osgi.framework.BundleException; +import java.util.Collection; import java.util.List; /** @@ -13,6 +14,7 @@ import java.util.List; * {@link BundleInstaller} since that provides common convenience methods. * * @author Simon Thoresen Hult + * @author gjoranv */ public interface OsgiFramework { @@ -81,6 +83,20 @@ public interface OsgiFramework { List<Bundle> bundles(); /** + * Returns all installed bundles that are visible to the requesting bundle. Bundle visibility + * is controlled via implementations of {@link org.osgi.framework.hooks.bundle.FindHook}; + */ + List<Bundle> getBundles(Bundle requestingBundle); + + /** + * Allows this framework to install duplicates of the given collection of bundles. Duplicate detection + * is handled by the {@link com.yahoo.jdisc.core.BundleCollisionHook}. + * + * @param bundles The bundles to allow duplicates of + */ + void allowDuplicateBundles(Collection<Bundle> bundles); + + /** * This method starts the framework instance. Before this method is called, any call to {@link * #installBundle(String)} or {@link #bundles()} will generate a {@link NullPointerException}. * diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/core/FelixFramework.java b/jdisc_core/src/main/java/com/yahoo/jdisc/core/FelixFramework.java index 19a1707e97c..8b226a0fe3e 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/core/FelixFramework.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/core/FelixFramework.java @@ -153,6 +153,7 @@ public class FelixFramework implements OsgiFramework { return Arrays.asList(felix.getBundleContext().getBundles()); } + @Override public List<Bundle> getBundles(Bundle requestingBundle) { return Arrays.asList(requestingBundle.getBundleContext().getBundles()); } diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/test/NonWorkingOsgiFramework.java b/jdisc_core/src/main/java/com/yahoo/jdisc/test/NonWorkingOsgiFramework.java index 6b129e82a45..0f927aa97d3 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/test/NonWorkingOsgiFramework.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/test/NonWorkingOsgiFramework.java @@ -5,6 +5,7 @@ import com.yahoo.jdisc.application.OsgiFramework; import org.osgi.framework.Bundle; import org.osgi.framework.BundleContext; +import java.util.Collection; import java.util.Collections; import java.util.List; @@ -39,6 +40,14 @@ public class NonWorkingOsgiFramework implements OsgiFramework { } @Override + public List<Bundle> getBundles(Bundle requestingBundle) { + return Collections.emptyList(); + } + + @Override + public void allowDuplicateBundles(Collection<Bundle> bundles) { } + + @Override public void start() { } diff --git a/standalone-container/src/main/java/com/yahoo/application/container/impl/ClassLoaderOsgiFramework.java b/standalone-container/src/main/java/com/yahoo/application/container/impl/ClassLoaderOsgiFramework.java index 08a70db7562..0d5b4c85264 100644 --- a/standalone-container/src/main/java/com/yahoo/application/container/impl/ClassLoaderOsgiFramework.java +++ b/standalone-container/src/main/java/com/yahoo/application/container/impl/ClassLoaderOsgiFramework.java @@ -63,7 +63,7 @@ public final class ClassLoaderOsgiFramework implements OsgiFramework { @Override public List<Bundle> installBundle(String bundleLocation) { - if (bundleLocation != null && bundleLocation.isEmpty() == false) { + if (bundleLocation != null && ! bundleLocation.isEmpty()) { try { URL url = new URL(bundleLocation); bundleLocations.add(url); @@ -106,6 +106,14 @@ public final class ClassLoaderOsgiFramework implements OsgiFramework { } @Override + public List<Bundle> getBundles(Bundle requestingBundle) { + return bundleList; + } + + @Override + public void allowDuplicateBundles(Collection<Bundle> bundles) { } + + @Override public void start() { } diff --git a/standalone-container/src/main/java/com/yahoo/container/standalone/StandaloneContainerActivator.java b/standalone-container/src/main/java/com/yahoo/container/standalone/StandaloneContainerActivator.java index 04c3396c95c..cfd5f753c4f 100644 --- a/standalone-container/src/main/java/com/yahoo/container/standalone/StandaloneContainerActivator.java +++ b/standalone-container/src/main/java/com/yahoo/container/standalone/StandaloneContainerActivator.java @@ -24,6 +24,7 @@ import java.net.InetSocketAddress; import java.nio.channels.ServerSocketChannel; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.Hashtable; import java.util.List; @@ -142,6 +143,14 @@ public class StandaloneContainerActivator implements BundleActivator { } @Override + public List<Bundle> getBundles(Bundle requestingBundle) { + return Collections.emptyList(); + } + + @Override + public void allowDuplicateBundles(Collection<Bundle> bundles) { } + + @Override public void start() { } |