diff --git a/core/src/main/java/hudson/ClassicPluginStrategy.java b/core/src/main/java/hudson/ClassicPluginStrategy.java index fd257fdb33d8..39486ce47616 100644 --- a/core/src/main/java/hudson/ClassicPluginStrategy.java +++ b/core/src/main/java/hudson/ClassicPluginStrategy.java @@ -33,6 +33,7 @@ import hudson.model.Hudson; import hudson.util.CyclicGraphDetector; import hudson.util.CyclicGraphDetector.CycleDetectedException; +import hudson.util.DelegatingClassLoader; import hudson.util.IOUtils; import hudson.util.MaskingClassLoader; import java.io.File; @@ -559,7 +560,7 @@ private static void unzipExceptClasses(File archive, File destDir, Project prj) /** * Used to load classes from dependency plugins. */ - static final class DependencyClassLoader extends ClassLoader { + static final class DependencyClassLoader extends DelegatingClassLoader { /** * This classloader is created for this plugin. Useful during debugging. */ @@ -574,10 +575,6 @@ static final class DependencyClassLoader extends ClassLoader { */ private volatile List transitiveDependencies; - static { - registerAsParallelCapable(); - } - DependencyClassLoader(ClassLoader parent, File archive, List dependencies, PluginManager pluginManager) { super("dependency ClassLoader for " + archive.getPath(), parent); this._for = archive; diff --git a/core/src/main/java/hudson/PluginManager.java b/core/src/main/java/hudson/PluginManager.java index c3965e719c00..9562775152a2 100644 --- a/core/src/main/java/hudson/PluginManager.java +++ b/core/src/main/java/hudson/PluginManager.java @@ -60,6 +60,8 @@ import hudson.security.PermissionScope; import hudson.util.CyclicGraphDetector; import hudson.util.CyclicGraphDetector.CycleDetectedException; +import hudson.util.DelegatingClassLoader; +import hudson.util.ExistenceCheckingClassLoader; import hudson.util.FormValidation; import hudson.util.PersistedList; import hudson.util.Retrier; @@ -2400,18 +2402,26 @@ public T of(String key, Class type, Supplier func) { /** * {@link ClassLoader} that can see all plugins. */ - public static final class UberClassLoader extends ClassLoader { + public static final class UberClassLoader extends DelegatingClassLoader { private final List activePlugins; /** Cache of loaded, or known to be unloadable, classes. */ private final ConcurrentMap>> loaded = new ConcurrentHashMap<>(); - static { - registerAsParallelCapable(); - } - + /** + * The servlet container's {@link ClassLoader} (the parent of Jenkins core) is + * parallel-capable and maintains its own growing {@link Map} of {@link + * ClassLoader#getClassLoadingLock} objects per class name for every load attempt (including + * misses), and we cannot override this behavior. Wrap the servlet container {@link + * ClassLoader} in {@link ExistenceCheckingClassLoader} to avoid calling {@link + * ClassLoader#getParent}'s {@link ClassLoader#loadClass(String, boolean)} at all for misses + * by first checking if the resource exists. If the resource does not exist, we immediately + * throw {@link ClassNotFoundException}. As a result, the servlet container's {@link + * ClassLoader} is never asked to try and fail, and it never creates/retains lock objects + * for those misses. + */ public UberClassLoader(List activePlugins) { - super("UberClassLoader", PluginManager.class.getClassLoader()); + super("UberClassLoader", new ExistenceCheckingClassLoader(PluginManager.class.getClassLoader())); this.activePlugins = activePlugins; } diff --git a/core/src/main/java/hudson/util/DelegatingClassLoader.java b/core/src/main/java/hudson/util/DelegatingClassLoader.java new file mode 100644 index 000000000000..fc0b8421d609 --- /dev/null +++ b/core/src/main/java/hudson/util/DelegatingClassLoader.java @@ -0,0 +1,83 @@ +package hudson.util; + +import java.util.Objects; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; + +/** + * A {@link ClassLoader} that does not define any classes itself but delegates class loading to + * other class loaders to avoid the JDK's per-class-name locking and lock retention. + * + *

This class first attempts to load classes via its {@link ClassLoader#getParent} class loader, + * then falls back to {@link ClassLoader#findClass} to allow for custom delegation logic. + * + *

In a parallel-capable {@link ClassLoader}, the JDK maintains a per-name lock object + * indefinitely. In Jenkins, many class loading misses across many loaders can accumulate hundreds + * of thousands of such locks, retaining significant memory. This loader never defines classes and + * bypasses {@link ClassLoader}'s default {@code loadClass} locking; it delegates to the parent + * first and then to {@code findClass} for custom delegation. + * + *

The actual defining loader (parent or a delegate) still performs the necessary synchronization + * and class definition. A runtime guard ({@link #verify}) throws if this loader ever becomes the + * defining loader. + * + *

Subclasses must not call {@code defineClass}; implement delegation in {@code findClass} if + * needed and do not mark subclasses as parallel-capable. + * + * @author Dmytro Ukhlov + */ +@Restricted(NoExternalUse.class) +public abstract class DelegatingClassLoader extends ClassLoader { + protected DelegatingClassLoader(String name, ClassLoader parent) { + super(name, Objects.requireNonNull(parent)); + } + + public DelegatingClassLoader(ClassLoader parent) { + super(Objects.requireNonNull(parent)); + } + + /** + * Parent-first delegation without synchronizing on {@link #getClassLoadingLock(String)}. This + * prevents creation/retention of per-name lock objects in a loader that does not define + * classes. The defining loader downstream still serializes class definition as required. + * + * @param name The binary name of the class + * @param resolve If {@code true} then resolve the class + * @return The resulting {@link Class} object + * @throws ClassNotFoundException If the class could not be found + */ + @Override + protected Class loadClass(String name, boolean resolve) throws ClassNotFoundException { + Class c = null; + try { + c = getParent().loadClass(name); + } catch (ClassNotFoundException e) { + // ClassNotFoundException thrown if class not found + // from the non-null parent class loader + } + + if (c == null) { + // If still not found, then invoke findClass in order + // to find the class. + c = findClass(name); + } + + verify(c); + if (resolve) { + resolveClass(c); + } + return c; + } + + /** + * Safety check to ensure this delegating loader never becomes the defining loader. + * + *

Fails fast if a subclass erroneously defines a class here, which would violate the + * delegation-only contract and could reintroduce locking/retention issues. + */ + private void verify(Class clazz) { + if (clazz.getClassLoader() == this) { + throw new IllegalStateException("DelegatingClassLoader must not be the defining loader: " + clazz.getName()); + } + } +} diff --git a/core/src/main/java/hudson/util/ExistenceCheckingClassLoader.java b/core/src/main/java/hudson/util/ExistenceCheckingClassLoader.java new file mode 100644 index 000000000000..a77e0c7aa6ee --- /dev/null +++ b/core/src/main/java/hudson/util/ExistenceCheckingClassLoader.java @@ -0,0 +1,60 @@ +package hudson.util; + +import java.util.Objects; +import jenkins.util.URLClassLoader2; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; + +/** + * A {@link ClassLoader} that verifies the existence of a {@code .class} resource before attempting + * to load the class. Intended to sit in front of servlet container loaders we do not control. + * + *

This implementation overrides {@link #loadClass(String, boolean)} and uses {@link + * #getResource(String)} to check whether the corresponding .class file is available in + * the classpath. If the resource is not found, a {@link ClassNotFoundException} is thrown + * immediately. + * + *

Parallel-capable parent loaders retain a per-class-name lock object for every load attempt, + * including misses. By checking getResource(name + ".class") first and throwing {@link + * ClassNotFoundException} on absence, we avoid calling {@code loadClass} on misses, thus preventing + * the parent from populating its lock map for nonexistent classes. + * + *

This class is only needed in {@link hudson.PluginManager.UberClassLoader}. It is unnecessary + * for plugin {@link ClassLoader}s (because {@link URLClassLoader2} mitigates lock retention via + * {@link ClassLoader#getClassLoadingLock}) and redundant for delegators (because {@link + * DelegatingClassLoader} already avoids base locking). + * + * @author Dmytro Ukhlov + * @see ClassLoader + * @see #getResource(String) + */ +@Restricted(NoExternalUse.class) +public final class ExistenceCheckingClassLoader extends DelegatingClassLoader { + + public ExistenceCheckingClassLoader(String name, ClassLoader parent) { + super(name, Objects.requireNonNull(parent)); + } + + public ExistenceCheckingClassLoader(ClassLoader parent) { + super(Objects.requireNonNull(parent)); + } + + /** + * Short-circuits misses by checking for the {@code .class} resource prior to delegation. + * Successful loads behave exactly as the parent would; misses do not touch the parent's + * per-name lock map. + */ + @Override + protected Class loadClass(String name, boolean resolve) throws ClassNotFoundException { + // Add support for loading of JaCoCo dynamic instrumentation classes + if (name.equals("java.lang.$JaCoCo")) { + return super.loadClass(name, resolve); + } + + if (getResource(name.replace('.', '/') + ".class") == null) { + throw new ClassNotFoundException(name); + } + + return super.loadClass(name, resolve); + } +} diff --git a/core/src/main/java/hudson/util/MaskingClassLoader.java b/core/src/main/java/hudson/util/MaskingClassLoader.java index dfad754fc170..d138440feb50 100644 --- a/core/src/main/java/hudson/util/MaskingClassLoader.java +++ b/core/src/main/java/hudson/util/MaskingClassLoader.java @@ -42,7 +42,7 @@ * * @author Kohsuke Kawaguchi */ -public class MaskingClassLoader extends ClassLoader { +public class MaskingClassLoader extends DelegatingClassLoader { /** * Prefix of the packages that should be hidden. */ @@ -50,10 +50,6 @@ public class MaskingClassLoader extends ClassLoader { private final List masksResources; - static { - registerAsParallelCapable(); - } - public MaskingClassLoader(ClassLoader parent, String... masks) { this(parent, Arrays.asList(masks)); } diff --git a/core/src/main/java/jenkins/util/URLClassLoader2.java b/core/src/main/java/jenkins/util/URLClassLoader2.java index a6e5194e14f3..8b21eb1c0436 100644 --- a/core/src/main/java/jenkins/util/URLClassLoader2.java +++ b/core/src/main/java/jenkins/util/URLClassLoader2.java @@ -2,6 +2,8 @@ import java.net.URL; import java.net.URLClassLoader; +import java.util.Objects; +import java.util.concurrent.atomic.AtomicInteger; import jenkins.ClassLoaderReflectionToolkit; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.NoExternalUse; @@ -12,6 +14,10 @@ */ @Restricted(NoExternalUse.class) public class URLClassLoader2 extends URLClassLoader implements JenkinsClassLoader { + private static final AtomicInteger NEXT_INSTANCE_NUMBER = new AtomicInteger(0); + + private final String lockObjectPrefixName = String.format( + "%s@%x-loadClassLock:", URLClassLoader2.class.getName(), NEXT_INSTANCE_NUMBER.getAndIncrement()); static { registerAsParallelCapable(); @@ -69,8 +75,25 @@ public Class findLoadedClass2(String name) { return super.findLoadedClass(name); } + /** + * Replace the JDK's per-name lock map with a GC-collectable lock object. This is a workaround + * for JDK-8005233. When JDK-8005233 is resolved, this should be deleted. See also the + * discussion in this OpenJDK + * thread. + * + *

Parallel-capable {@link ClassLoader} implementations keep a distinct lock object per class + * name indefinitely, which can retain huge maps when there are many misses. Returning an + * interned {@link String} keyed by this loader and the class name preserves mutual exclusion + * for a given (loader, name) pair but allows the JVM to reclaim the lock when no longer + * referenced. Interned Strings are heap objects and GC-eligible on modern JDKs (7+). + * + * @param className the binary name of the class being loaded (must not be null) + * @return a lock object unique to this classloader/class pair + */ @Override public Object getClassLoadingLock(String className) { - return super.getClassLoadingLock(className); + Objects.requireNonNull(className); + return (lockObjectPrefixName + className).intern(); } }