diff options
author | gjoranv <gv@verizonmedia.com> | 2019-10-26 02:04:18 +0200 |
---|---|---|
committer | gjoranv <gv@verizonmedia.com> | 2019-10-28 14:09:03 +0100 |
commit | 38ab0914bb97438ea8ddbe03089155192154580c (patch) | |
tree | 37036f4c68f44b2ccf0255c7a5818e56b7d16fcc | |
parent | ea149387f4e94edefdd8a4677075a1531e2357d0 (diff) |
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.
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() { } |