Bug 577010 - [loader] avoid ClassNotFoundExceptionsS4_22_0_M3I20211116-0000I20211115-0600I20211114-1800I20211114-0600I20211113-1800I20211113-0600I20211112-2030I20211111-0910I20211110-1800I20211110-0750I20211110-0600I20211109-1800I20211109-0840I20211109-0720I20211108-1800I20211108-0620I20211107-1800I20211107-0600I20211106-1800I20211106-0600I20211105-1800I20211104-1800

Instead of throwing ClassNotFoundException a null value should be
returned when delegating.  This avoid unnecessarily generating an
exception in the normal flow of the code.  An exception should only be
generated when required by the APIs we are implementing in OSGi
(Bundle.loadClass) or the JVM (ClassLoader.loadClass).

There is a special case where a ClassNotFoundException is thrown when a
class load triggers a lazy activation and the bundle fails to start with
a BundleException.  But generally null can be used for "normal" class
not founds when delegating.

Change-Id: Ic8add2176ee5e80a79212629950b63724f9f28e8
Signed-off-by: Thomas Watson <tjwatson@us.ibm.com>
Signed-off-by: Jörg Kubitz <jkubitz-eclipse@gmx.de>
Reviewed-on: https://git.eclipse.org/r/c/equinox/rt.equinox.framework/+/187105
Tested-by: Equinox Bot <equinox-bot@eclipse.org>

7 files changed, 84 insertions, 77 deletions

diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/connect/DelegatingConnectClassLoader.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/connect/DelegatingConnectClassLoader.java
index e4be48b6b..0b03defd4 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/connect/DelegatingConnectClassLoader.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/connect/DelegatingConnectClassLoader.java
@@ -40,7 +40,7 @@ public class DelegatingConnectClassLoader extends EquinoxClassLoader {
 	@Override
 	public Class<?> findLocalClass(String classname) throws ClassNotFoundException {
 		if (connectClassLoader == null) {
-			throw new ClassNotFoundException();
+			return null;
 		}
 		return connectClassLoader.loadClass(classname);
 	}
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 0e5767558..9414400bd 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
@@ -400,8 +400,22 @@ public class BundleLoader extends ModuleLoader {
 	 * Finds the class for a bundle.  This method is used for delegation by the bundle's classloader.
 	 */
 	public Class<?> findClass(String name) throws ClassNotFoundException {
+		return findClass0(name, true);
+	}
 
-		if (parent != null && name.startsWith(JAVA_PACKAGE)) {
+	public Class<?> findClassNoParentNoException(String name) {
+		try {
+			return findClass0(name, false);
+		} catch (ClassNotFoundException e) {
+			// should rarely happen
+			// e.g. when a lazy activation fails to start a bundle
+			return null;
+		}
+	}
+
+	private Class<?> findClass0(String name, boolean parentAndGenerateException)
+			throws ClassNotFoundException {
+		if (parentAndGenerateException && parent != null && name.startsWith(JAVA_PACKAGE)) {
 			// 1) if startsWith "java." delegate to parent and terminate search
 			// we want to throw ClassNotFoundExceptions if a java.* class cannot be loaded from the parent.
 			return parent.loadClass(name);
@@ -413,7 +427,7 @@ public class BundleLoader extends ModuleLoader {
 		String pkgName = getPackageName(name);
 		boolean bootDelegation = false;
 		// follow the OSGi delegation model
-		if (parent != null && container.isBootDelegationPackage(pkgName)) {
+		if (parentAndGenerateException && parent != null && container.isBootDelegationPackage(pkgName)) {
 			// 2) if part of the bootdelegation list then delegate to parent and continue of failure
 			try {
 				return parent.loadClass(name);
@@ -425,8 +439,6 @@ public class BundleLoader extends ModuleLoader {
 		Class<?> result = null;
 		try {
 			result = (Class<?>) searchHooks(name, PRE_CLASS);
-		} catch (ClassNotFoundException e) {
-			throw e;
 		} catch (FileNotFoundException e) {
 			// will not happen
 		}
@@ -447,7 +459,7 @@ public class BundleLoader extends ModuleLoader {
 			}
 			if (result != null)
 				return result;
-			throw new ClassNotFoundException(name + " cannot be found by " + this); //$NON-NLS-1$
+			return generateException(name, parentAndGenerateException);
 		}
 		// 4) search the required bundles
 		source = findRequiredSource(pkgName, null);
@@ -470,16 +482,13 @@ public class BundleLoader extends ModuleLoader {
 				result = source.loadClass(name);
 				if (result != null)
 					return result;
-				// must throw CNFE if dynamic import source does not have the class
-				throw new ClassNotFoundException(name + " cannot be found by " + this); //$NON-NLS-1$
+				return generateException(name, parentAndGenerateException);
 			}
 		}
 
 		if (result == null)
 			try {
 				result = (Class<?>) searchHooks(name, POST_CLASS);
-			} catch (ClassNotFoundException e) {
-				throw e;
 			} catch (FileNotFoundException e) {
 				// will not happen
 			}
@@ -490,7 +499,7 @@ public class BundleLoader extends ModuleLoader {
 			return result;
 		// hack to support backwards compatibility for bootdelegation
 		// or last resort; do class context trick to work around VM bugs
-		if (parent != null && !bootDelegation
+		if (parentAndGenerateException && parent != null && !bootDelegation
 				&& ((container.getConfiguration().compatibilityBootDelegation) || isRequestFromVM())) {
 			// we don't need to continue if a CNFE is thrown here.
 			try {
@@ -499,7 +508,19 @@ public class BundleLoader extends ModuleLoader {
 				// we want to generate our own exception below
 			}
 		}
-		throw new ClassNotFoundException(name + " cannot be found by " + this); //$NON-NLS-1$
+		return generateException(name, parentAndGenerateException);
+	}
+
+	private Class<?> generateException(String name, boolean generate) throws ClassNotFoundException {
+		if (generate) {
+			ClassNotFoundException e = new ClassNotFoundException(name + " cannot be found by " + this); //$NON-NLS-1$
+			if (debug.DEBUG_LOADER) {
+				Debug.println("BundleLoader[" + this + "].loadClass(" + name + ") failed."); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
+				Debug.printStackTrace(e);
+			}
+			throw e;
+		}
+		return null;
 	}
 
 	@SuppressWarnings("unchecked")
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/ModuleClassLoader.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/ModuleClassLoader.java
index c09cfa8b7..ede519ad2 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/ModuleClassLoader.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/ModuleClassLoader.java
@@ -163,24 +163,14 @@ public abstract class ModuleClassLoader extends ClassLoader implements BundleRef
 	 */
 	@Override
 	protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
-		if (getDebug().DEBUG_LOADER)
-			Debug.println("ModuleClassLoader[" + getBundleLoader() + "].loadClass(" + name + ")"); //$NON-NLS-1$ //$NON-NLS-2$//$NON-NLS-3$
-		try {
-			// Just ask the delegate.  This could result in findLocalClass(name) being called.
-			Class<?> clazz = getBundleLoader().findClass(name);
-			// resolve the class if asked to.
-			if (resolve)
-				resolveClass(clazz);
-			return (clazz);
-		} catch (Error | ClassNotFoundException e) {
-			// If the class is not found do not try to look for it locally.
-			// The delegate would have already done that for us.
-			if (getDebug().DEBUG_LOADER) {
-				Debug.println("ModuleClassLoader[" + getBundleLoader() + "].loadClass(" + name + ") failed."); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
-				Debug.printStackTrace(e);
-			}
-			throw e;
+		// Just ask the delegate. This could result in findLocalClass(name) being
+		// called.
+		Class<?> clazz = getBundleLoader().findClass(name);
+		// resolve the class if asked to.
+		if (resolve) {
+			resolveClass(clazz);
 		}
+		return (clazz);
 	}
 
 	// preparing for Java 9
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/buddy/DependentPolicy.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/buddy/DependentPolicy.java
index 3e8dd60e2..10235c025 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/buddy/DependentPolicy.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/buddy/DependentPolicy.java
@@ -16,7 +16,9 @@ package org.eclipse.osgi.internal.loader.buddy;
 
 import java.io.IOException;
 import java.net.URL;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Enumeration;
+import java.util.List;
 import org.eclipse.osgi.container.ModuleWire;
 import org.eclipse.osgi.container.ModuleWiring;
 import org.eclipse.osgi.internal.loader.BundleLoader;
@@ -46,24 +48,22 @@ public class DependentPolicy implements IBuddyPolicy {
 
 	@Override
 	public Class<?> loadClass(String name) {
-		if (allDependents == null)
+		if (allDependents == null) {
 			return null;
-
-		Class<?> result = null;
+		}
 		//size may change, so we must check it every time
-		for (int i = 0; i < allDependents.size() && result == null; i++) {
+		for (int i = 0; i < allDependents.size(); i++) {
 			ModuleWiring searchWiring = allDependents.get(i);
 			BundleLoader searchLoader = (BundleLoader) searchWiring.getModuleLoader();
 			if (searchLoader != null) {
-				try {
-					result = searchLoader.findClass(name);
-				} catch (ClassNotFoundException e) {
-					if (result == null)
-						addDependent(i, searchWiring);
+				Class<?> result = searchLoader.findClassNoParentNoException(name);
+				if (result != null) {
+					return result;
 				}
+				addDependent(i, searchWiring);
 			}
 		}
-		return result;
+		return null;
 	}
 
 	private synchronized void addDependent(int i, ModuleWiring searchedWiring) {
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/buddy/GlobalPolicy.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/buddy/GlobalPolicy.java
index e3b3eda06..16bab2eae 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/buddy/GlobalPolicy.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/buddy/GlobalPolicy.java
@@ -21,15 +21,14 @@ import java.util.Collections;
 import java.util.Enumeration;
 import java.util.Map;
 import org.eclipse.osgi.container.ModuleContainer;
+import org.eclipse.osgi.container.ModuleWire;
 import org.eclipse.osgi.container.ModuleWiring;
 import org.eclipse.osgi.internal.container.Capabilities;
 import org.eclipse.osgi.internal.loader.BundleLoader;
-import org.osgi.framework.Bundle;
 import org.osgi.framework.namespace.HostNamespace;
 import org.osgi.framework.namespace.PackageNamespace;
 import org.osgi.framework.wiring.BundleCapability;
 import org.osgi.framework.wiring.BundleRevision;
-import org.osgi.framework.wiring.BundleWire;
 import org.osgi.framework.wiring.FrameworkWiring;
 import org.osgi.resource.Namespace;
 
@@ -49,28 +48,22 @@ public class GlobalPolicy implements IBuddyPolicy {
 	@Override
 	public Class<?> loadClass(String name) {
 		return getExportingBundles(BundleLoader.getPackageName(name)) //
-				.stream().findFirst().map(b -> {
-					try {
-						return b.loadClass(name);
-					} catch (ClassNotFoundException e) {
-						return null;
-					}
-				}).orElse(null);
+				.stream().findFirst().map(b -> b.findClassNoParentNoException(name)).orElse(null);
 	}
 
 	@Override
 	public URL loadResource(String name) {
 		return getExportingBundles(BundleLoader.getResourcePackageName(name)) //
-				.stream().findFirst().map(b -> b.getResource(name)).orElse(null);
+				.stream().findFirst().map(b -> b.findResource(name)).orElse(null);
 	}
 
 	@Override
 	public Enumeration<URL> loadResources(String name) {
 		Enumeration<URL> results = null;
-		Collection<Bundle> exporters = getExportingBundles(name);
-		for (Bundle exporter : exporters) {
+		Collection<BundleLoader> exporters = getExportingBundles(name);
+		for (BundleLoader exporter : exporters) {
 			try {
-				results = BundleLoader.compoundEnumerations(results, exporter.getResources(name));
+				results = BundleLoader.compoundEnumerations(results, exporter.findResources(name));
 			}catch (IOException e) {
 				//ignore IO problems and try next package
 			}
@@ -78,22 +71,26 @@ public class GlobalPolicy implements IBuddyPolicy {
 		return results;
 	}
 
-	private Collection<Bundle> getExportingBundles(String pkgName) {
-		Collection<Bundle> result = new ArrayList<>();
+	private Collection<BundleLoader> getExportingBundles(String pkgName) {
+		Collection<BundleLoader> result = new ArrayList<>();
 		String filter = "(" + PackageNamespace.PACKAGE_NAMESPACE + "=" + pkgName + ")"; //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
 		Map<String, String> directives = Collections.singletonMap(Namespace.REQUIREMENT_FILTER_DIRECTIVE, filter);
 		Map<String, Boolean> attributes = Collections.singletonMap(Capabilities.SYNTHETIC_REQUIREMENT, Boolean.TRUE);
 		Collection<BundleCapability> packages = frameworkWiring.findProviders(
 				ModuleContainer.createRequirement(PackageNamespace.PACKAGE_NAMESPACE, directives, attributes));
 		for (BundleCapability pkg : packages) {
-			if ((pkg.getRevision().getTypes() & BundleRevision.TYPE_FRAGMENT) != 0) {
-				// use the hosts
-				ModuleWiring wiring = (ModuleWiring) pkg.getRevision().getWiring();
-				for (BundleWire hostWire : wiring.getRequiredModuleWires(HostNamespace.HOST_NAMESPACE)) {
-					result.add(hostWire.getProvider().getBundle());
+			ModuleWiring wiring = (ModuleWiring) pkg.getRevision().getWiring();
+			if (wiring != null) {
+				if ((pkg.getRevision().getTypes() & BundleRevision.TYPE_FRAGMENT) != 0) {
+					// use the hosts
+					if (wiring != null) {
+						for (ModuleWire hostWire : wiring.getRequiredModuleWires(HostNamespace.HOST_NAMESPACE)) {
+							result.add((BundleLoader) hostWire.getProviderWiring().getModuleLoader());
+						}
+					}
+				} else {
+					result.add((BundleLoader) wiring.getModuleLoader());
 				}
-			} else {
-				result.add(pkg.getRevision().getBundle());
 			}
 		}
 		return result;
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/buddy/RegisteredPolicy.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/buddy/RegisteredPolicy.java
index 8bfc83ba7..19033d5f1 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/buddy/RegisteredPolicy.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/buddy/RegisteredPolicy.java
@@ -15,7 +15,9 @@ package org.eclipse.osgi.internal.loader.buddy;
 
 import java.io.IOException;
 import java.net.URL;
-import java.util.*;
+import java.util.Enumeration;
+import java.util.Iterator;
+import java.util.List;
 import org.eclipse.osgi.container.ModuleCapability;
 import org.eclipse.osgi.container.ModuleWiring;
 import org.eclipse.osgi.container.namespaces.EquinoxModuleDataNamespace;
@@ -64,24 +66,21 @@ public class RegisteredPolicy extends DependentPolicy {
 
 	@Override
 	public Class<?> loadClass(String name) {
-		if (allDependents == null)
+		if (allDependents == null) {
 			return null;
-
-		Class<?> result = null;
+		}
 		int size = allDependents.size();
-		for (int i = 0; i < size && result == null; i++) {
+		for (int i = 0; i < size; i++) {
 			ModuleWiring searchWiring = allDependents.get(i);
 			BundleLoader searchLoader = (BundleLoader) searchWiring.getModuleLoader();
 			if (searchLoader != null) {
-				try {
-					result = searchLoader.findClass(name);
-				} catch (ClassNotFoundException e) {
-					//Nothing to do, just keep looking
-					continue;
+				Class<?> result = searchLoader.findClassNoParentNoException(name);
+				if (result != null) {
+					return result;
 				}
 			}
 		}
-		return result;
+		return null;
 	}
 
 	@Override
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/classpath/ClasspathManager.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/classpath/ClasspathManager.java
index cf684bb44..dbaf19969 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/classpath/ClasspathManager.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/classpath/ClasspathManager.java
@@ -550,8 +550,8 @@ public class ClasspathManager {
 	 * Finally all the configured class loading hooks
 	 * {@link ClassLoaderHook#postFindLocalClass(String, Class, ClasspathManager)} methods are called.
 	 * @param classname the requested class name.
-	 * @return the requested class
-	 * @throws ClassNotFoundException if the class does not exist
+	 * @return the requested class or null if the class does not exist
+	 * @throws ClassNotFoundException if a ClassLoaderHook prevents the requested class from loading
 	 */
 	public Class<?> findLocalClass(String classname) throws ClassNotFoundException {
 		Class<?> result = null;
@@ -572,7 +572,7 @@ public class ClasspathManager {
 		}
 	}
 
-	private Class<?> findLocalClassImpl(String classname, List<ClassLoaderHook> hooks) throws ClassNotFoundException {
+	private Class<?> findLocalClassImpl(String classname, List<ClassLoaderHook> hooks) {
 		Class<?> result;
 
 		// look in hook specific entries if any
@@ -597,7 +597,7 @@ public class ClasspathManager {
 			}
 		}
 
-		throw new ClassNotFoundException(classname);
+		return null;
 	}
 
 	private Class<?> findLocalClassImpl(String classname, ClasspathEntry[] cpEntries, List<ClassLoaderHook> hooks) {