Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure ClassLoaders have names for debugging #9277

Merged
merged 7 commits into from
May 18, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 24 additions & 6 deletions core/src/main/java/hudson/ClassicPluginStrategy.java
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.UUID;
import java.util.jar.Attributes;
import java.util.jar.JarFile;
import java.util.jar.Manifest;
Expand Down Expand Up @@ -235,7 +236,11 @@
dependencyLoader = getBaseClassLoader(atts, dependencyLoader);

return new PluginWrapper(pluginManager, archive, manifest, baseResourceURL,
createClassLoader(paths, dependencyLoader, atts), disableFile, dependencies, optionalDependencies);
createClassLoader(computeClassLoaderName(manifest, archive), paths, dependencyLoader, atts), disableFile, dependencies, optionalDependencies);
}

private static String computeClassLoaderName(Manifest mf, File archive) {
return "PluginClassLoader for " + PluginWrapper.computeShortName(mf, archive.getName());
}

private void fix(Attributes atts, List<PluginWrapper.Dependency> optionalDependencies) {
Expand Down Expand Up @@ -263,15 +268,28 @@
return DetachedPluginsUtil.getImpliedDependencies(pluginName, jenkinsVersion);
}

@Deprecated
/**
* @deprecated since TODO use {@link #createClassLoader(String, List, ClassLoader, Attributes)}
*/
@Deprecated(since = "//TODO")
jtnord marked this conversation as resolved.
Show resolved Hide resolved
protected ClassLoader createClassLoader(List<File> paths, ClassLoader parent) throws IOException {
return createClassLoader(paths, parent, null);
}

/**
* Creates the classloader that can load all the specified jar files and delegate to the given parent.
* @deprecated since TODO use {@link #createClassLoader(String, List, ClassLoader, Attributes)}
*/
@Deprecated(since="@TODO")
jtnord marked this conversation as resolved.
Show resolved Hide resolved
protected ClassLoader createClassLoader(List<File> paths, ClassLoader parent, Attributes atts) throws IOException {
// generate a legacy id so at least we can track to something
return createClassLoader("legacy"+UUID.randomUUID(), paths, parent, atts);

Check warning on line 285 in core/src/main/java/hudson/ClassicPluginStrategy.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 285 is not covered by tests
jtnord marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Creates a classloader that can load all the specified jar files and delegate to the given parent.
* @since //TODO
jtnord marked this conversation as resolved.
Show resolved Hide resolved
*/
protected ClassLoader createClassLoader(String name, List<File> paths, ClassLoader parent, Attributes atts) throws IOException {
boolean usePluginFirstClassLoader =
atts != null && Boolean.parseBoolean(atts.getValue("PluginFirstClassLoader"));

Expand All @@ -285,9 +303,9 @@
}
URLClassLoader2 classLoader;
if (usePluginFirstClassLoader) {
classLoader = new PluginFirstClassLoader2(urls.toArray(new URL[0]), parent);
classLoader = new PluginFirstClassLoader2(name, urls.toArray(new URL[0]), parent);
} else {
classLoader = new URLClassLoader2(urls.toArray(new URL[0]), parent);
classLoader = new URLClassLoader2(name, urls.toArray(new URL[0]), parent);
}
return classLoader;
}
Expand Down Expand Up @@ -561,7 +579,7 @@
}

DependencyClassLoader(ClassLoader parent, File archive, List<Dependency> dependencies, PluginManager pluginManager) {
super(parent);
super("dependency ClassLoader for " + archive.getPath(), parent);
this._for = archive;
this.dependencies = List.copyOf(dependencies);
this.pluginManager = pluginManager;
Expand Down
5 changes: 3 additions & 2 deletions core/src/main/java/hudson/PluginFirstClassLoader2.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,9 @@ public class PluginFirstClassLoader2 extends URLClassLoader2 {
registerAsParallelCapable();
}

public PluginFirstClassLoader2(@NonNull URL[] urls, @NonNull ClassLoader parent) {
super(Objects.requireNonNull(urls), Objects.requireNonNull(parent));

public PluginFirstClassLoader2(String name, @NonNull URL[] urls, @NonNull ClassLoader parent) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

class is annotated with @Restricted(NoExternalUse.class) which is not visible in the collapsed diff.

super(name, Objects.requireNonNull(urls), Objects.requireNonNull(parent));
}

/**
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/hudson/PluginManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -1099,7 +1099,7 @@ protected void copyBundledPlugin(URL src, String fileName) throws IOException {
}

/*package*/ static @CheckForNull Manifest parsePluginManifest(URL bundledJpi) {
try (URLClassLoader cl = new URLClassLoader(new URL[]{bundledJpi})) {
try (URLClassLoader cl = new URLClassLoader("Temporary classloader for parsing " + bundledJpi.toString(), new URL[]{bundledJpi}, ClassLoader.getSystemClassLoader())) {
InputStream in = null;
try {
URL res = cl.findResource(PluginWrapper.MANIFEST_FILENAME);
Expand Down Expand Up @@ -2337,7 +2337,7 @@ public static final class UberClassLoader extends ClassLoader {
}

public UberClassLoader(List<PluginWrapper> activePlugins) {
super(PluginManager.class.getClassLoader());
super(activePlugins.stream().map(PluginWrapper::getShortName).collect(Collectors.joining(", ", "UberClassLoader for ", "")), PluginManager.class.getClassLoader());
jtnord marked this conversation as resolved.
Show resolved Hide resolved
this.activePlugins = activePlugins;
}

Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/hudson/util/MaskingClassLoader.java
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public MaskingClassLoader(ClassLoader parent, String... masks) {
}

public MaskingClassLoader(ClassLoader parent, Collection<String> masks) {
super(parent);
super("Masking ClassLoader of " + parent.getName(), parent);
this.masksClasses = List.copyOf(masks);

/*
Expand Down
29 changes: 29 additions & 0 deletions core/src/main/java/jenkins/util/URLClassLoader2.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,43 @@ public class URLClassLoader2 extends URLClassLoader implements JenkinsClassLoade
registerAsParallelCapable();
}

/**
* @deprecated use {@link URLClassLoader2#URLClassLoader2(String, URL[])}
*/
@Deprecated(since = "//TODO")
jtnord marked this conversation as resolved.
Show resolved Hide resolved
public URLClassLoader2(URL[] urls) {
super(urls);
}

/**
* @deprecated use {@link URLClassLoader2#URLClassLoader2(String, URL[], ClassLoader)}
*/
@Deprecated(since = "//TODO")
jtnord marked this conversation as resolved.
Show resolved Hide resolved
public URLClassLoader2(URL[] urls, ClassLoader parent) {
super(urls, parent);
}

/**
* Create a new {@link URLClassLoader2} with the given name and URLS and the {@link #getSystemClassLoader()} as its parent.
* @param name name of this classloader.
* @param urls the list of URLS to find classes in.
* @since //TODO
jtnord marked this conversation as resolved.
Show resolved Hide resolved
*/
public URLClassLoader2(String name, URL[] urls) {
super(name, urls, getSystemClassLoader());
}

/**
* Create a new {@link URLClassLoader2} with the given name, URLS parent.
* @param name name of this classloader.
* @param urls the list of URLS to find classes in.
* @param parent the parent to search for classes before we look in the {@code urls}
* @since //TODO
jtnord marked this conversation as resolved.
Show resolved Hide resolved
*/
public URLClassLoader2(String name, URL[] urls, ClassLoader parent) {
super(name, urls, parent);
}

@Override
public void addURL(URL url) {
super.addURL(url);
Expand Down
2 changes: 1 addition & 1 deletion core/src/test/java/hudson/PluginWrapperTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public void dependencyFailedToLoad() {
@Issue("JENKINS-66563")
@Test
public void insertJarsIntoClassPath() throws Exception {
try (URLClassLoader2 cl = new URLClassLoader2(new URL[0])) {
try (URLClassLoader2 cl = new URLClassLoader2("Test", new URL[0])) {
assertInjectingJarsWorks(cl);
}
}
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ THE SOFTWARE.
<changelog.url>https://www.jenkins.io/changelog</changelog.url>

<!-- Bundled Remoting version -->
<remoting.version>3206.vb_15dcf73f6a_9</remoting.version>
<remoting.version>3242.vfe5ce6301591</remoting.version>
jtnord marked this conversation as resolved.
Show resolved Hide resolved
<!-- Minimum Remoting version, which is tested for API compatibility -->
<remoting.minimum.supported.version>4.13</remoting.minimum.supported.version>
jtnord marked this conversation as resolved.
Show resolved Hide resolved

Expand Down
2 changes: 1 addition & 1 deletion war/src/main/java/executable/Main.java
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ public static void main(String[] args) throws IllegalAccessException {
// locate the Winstone launcher
ClassLoader cl;
try {
cl = new URLClassLoader(new URL[] {tmpJar.toURI().toURL()});
cl = new URLClassLoader("Jenkins Main ClassLoader", new URL[] {tmpJar.toURI().toURL()}, ClassLoader.getSystemClassLoader());
} catch (MalformedURLException e) {
throw new UncheckedIOException(e);
}
Expand Down
Loading