Skip to content
Permalink
Browse files
[FIXED JENKINS-12124] Occasionally errors loading plugin classes sinc…
…e it is expected that findClass (and findLoadedClass) are called under synchronization.

The problem was masked by a blind assumption that an InvocationTargetException was in fact wrapping a ClassNotFoundException.
Many thanks to @gotwarlost for demonstrating how to reproduce the problem and diagnosing the cause.
  • Loading branch information
jglick committed Feb 21, 2014
1 parent 8482756 commit 898f1f76a37e1c69cf38df718a5d3899544ebb44
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 46 deletions.
@@ -59,6 +59,9 @@
Build history widget only showed the last day of builds.
(Due to JENKINS-20892, even with this fix at most 20 builds are shown.)
(<a href="https://issues.jenkins-ci.org/browse/JENKINS-21159">issue 21159</a>)
<li class=bug>
Random class loading error mostly known to affect static analysis plugins.
(<a href="https://issues.jenkins-ci.org/browse/JENKINS-12124">issue 12124</a>)
<li class=rfe>
Slave started from Java Web Start can now install itself as a systemd service.
<li class=rfe>
@@ -56,7 +56,6 @@
import java.io.FilenameFilter;
import java.io.IOException;
import java.lang.reflect.Field;
import java.lang.reflect.InvocationTargetException;
import java.net.URL;
import java.util.ArrayList;
import java.util.Arrays;
@@ -72,7 +71,6 @@
import java.util.logging.Logger;

public class ClassicPluginStrategy implements PluginStrategy {
private final ClassLoaderReflectionToolkit clt = new ClassLoaderReflectionToolkit();

/**
* Filter for jar files.
@@ -569,10 +567,10 @@ protected Class<?> findClass(String name) throws ClassNotFoundException {
if (PluginManager.FAST_LOOKUP) {
for (PluginWrapper pw : getTransitiveDependencies()) {
try {
Class c = clt.findLoadedClass(pw.classLoader,name);
Class<?> c = ClassLoaderReflectionToolkit._findLoadedClass(pw.classLoader, name);
if (c!=null) return c;
return clt.findClass(pw.classLoader,name);
} catch (InvocationTargetException e) {
return ClassLoaderReflectionToolkit._findClass(pw.classLoader, name);
} catch (ClassNotFoundException e) {
//not found. try next
}
}
@@ -596,15 +594,11 @@ protected Enumeration<URL> findResources(String name) throws IOException {
HashSet<URL> result = new HashSet<URL>();

if (PluginManager.FAST_LOOKUP) {
try {
for (PluginWrapper pw : getTransitiveDependencies()) {
Enumeration<URL> urls = clt.findResources(pw.classLoader, name);
Enumeration<URL> urls = ClassLoaderReflectionToolkit._findResources(pw.classLoader, name);
while (urls != null && urls.hasMoreElements())
result.add(urls.nextElement());
}
} catch (InvocationTargetException e) {
throw new Error(e);
}
} else {
for (Dependency dep : dependencies) {
PluginWrapper p = pluginManager.getPlugin(dep.shortName);
@@ -622,14 +616,10 @@ protected Enumeration<URL> findResources(String name) throws IOException {
@Override
protected URL findResource(String name) {
if (PluginManager.FAST_LOOKUP) {
try {
for (PluginWrapper pw : getTransitiveDependencies()) {
URL url = clt.findResource(pw.classLoader,name);
URL url = ClassLoaderReflectionToolkit._findResource(pw.classLoader, name);
if (url!=null) return url;
}
} catch (InvocationTargetException e) {
throw new Error(e);
}
} else {
for (Dependency dep : dependencies) {
PluginWrapper p = pluginManager.getPlugin(dep.shortName);
@@ -82,7 +82,6 @@
import java.io.IOException;
import java.io.InputStream;
import java.lang.ref.WeakReference;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.net.URL;
import java.util.ArrayList;
@@ -941,8 +940,6 @@ public final class UberClassLoader extends ClassLoader {
*/
private ConcurrentMap<String, WeakReference<Class>> generatedClasses = new ConcurrentHashMap<String, WeakReference<Class>>();

private ClassLoaderReflectionToolkit clt = new ClassLoaderReflectionToolkit();

public UberClassLoader() {
super(PluginManager.class.getClassLoader());
}
@@ -963,11 +960,11 @@ protected Class<?> findClass(String name) throws ClassNotFoundException {
if (FAST_LOOKUP) {
for (PluginWrapper p : activePlugins) {
try {
Class c = clt.findLoadedClass(p.classLoader,name);
Class<?> c = ClassLoaderReflectionToolkit._findLoadedClass(p.classLoader, name);
if (c!=null) return c;
// calling findClass twice appears to cause LinkageError: duplicate class def
return clt.findClass(p.classLoader,name);
} catch (InvocationTargetException e) {
return ClassLoaderReflectionToolkit._findClass(p.classLoader, name);
} catch (ClassNotFoundException e) {
//not found. try next
}
}
@@ -987,15 +984,11 @@ protected Class<?> findClass(String name) throws ClassNotFoundException {
@Override
protected URL findResource(String name) {
if (FAST_LOOKUP) {
try {
for (PluginWrapper p : activePlugins) {
URL url = clt.findResource(p.classLoader,name);
URL url = ClassLoaderReflectionToolkit._findResource(p.classLoader, name);
if(url!=null)
return url;
}
} catch (InvocationTargetException e) {
throw new Error(e);
}
} else {
for (PluginWrapper p : activePlugins) {
URL url = p.classLoader.getResource(name);
@@ -1010,13 +1003,9 @@ protected URL findResource(String name) {
protected Enumeration<URL> findResources(String name) throws IOException {
List<URL> resources = new ArrayList<URL>();
if (FAST_LOOKUP) {
try {
for (PluginWrapper p : activePlugins) {
resources.addAll(Collections.list(clt.findResources(p.classLoader, name)));
resources.addAll(Collections.list(ClassLoaderReflectionToolkit._findResources(p.classLoader, name)));
}
} catch (InvocationTargetException e) {
throw new Error(e);
}
} else {
for (PluginWrapper p : activePlugins) {
resources.addAll(Collections.list(p.classLoader.getResources(name)));
@@ -1,22 +1,22 @@
package jenkins;

import java.io.IOException;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.net.URL;
import java.util.Enumeration;
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;

/**
* Reflection access to various {@link ClassLoader} methods.
*
* @author Kohsuke Kawaguchi
* Reflective access to various {@link ClassLoader} methods which are otherwise {@code protected}.
*/
@SuppressWarnings({"unchecked", "rawtypes"})
public class ClassLoaderReflectionToolkit {
/**
* ClassLoader.findClass(String) for a call that bypasses access modifier.
*/
private final Method FIND_CLASS, FIND_LOADED_CLASS, FIND_RESOURCE, FIND_RESOURCES;

public ClassLoaderReflectionToolkit() {
private static final Method FIND_CLASS, FIND_LOADED_CLASS, FIND_RESOURCE, FIND_RESOURCES, GET_CLASS_LOADING_LOCK;

static {
try {
FIND_CLASS = ClassLoader.class.getDeclaredMethod("findClass",String.class);
FIND_CLASS.setAccessible(true);
@@ -29,8 +29,85 @@ public ClassLoaderReflectionToolkit() {
} catch (NoSuchMethodException e) {
throw new AssertionError(e);
}
Method gCLL = null;
try {
gCLL = ClassLoader.class.getDeclaredMethod("getClassLoadingLock", String.class);
gCLL.setAccessible(true);
} catch (NoSuchMethodException x) {
// OK, Java 6
}
GET_CLASS_LOADING_LOCK = gCLL;
}

private static <T extends Exception> Object invoke(Method method, Class<T> exception, Object obj, Object... args) throws T {
try {
return method.invoke(obj, args);
} catch (IllegalAccessException x) {
throw new AssertionError(x);
} catch (InvocationTargetException x) {
Throwable x2 = x.getCause();
if (x2 instanceof RuntimeException) {
throw (RuntimeException) x2;
} else if (x2 instanceof Error) {
throw (Error) x2;
} else if (exception.isInstance(x2)) {
throw exception.cast(x2);
} else {
throw new AssertionError(x2);
}
}
}

private static Object getClassLoadingLock(ClassLoader cl, String name) {
if (GET_CLASS_LOADING_LOCK != null) {
return invoke(GET_CLASS_LOADING_LOCK, RuntimeException.class, cl, name);
} else {
// Java 6 expects you to always synchronize on this.
return cl;
}
}

/**
* Calls {@link ClassLoader#findLoadedClass} while holding {@link ClassLoader#getClassLoadingLock}.
* @since 1.553
*/
public static @CheckForNull Class<?> _findLoadedClass(ClassLoader cl, String name) {
synchronized (getClassLoadingLock(cl, name)) {
return (Class) invoke(FIND_LOADED_CLASS, RuntimeException.class, cl, name);
}
}

/**
* Calls {@link ClassLoader#findClass} while holding {@link ClassLoader#getClassLoadingLock}.
* @since 1.553
*/
public static @Nonnull Class<?> _findClass(ClassLoader cl, String name) throws ClassNotFoundException {
synchronized (getClassLoadingLock(cl, name)) {
return (Class) invoke(FIND_CLASS, ClassNotFoundException.class, cl, name);
}
}

/**
* Calls {@link ClassLoader#findResource}.
* @since 1.553
*/
public static @CheckForNull URL _findResource(ClassLoader cl, String name) {
return (URL) invoke(FIND_RESOURCE, RuntimeException.class, cl, name);
}

/**
* Calls {@link ClassLoader#findResources}.
* @since 1.553
*/
public static @Nonnull Enumeration<URL> _findResources(ClassLoader cl, String name) throws IOException {
return (Enumeration<URL>) invoke(FIND_RESOURCES, IOException.class, cl, name);
}

/** @deprecated unsafe */
@Deprecated public ClassLoaderReflectionToolkit() {}

/** @deprecated unsafe */
@Deprecated
public Class findLoadedClass(ClassLoader cl, String name) throws InvocationTargetException {
try {
return (Class)FIND_LOADED_CLASS.invoke(cl,name);
@@ -39,6 +116,8 @@ public Class findLoadedClass(ClassLoader cl, String name) throws InvocationTarge
}
}

/** @deprecated unsafe */
@Deprecated
public Class findClass(ClassLoader cl, String name) throws InvocationTargetException {
try {
return (Class)FIND_CLASS.invoke(cl,name);
@@ -47,6 +126,8 @@ public Class findClass(ClassLoader cl, String name) throws InvocationTargetExcep
}
}

/** @deprecated unsafe */
@Deprecated
public URL findResource(ClassLoader cl, String name) throws InvocationTargetException {
try {
return (URL)FIND_RESOURCE.invoke(cl,name);
@@ -55,6 +136,8 @@ public URL findResource(ClassLoader cl, String name) throws InvocationTargetExce
}
}

/** @deprecated unsafe */
@Deprecated
public Enumeration<URL> findResources(ClassLoader cl, String name) throws InvocationTargetException {
try {
return (Enumeration<URL>)FIND_RESOURCES.invoke(cl,name);
@@ -63,11 +146,4 @@ public Enumeration<URL> findResources(ClassLoader cl, String name) throws Invoca
}
}

// private void check(InvocationTargetException e) {
// Throwable t = e.getTargetException();
// if (t instanceof Error)
// throw (Error)t;
// if (t instanceof RuntimeException)
// throw (RuntimeException)t;
// }
}

0 comments on commit 898f1f7

Please sign in to comment.