aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorgjoranv <gv@verizonmedia.com>2019-10-26 02:04:18 +0200
committergjoranv <gv@verizonmedia.com>2019-10-28 14:09:03 +0100
commit38ab0914bb97438ea8ddbe03089155192154580c (patch)
tree37036f4c68f44b2ccf0255c7a5818e56b7d16fcc
parentea149387f4e94edefdd8a4677075a1531e2357d0 (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.
-rw-r--r--container-core/src/main/java/com/yahoo/container/core/config/BundleLoader.java62
-rw-r--r--container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java10
-rw-r--r--container-core/src/main/java/com/yahoo/container/core/config/testutil/HandlersConfigurerTestWrapper.java4
-rw-r--r--container-core/src/main/java/com/yahoo/osgi/MockOsgi.java10
-rw-r--r--container-core/src/main/java/com/yahoo/osgi/Osgi.java10
-rw-r--r--container-core/src/main/java/com/yahoo/osgi/OsgiImpl.java48
-rw-r--r--container-di/src/main/java/com/yahoo/container/di/ComponentDeconstructor.java6
-rw-r--r--container-di/src/main/java/com/yahoo/container/di/Container.java36
-rw-r--r--container-di/src/main/java/com/yahoo/container/di/Osgi.java9
-rw-r--r--container-di/src/test/java/com/yahoo/container/di/ContainerTest.java22
-rw-r--r--container-di/src/test/java/com/yahoo/container/di/ContainerTestBase.java6
-rw-r--r--container-disc/src/main/java/com/yahoo/container/jdisc/DisableOsgiFramework.java11
-rw-r--r--container-disc/src/main/java/com/yahoo/container/jdisc/component/Deconstructor.java30
-rw-r--r--container-disc/src/test/java/com/yahoo/container/jdisc/component/DeconstructorTest.java7
-rw-r--r--jdisc_core/src/main/java/com/yahoo/jdisc/application/OsgiFramework.java16
-rw-r--r--jdisc_core/src/main/java/com/yahoo/jdisc/core/FelixFramework.java1
-rw-r--r--jdisc_core/src/main/java/com/yahoo/jdisc/test/NonWorkingOsgiFramework.java9
-rw-r--r--standalone-container/src/main/java/com/yahoo/application/container/impl/ClassLoaderOsgiFramework.java10
-rw-r--r--standalone-container/src/main/java/com/yahoo/container/standalone/StandaloneContainerActivator.java9
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() {
}