Bug 540230 - NPEs in BundleLoader caused by invalidated wirings

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

2 files changed, 128 insertions, 5 deletions

diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ClassLoadingBundleTests.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ClassLoadingBundleTests.java
index 65943c0f3..35c7646b9 100644
--- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ClassLoadingBundleTests.java
+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ClassLoadingBundleTests.java
@@ -35,6 +35,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import junit.framework.AssertionFailedError;
 import junit.framework.Test;
@@ -49,6 +50,7 @@ import org.osgi.framework.BundleListener;
 import org.osgi.framework.BundleReference;
 import org.osgi.framework.Constants;
 import org.osgi.framework.FrameworkEvent;
+import org.osgi.framework.FrameworkListener;
 import org.osgi.framework.FrameworkUtil;
 import org.osgi.framework.ServiceFactory;
 import org.osgi.framework.ServiceReference;
@@ -64,6 +66,7 @@ import org.osgi.framework.wiring.BundleRequirement;
 import org.osgi.framework.wiring.BundleRevision;
 import org.osgi.framework.wiring.BundleWire;
 import org.osgi.framework.wiring.BundleWiring;
+import org.osgi.framework.wiring.FrameworkWiring;
 import org.osgi.service.packageadmin.ExportedPackage;
 import org.osgi.service.packageadmin.PackageAdmin;
 import org.osgi.service.startlevel.StartLevel;
@@ -2322,4 +2325,119 @@ public class ClassLoadingBundleTests extends AbstractBundleTests {
 		actualFrameworkEvents = frameworkListenerResults.getResults(1);
 		compareResults(expectedFrameworkEvents, actualFrameworkEvents);
 	}
+
+	public void testStaleLoaderNPE() throws BundleException, IOException, ClassNotFoundException, InterruptedException {
+		File config = OSGiTestsActivator.getContext().getDataFile(getName()); //$NON-NLS-1$
+		config.mkdirs();
+
+		Map<String, String> exporterHeaders = new HashMap<>();
+		exporterHeaders.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+		exporterHeaders.put(Constants.BUNDLE_SYMBOLICNAME, "exporter");
+		exporterHeaders.put(Constants.EXPORT_PACKAGE, "export1, export2, export3, export4");
+		Map<String, String> exporterContent = new HashMap<>();
+		exporterContent.put("export1/", null);
+		exporterContent.put("export1/SomeClass.class", "SomeClass.class");
+		exporterContent.put("export2/", null);
+		exporterContent.put("export2/SomeClass.class", "SomeClass.class");
+		exporterContent.put("export3/", null);
+		exporterContent.put("export3/resource.txt", "resource.txt");
+		exporterContent.put("export4/", null);
+		exporterContent.put("export4/resource.txt", "resource.txt");
+		File exporterBundleFile = SystemBundleTests.createBundle(config, getName() + "-exporter", exporterHeaders, exporterContent);
+
+		Map<String, String> importerHeaders = new HashMap<>();
+		importerHeaders.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+		importerHeaders.put(Constants.BUNDLE_SYMBOLICNAME, "importer");
+		importerHeaders.put(Constants.IMPORT_PACKAGE, "export1, export2, export3, export4");
+		Map<String, String> importerContent = new HashMap<>();
+		importerContent.put("importer/", null);
+		importerContent.put("importer/resource.txt", "resource.txt");
+		importerContent.put("importer/SomeClass.class", "SomeClass.class");
+		File importerBundleFile = SystemBundleTests.createBundle(config, getName() + "-importer", importerHeaders, importerContent);
+
+		Map<String, String> requirerHeaders = new HashMap<>();
+		requirerHeaders.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+		requirerHeaders.put(Constants.BUNDLE_SYMBOLICNAME, "requirer");
+		requirerHeaders.put(Constants.REQUIRE_BUNDLE, "exporter");
+		Map<String, String> requirerContent = new HashMap<>();
+		requirerContent.put("requirer/", null);
+		requirerContent.put("requirer/resource.txt", "resource.txt");
+		requirerContent.put("requirer/SomeClass.class", "SomeClass.class");
+		File requirerBundleFile = SystemBundleTests.createBundle(config, getName() + "-requirer", requirerHeaders, requirerContent);
+
+		Bundle exporter = getContext().installBundle(getName() + "-exporter", new FileInputStream(exporterBundleFile));
+		exporter.start();
+		Bundle importer = getContext().installBundle(getName() + "-importer", new FileInputStream(importerBundleFile));
+		importer.start();
+		Bundle requirer = getContext().installBundle(getName() + "-requirer", new FileInputStream(requirerBundleFile));
+		requirer.start();
+
+		BundleWiring exporterWiring = exporter.adapt(BundleWiring.class);
+		BundleWiring importerWiring = importer.adapt(BundleWiring.class);
+		BundleWiring requirerWiring = requirer.adapt(BundleWiring.class);
+
+		// get class loaders so we can use them after invalidating the wires
+		ClassLoader importerCL = importerWiring.getClassLoader();
+		ClassLoader requirerCL = requirerWiring.getClassLoader();
+
+		try {
+			importerCL.loadClass("export1.SomeClass");
+			fail("Expecting LinkageError.");
+		} catch (LinkageError e) {
+			// expected; the class is invalid
+		}
+		URL export3Resource = importerCL.getResource("export3/resource.txt");
+		assertNotNull("Missing resource.", export3Resource);
+
+		try {
+			requirerCL.loadClass("export1.SomeClass");
+			fail("Expecting LinkageError.");
+		} catch (LinkageError e) {
+			// expected; the class is invalid
+		}
+		export3Resource = requirerCL.getResource("export3/resource.txt");
+		assertNotNull("Missing resource.", export3Resource);
+
+		// invalid wires by refreshing the exporter
+		refreshBundles(Collections.singleton(exporter));
+
+		try {
+			importerCL.loadClass("export2.SomeClass");
+			fail("Expecting LinkageError.");
+		} catch (LinkageError e) {
+			// expected; the class is invalid
+			// NOTE the import sources are calculated at loader creating time
+			// which happened before the refresh
+		}
+		URL export4Resource = importerCL.getResource("export4/resource.txt");
+		// Note that import sources are calculated at loader creating time
+		// which happened before the refresh
+		assertNotNull("Missing resource.", export4Resource);
+
+		try {
+			requirerCL.loadClass("export2.SomeClass");
+			fail("Expecting ClassNotFoundException.");
+		} catch (ClassNotFoundException e) {
+			// expected; the wire is invalid
+			// NOTE the require sources are calculated lazily but now
+			// the wire is invalid after the refresh
+		}
+
+		export4Resource = requirerCL.getResource("export4/resource.txt");
+		assertNull("Found resource from invalid wire.", export4Resource);
+	}
+
+	void refreshBundles(Collection<Bundle> bundles) throws InterruptedException {
+		final CountDownLatch refreshSignal = new CountDownLatch(1);
+		getContext().getBundle(Constants.SYSTEM_BUNDLE_LOCATION).adapt(FrameworkWiring.class).refreshBundles(bundles, new FrameworkListener() {
+
+			@Override
+			public void frameworkEvent(FrameworkEvent event) {
+				if (event.getType() == FrameworkEvent.PACKAGES_REFRESHED) {
+					refreshSignal.countDown();
+				}
+			}
+		});
+		refreshSignal.await(30, TimeUnit.SECONDS);
+	}
 }
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/BundleLoader.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/BundleLoader.java
index 0666185ab..3db16962b 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/BundleLoader.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/BundleLoader.java
@@ -223,7 +223,7 @@ public class BundleLoader extends ModuleLoader {
 
 	final PackageSource createExportPackageSource(ModuleWire importWire, Collection<BundleLoader> visited) {
 		String name = (String) importWire.getCapability().getAttributes().get(PackageNamespace.PACKAGE_NAMESPACE);
-		BundleLoader providerLoader = (BundleLoader) importWire.getProviderWiring().getModuleLoader();
+		BundleLoader providerLoader = getProviderLoader(importWire);
 		if (providerLoader == null) {
 			return createMultiSource(name, new PackageSource[0]);
 		}
@@ -799,7 +799,7 @@ public class BundleLoader extends ModuleLoader {
 		Collection<BundleLoader> visited = new ArrayList<>();
 		visited.add(this); // always add ourselves so we do not recurse back to ourselves
 		for (ModuleWire bundleWire : requiredBundleWires) {
-			BundleLoader loader = (BundleLoader) bundleWire.getProviderWiring().getModuleLoader();
+			BundleLoader loader = getProviderLoader(bundleWire);
 			if (loader != null) {
 				loader.addProvidedPackageNames(pkgName, packages, subPackages, visited);
 			}
@@ -951,7 +951,7 @@ public class BundleLoader extends ModuleLoader {
 				// always add required bundles first if we locally provide the package
 				// This allows a bundle to provide a package from a required bundle without 
 				// re-exporting the whole required bundle.
-				BundleLoader loader = (BundleLoader) bundleWire.getProviderWiring().getModuleLoader();
+				BundleLoader loader = getProviderLoader(bundleWire);
 				if (loader != null) {
 					loader.addExportedProvidersFor(packageName, result, visited);
 				}
@@ -962,6 +962,11 @@ public class BundleLoader extends ModuleLoader {
 			result.add(local);
 	}
 
+	private BundleLoader getProviderLoader(ModuleWire wire) {
+		ModuleWiring provider = wire.getProviderWiring();
+		return provider == null ? null : (BundleLoader) provider.getModuleLoader();
+	}
+
 	final void addProvidedPackageNames(String packageName, List<String> result, boolean subPackages, Collection<BundleLoader> visited) {
 		if (visited.contains(this))
 			return;
@@ -982,7 +987,7 @@ public class BundleLoader extends ModuleLoader {
 		}
 		for (ModuleWire bundleWire : requiredBundleWires) {
 			if (BundleNamespace.VISIBILITY_REEXPORT.equals(bundleWire.getRequirement().getDirectives().get(BundleNamespace.REQUIREMENT_VISIBILITY_DIRECTIVE))) {
-				BundleLoader loader = (BundleLoader) bundleWire.getProviderWiring().getModuleLoader();
+				BundleLoader loader = getProviderLoader(bundleWire);
 				if (loader != null) {
 					loader.addProvidedPackageNames(packageName, result, subPackages, visited);
 				}
@@ -1187,7 +1192,7 @@ public class BundleLoader extends ModuleLoader {
 			visited.add(this); // always add ourselves so we do not recurse back to ourselves
 		List<PackageSource> result = new ArrayList<>(3);
 		for (ModuleWire bundleWire : requiredBundleWires) {
-			BundleLoader loader = (BundleLoader) bundleWire.getProviderWiring().getModuleLoader();
+			BundleLoader loader = getProviderLoader(bundleWire);
 			if (loader != null) {
 				loader.addExportedProvidersFor(pkgName, result, visited);
 			}