From f322503d3a615ad259877a4cf7b07c2129a13298 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Mon, 29 Aug 2011 10:56:10 -0700 Subject: [PATCH] Reverting 4de81f1eb3a63c5e4206ce99abe761e6d5a7e279. Max Spring discovered a stack overflow in http://jenkins.361315.n4.nabble.com/channel-example-and-plugin-classes-gives-ClassNotFoundException-td3756092.html that pointed this call into thread context classloader (TCL) as the culprit. Independently, Paul Sandoz discovered the same issue in our products. If TCL is another classloader that delegates to UberClassLoader (UCL), this code causes infinite recursion. And moreover, given the goal of UCL, it seems wrong that this class is changing behaviour based on contextual information like TCL. In looking at 4de81f1eb3a63c5e4206ce99abe761e6d5a7e279, the change was introduced to make sure XStream unmarshalling invoked from Plugin.start() would see the classes in that plugin. This was an issue back then when plugins are prepared and loaded one by one, as UCL didn't have visibility into plugins being prepared. But in the current Jenkins, classloaders for all the plugins are prepared, before any plugin gets started, so by the time Plugin.start() runs, UCL is fully functioning. Therefore, there's no need to consult TCL for the purpose of resolving its own classes. I could have also removed the code that sets TCL, but because some libraries often depend on TCL and doesn't properly look at caller class, I'm leaving it in, even though it's generally a bad practice to rely on TCL in multi-classloader apps like Jenkins --- core/src/main/java/hudson/ClassicPluginStrategy.java | 4 ++-- core/src/main/java/hudson/PluginManager.java | 10 ---------- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/core/src/main/java/hudson/ClassicPluginStrategy.java b/core/src/main/java/hudson/ClassicPluginStrategy.java index 57d7cff079..dd6683b91f 100644 --- a/core/src/main/java/hudson/ClassicPluginStrategy.java +++ b/core/src/main/java/hudson/ClassicPluginStrategy.java @@ -257,8 +257,8 @@ public class ClassicPluginStrategy implements PluginStrategy { } public void load(PluginWrapper wrapper) throws IOException { - // override the context classloader so that XStream activity in plugin.start() - // will be able to resolve classes in this plugin + // override the context classloader. This no longer makes sense, + // but it is left for the backward compatibility ClassLoader old = Thread.currentThread().getContextClassLoader(); Thread.currentThread().setContextClassLoader(wrapper.classLoader); try { diff --git a/core/src/main/java/hudson/PluginManager.java b/core/src/main/java/hudson/PluginManager.java index b4b843fc07..a98081ac3f 100644 --- a/core/src/main/java/hudson/PluginManager.java +++ b/core/src/main/java/hudson/PluginManager.java @@ -607,16 +607,6 @@ public abstract class PluginManager extends AbstractModelObject { else generatedClasses.remove(name,wc); } - // first, use the context classloader so that plugins that are loading - // can use its own classloader first. - ClassLoader cl = Thread.currentThread().getContextClassLoader(); - if(cl!=null && cl!=this) - try { - return cl.loadClass(name); - } catch(ClassNotFoundException e) { - // not found. try next - } - for (PluginWrapper p : activePlugins) { try { return p.classLoader.loadClass(name); -- GitLab