Skip to content

Commit

Permalink
[FIXED JENKINS-18677]
Browse files Browse the repository at this point in the history
Integrated bytecode-compatibility-transformer that allows core to
do signature changes on properties that plugins might depend on.

The library performs necessary bytecode transformation to achieve this.

The first use of this is to fix plugins that looks for List
AbstractProject.triggers, thereby resolving JENKINS-18677.

For the time being, I'm not loading such compatibility annotations from
plugins, but I did code that in PluginManager. Let's see how this
feature work out for a while in the core, and if it looks stable and
solid, we'll open it up to plugins at that point.

(cherry picked from commit 47de54d)

Conflicts:
	changelog.html
	pom.xml
  • Loading branch information
kohsuke authored and olivergondza committed Sep 20, 2013
1 parent 5407d9f commit 235e9b8
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 16 deletions.
5 changes: 5 additions & 0 deletions core/pom.xml
Expand Up @@ -214,6 +214,11 @@ THE SOFTWARE.
<artifactId>annotation-indexer</artifactId>
<version>1.4</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci</groupId>
<artifactId>bytecode-compatibility-transformer</artifactId>
<version>1.3</version>
</dependency>
<dependency>
<groupId>org.jvnet.hudson</groupId>
<artifactId>task-reactor</artifactId>
Expand Down
26 changes: 11 additions & 15 deletions core/src/main/java/hudson/ClassicPluginStrategy.java
Expand Up @@ -50,7 +50,6 @@
import java.lang.reflect.Field;
import java.lang.reflect.InvocationTargetException;
import java.net.URL;
import java.net.URLClassLoader;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
Expand Down Expand Up @@ -215,19 +214,10 @@ protected ClassLoader createClassLoader(List<File> paths, ClassLoader parent, At
return classLoader;
}
}
if(useAntClassLoader && !Closeable.class.isAssignableFrom(URLClassLoader.class)) {
// using AntClassLoader with Closeable so that we can predictably release jar files opened by URLClassLoader
AntClassLoader2 classLoader = new AntClassLoader2(parent);
classLoader.addPathFiles(paths);
return classLoader;
} else {
// Tom reported that AntClassLoader has a performance issue when Hudson keeps trying to load a class that doesn't exist,
// so providing a legacy URLClassLoader support, too
List<URL> urls = new ArrayList<URL>();
for (File path : paths)
urls.add(path.toURI().toURL());
return new URLClassLoader(urls.toArray(new URL[urls.size()]),parent);
}

AntClassLoader2 classLoader = new AntClassLoader2(parent);
classLoader.addPathFiles(paths);
return classLoader;

This comment has been minimized.

Copy link
@jglick

jglick Jan 8, 2014

Member

This should be reconsidered: better to use plain URLClassLoader when we can (on Java 7 it is Closeable). Would need to subclass it to override defineClassFromData of course.

}

/**
Expand Down Expand Up @@ -588,7 +578,7 @@ protected URL findResource(String name) {
* {@link AntClassLoader} with a few methods exposed and {@link Closeable} support.
* Deprecated as of Java 7, retained only for Java 5/6.
*/
private static final class AntClassLoader2 extends AntClassLoader implements Closeable {
private final class AntClassLoader2 extends AntClassLoader implements Closeable {
private final Vector pathComponents;

private AntClassLoader2(ClassLoader parent) {
Expand All @@ -605,6 +595,7 @@ private AntClassLoader2(ClassLoader parent) {
}
}


public void addPathFiles(Collection<File> paths) throws IOException {
for (File f : paths)
addPathFile(f);
Expand Down Expand Up @@ -635,6 +626,11 @@ protected URL findResource(String name) {

return url;
}

@Override
protected Class defineClassFromData(File container, byte[] classData, String classname) throws IOException {
return super.defineClassFromData(container, pluginManager.getCompatibilityTransformer().transform(classname,classData), classname);
}
}

public static boolean useAntClassLoader = Boolean.getBoolean(ClassicPluginStrategy.class.getName()+".useAntClassLoader");
Expand Down
21 changes: 21 additions & 0 deletions core/src/main/java/hudson/PluginManager.java
Expand Up @@ -59,6 +59,7 @@
import org.apache.commons.io.FileUtils;
import org.apache.commons.io.FilenameUtils;
import org.apache.commons.logging.LogFactory;
import org.jenkinsci.bytecode.Transformer;
import org.jvnet.hudson.reactor.Executable;
import org.jvnet.hudson.reactor.Reactor;
import org.jvnet.hudson.reactor.ReactorException;
Expand Down Expand Up @@ -151,6 +152,8 @@ public abstract class PluginManager extends AbstractModelObject implements OnMas
// and load plugin-contributed classes.
public final ClassLoader uberClassLoader = new UberClassLoader();

private final Transformer compatibilityTransformer = new Transformer();

/**
* Once plugin is uploaded, this flag becomes true.
* This is used to report a message that Jenkins needs to be restarted
Expand Down Expand Up @@ -179,6 +182,17 @@ public PluginManager(ServletContext context, File rootDir) {
rootDir.mkdirs();

strategy = createPluginStrategy();

// load up rules for the core first
try {
compatibilityTransformer.loadRules(getClass().getClassLoader());
} catch (IOException e) {
LOGGER.log(Level.WARNING, "Failed to load compatibility rewrite rules",e);
}
}

public Transformer getCompatibilityTransformer() {
return compatibilityTransformer;
}

public Api getApi() {
Expand Down Expand Up @@ -301,6 +315,13 @@ protected void reactOnCycle(PluginWrapper q, List<PluginWrapper> cycle)
}
});

// Let's see for a while until we open this functionality up to plugins
// g.followedBy().attains(PLUGINS_LISTED).add("Load compatibility rules", new Executable() {
// public void run(Reactor reactor) throws Exception {
// compatibilityTransformer.loadRules(uberClassLoader);
// }
// });

session.addAll(g.discoverTasks(session));

pluginListed = true; // technically speaking this is still too early, as at this point tasks are merely scheduled, not necessarily executed.
Expand Down
2 changes: 2 additions & 0 deletions core/src/main/java/hudson/model/AbstractProject.java
Expand Up @@ -91,6 +91,7 @@
import net.sf.json.JSONObject;
import org.acegisecurity.context.SecurityContext;
import org.acegisecurity.context.SecurityContextHolder;
import org.jenkinsci.bytecode.AdaptField;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.args4j.Argument;
Expand Down Expand Up @@ -235,6 +236,7 @@ public abstract class AbstractProject<P extends AbstractProject<P,R>,R extends A
/**
* List of all {@link Trigger}s for this project.
*/
@AdaptField(was=List.class)
protected volatile DescribableList<Trigger<?>,TriggerDescriptor> triggers = new DescribableList<Trigger<?>,TriggerDescriptor>(this);
private AtomicReferenceFieldUpdater<AbstractProject,List> triggersUpdater
= AtomicReferenceFieldUpdater.newUpdater(AbstractProject.class,List.class,"triggers");
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Expand Up @@ -172,7 +172,7 @@ THE SOFTWARE.
<dependency>
<groupId>org.jenkins-ci.main</groupId>
<artifactId>remoting</artifactId>
<version>2.23</version>
<version>2.32</version>
</dependency>

<dependency>
Expand Down

0 comments on commit 235e9b8

Please sign in to comment.