Bug 570984 - StackOverflowError when Import-Package fails in fragment toI20210208-1800

circularly dependent plug-in

Using development mode in the old resolver can lead to endless recursion
when trying to set the suppliers involved in a cycle.  This is because
the cycle code is trying to drop bundles involved in the cycle that have
unsatisified requirements.  The problem is that when in development mode
the resolver allows requirements to be wired to capabilities from
unresolvable suppliers.  This means the cycle code is not properly
dropping the supplier so the supplier gets pulled into the resolve again
after dropping it which leads back to the cycle check again.

The fix is to avoid dropping the bundles involved in a cycle when in
development mode.


Change-Id: Ib3ef6b1796cd582e30a441799ed965b23cce973d
Signed-off-by: Thomas Watson <tjwatson@us.ibm.com>

3 files changed, 107 insertions, 3 deletions

diff --git a/bundles/org.eclipse.osgi.compatibility.state/src/org/eclipse/osgi/internal/module/ResolverImpl.java b/bundles/org.eclipse.osgi.compatibility.state/src/org/eclipse/osgi/internal/module/ResolverImpl.java
index 25af89e8e..7cd4e670c 100644
--- a/bundles/org.eclipse.osgi.compatibility.state/src/org/eclipse/osgi/internal/module/ResolverImpl.java
+++ b/bundles/org.eclipse.osgi.compatibility.state/src/org/eclipse/osgi/internal/module/ResolverImpl.java
@@ -1320,7 +1320,8 @@ public class ResolverImpl implements Resolver {
 			return;
 		cycleLoop: for (Iterator<ResolverBundle> iCycle = cycle.iterator(); iCycle.hasNext();) {
 			ResolverBundle cycleBundle = iCycle.next();
-			if (!cycleBundle.isResolvable()) {
+			// only clear cycles when not in dev mode
+			if (!developmentMode && !cycleBundle.isResolvable()) {
 				iCycle.remove(); // remove this bundle from the list of bundles that need re-resolved
 				continue cycleLoop;
 			}
@@ -1337,13 +1338,24 @@ public class ResolverImpl implements Resolver {
 					}
 				}
 				if (!resolverImport.isDynamic() && !resolverImport.isOptional() && resolverImport.getSelectedSupplier() == null) {
-					cycleBundle.setResolvable(false);
+					if (resolverImport.isFromFragment()) {
+						resolverImport.getBundle().setResolvable(false);
+					} else {
+						cycleBundle.setResolvable(false);
+					}
 					state.addResolverError(resolverImport.getVersionConstraint().getBundle(), ResolverError.MISSING_IMPORT_PACKAGE, resolverImport.getVersionConstraint().toString(), resolverImport.getVersionConstraint());
-					iCycle.remove();
 					continue cycleLoop;
 				}
 			}
 		}
+		// only clear cycles when not in dev mode
+		if (!developmentMode) {
+			for (Iterator<ResolverBundle> iCycle = cycle.iterator(); iCycle.hasNext();) {
+				if (!iCycle.next().isResolvable()) {
+					iCycle.remove();
+				}
+			}
+		}
 		if (cycle.size() != cycleSize) {
 			//we removed an un-resolvable bundle; must re-resolve remaining cycle
 			for (int i = 0; i < cycle.size(); i++) {
diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/TestModuleContainer.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/TestModuleContainer.java
index d1ce33195..53888f606 100644
--- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/TestModuleContainer.java
+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/TestModuleContainer.java
@@ -3762,6 +3762,50 @@ public class TestModuleContainer extends AbstractTest {
 		Assert.assertNull("Failed to resolve", report.getResolutionException());
 	}
 
+	@Test
+	public void testCycleBug570984() throws BundleException, IOException {
+		DummyContainerAdaptor adaptor = createDummyAdaptor();
+		ModuleContainer container = adaptor.getContainer();
+
+		// install the system.bundle
+		Module systemBundle = installDummyModule("system.bundle.MF", Constants.SYSTEM_BUNDLE_LOCATION,
+				Constants.SYSTEM_BUNDLE_SYMBOLICNAME, null, null, container);
+		ResolutionReport report = container.resolve(Arrays.asList(systemBundle), true);
+		Assert.assertNull("Failed to resolve system.bundle.", report.getResolutionException());
+
+		Map<String, String> manifestA = new HashMap<>();
+		manifestA.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+		manifestA.put(Constants.BUNDLE_SYMBOLICNAME, "a");
+		manifestA.put(Constants.BUNDLE_VERSION, "1");
+		manifestA.put(Constants.REQUIRE_BUNDLE, "b");
+		manifestA.put(Constants.PROVIDE_CAPABILITY, "ca");
+		manifestA.put(Constants.REQUIRE_CAPABILITY, "cb");
+		Module moduleA = installDummyModule(manifestA, "a", container);
+
+		Map<String, String> manifestB = new HashMap<>();
+		manifestB.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+		manifestB.put(Constants.BUNDLE_SYMBOLICNAME, "b");
+		manifestB.put(Constants.BUNDLE_VERSION, "1");
+		manifestB.put(Constants.REQUIRE_BUNDLE, "a");
+		manifestB.put(Constants.PROVIDE_CAPABILITY, "cb");
+		manifestB.put(Constants.REQUIRE_CAPABILITY, "ca");
+		Module moduleB = installDummyModule(manifestB, "b", container);
+
+		Map<String, String> manifestBF = new HashMap<>();
+		manifestBF.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+		manifestBF.put(Constants.BUNDLE_SYMBOLICNAME, "bf");
+		manifestBF.put(Constants.BUNDLE_VERSION, "1");
+		manifestBF.put(Constants.FRAGMENT_HOST, "b");
+		manifestBF.put(Constants.IMPORT_PACKAGE, "e");
+		Module moduleBF = installDummyModule(manifestBF, "bf", container);
+
+		report = container.resolve(Arrays.asList(moduleA, moduleB, moduleBF), false);
+		Assert.assertNull("Failed to resolve", report.getResolutionException());
+		assertEquals("Wrong state for moduleA", State.RESOLVED, moduleA.getState());
+		assertEquals("Wrong state for moduleB", State.RESOLVED, moduleB.getState());
+		assertEquals("Wrong state for moduleBF", State.INSTALLED, moduleBF.getState());
+	}
+
 	private static void assertWires(List<ModuleWire> required, List<ModuleWire>... provided) {
 		for (ModuleWire requiredWire : required) {
 			for (List<ModuleWire> providedList : provided) {
diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/services/resolver/StateResolverTest.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/services/resolver/StateResolverTest.java
index 4c3fc82cc..8bd3e5b2d 100644
--- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/services/resolver/StateResolverTest.java
+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/services/resolver/StateResolverTest.java
@@ -4410,6 +4410,54 @@ public class StateResolverTest extends AbstractStateTest {
 		assertEquals("Wrong number of visible", 2, visible.length);
 	}
 
+	public void testCycleBug570984() throws BundleException {
+		State state = buildEmptyState();
+		int bundleID = 0;
+
+		Hashtable<String, String> manifestA = new Hashtable<>();
+		manifestA.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+		manifestA.put(Constants.BUNDLE_SYMBOLICNAME, "a");
+		manifestA.put(Constants.BUNDLE_VERSION, "1");
+		manifestA.put(Constants.REQUIRE_BUNDLE, "b");
+		manifestA.put(Constants.PROVIDE_CAPABILITY, "ca");
+		manifestA.put(Constants.REQUIRE_CAPABILITY, "cb");
+		BundleDescription moduleA = state.getFactory().createBundleDescription(state, manifestA,
+				manifestA.get(Constants.BUNDLE_SYMBOLICNAME), bundleID++);
+
+		Hashtable<String, String> manifestB = new Hashtable<>();
+		manifestB.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+		manifestB.put(Constants.BUNDLE_SYMBOLICNAME, "b");
+		manifestB.put(Constants.BUNDLE_VERSION, "1");
+		manifestB.put(Constants.REQUIRE_BUNDLE, "a");
+		manifestB.put(Constants.PROVIDE_CAPABILITY, "cb");
+		manifestB.put(Constants.REQUIRE_CAPABILITY, "ca");
+		BundleDescription moduleB = state.getFactory().createBundleDescription(state, manifestB,
+				manifestB.get(Constants.BUNDLE_SYMBOLICNAME), bundleID++);
+
+		Hashtable<String, String> manifestBF = new Hashtable<>();
+		manifestBF.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+		manifestBF.put(Constants.BUNDLE_SYMBOLICNAME, "bf");
+		manifestBF.put(Constants.BUNDLE_VERSION, "1");
+		manifestBF.put(Constants.FRAGMENT_HOST, "b");
+		manifestBF.put(Constants.IMPORT_PACKAGE, "e");
+		BundleDescription moduleBF = state.getFactory().createBundleDescription(state, manifestBF,
+				manifestBF.get(Constants.BUNDLE_SYMBOLICNAME), bundleID++);
+
+		state.addBundle(moduleA);
+		state.addBundle(moduleB);
+		state.addBundle(moduleBF);
+		state.resolve();
+		assertTrue("A is not resolved", moduleA.isResolved()); //$NON-NLS-1$
+		assertTrue("B is not resolved", moduleB.isResolved()); //$NON-NLS-1$
+		assertFalse("BF not resolved", moduleBF.isResolved()); //$NON-NLS-1$
+
+		// now try dev mode - this will trigger a StackOverflowError
+		Dictionary[] props = new Dictionary[] { new Hashtable() };
+		props[0].put("osgi.resolverMode", "development"); //$NON-NLS-1$ //$NON-NLS-2$
+		state.setPlatformProperties(props);
+		state.resolve(new BundleDescription[] { moduleA, moduleB, moduleBF }, true);
+	}
+
 	public static class CatchAllValue {
 		public CatchAllValue(String s) {
 			//do nothing