Skip to content

Commit

Permalink
[FIXED JENKINS-36991] Do not give up on classloading if loading is in…
Browse files Browse the repository at this point in the history 
…terrupted (#94)

* [FIXED JENKINS-36991] Do not give up on classloading if loading is interrupted while fetching ClassRefference

* [JENKINS-36991] Propagate unrelated exceptions
  • Loading branch information
@olivergondza @oleg-nenashev
olivergondza authored and oleg-nenashev committed Aug 3, 2016
1 parent 030ee12 commit fe2feff
Show file tree
Hide file tree
Showing 2 changed files with 149 additions and 73 deletions.
123 changes: 78 additions & 45 deletions src/main/java/hudson/remoting/RemoteClassLoader.java
View file
Expand Up @@ -168,48 +168,81 @@ then the class file image is wasted.)
cr = prefetchedClasses.remove(name); cr = prefetchedClasses.remove(name);
if (cr == null) { if (cr == null) {
LOGGER.log(Level.FINER, "fetch3({0})", name); LOGGER.log(Level.FINER, "fetch3({0})", name);
Map<String,ClassFile2> all = proxy.fetch3(name);
synchronized (prefetchedClasses) { boolean interrupted = false;
/** try {
* Converts {@link ClassFile2} to {@link ClassReference} with minimal // the code in this try block may throw InterruptException, but findClass
* proxy creation. This creates a reference to {@link ClassLoader}, so // method is supposed to be uninterruptible. So we catch interrupt exception
* it shoudn't be kept beyond the scope of single {@link #findClass(String)} call. // and just retry until it succeeds, but in the end we set the interrupt flag
*/ // back on to let the interrupt in the next earliest occasion.
class ClassReferenceBuilder {
private final Map<Integer,ClassLoader> classLoaders = new HashMap<Integer, ClassLoader>(); while (true) {

try {
ClassReference toRef(ClassFile2 cf) { if (TESTING_CLASS_REFERENCE_LOAD != null) {
int n = cf.classLoader; TESTING_CLASS_REFERENCE_LOAD.run();

}
ClassLoader cl = classLoaders.get(n);
if (cl==null) Map<String,ClassFile2> all = proxy.fetch3(name);
classLoaders.put(n,cl = channel.importedClassLoaders.get(n)); synchronized (prefetchedClasses) {

/**
return new ClassReference(cl,cf.image); * Converts {@link ClassFile2} to {@link ClassReference} with minimal
} * proxy creation. This creates a reference to {@link ClassLoader}, so
} * it shoudn't be kept beyond the scope of single {@link #findClass(String)} call.
ClassReferenceBuilder crf = new ClassReferenceBuilder(); */

class ClassReferenceBuilder {
for (Map.Entry<String,ClassFile2> entry : all.entrySet()) { private final Map<Integer,ClassLoader> classLoaders = new HashMap<Integer, ClassLoader>();
String cn = entry.getKey();
ClassFile2 cf = entry.getValue(); ClassReference toRef(ClassFile2 cf) {
ClassReference ref = crf.toRef(cf); int n = cf.classLoader;


if (cn.equals(name)) { ClassLoader cl = classLoaders.get(n);
cr = ref; if (cl==null)
} else { classLoaders.put(n,cl = channel.importedClassLoaders.get(n));
// where we remember the prefetch is sensitive to who references it,
// because classes need not be transitively visible in Java return new ClassReference(cl,cf.image);
if (cf.referer!=null) }
ref.rememberIn(cn, crf.toRef(cf.referer).classLoader); }
else ClassReferenceBuilder crf = new ClassReferenceBuilder();
ref.rememberIn(cn, this);

for (Map.Entry<String,ClassFile2> entry : all.entrySet()) {
LOGGER.log(Level.FINER, "prefetch {0} -> {1}", new Object[]{name, cn}); String cn = entry.getKey();
ClassFile2 cf = entry.getValue();
ClassReference ref = crf.toRef(cf);

if (cn.equals(name)) {
cr = ref;
} else {
// where we remember the prefetch is sensitive to who references it,
// because classes need not be transitively visible in Java
if (cf.referer!=null)
ref.rememberIn(cn, crf.toRef(cf.referer).classLoader);
else
ref.rememberIn(cn, this);

LOGGER.log(Level.FINER, "prefetch {0} -> {1}", new Object[]{name, cn});
}

ref.rememberIn(cn, ref.classLoader);
}
}
break;
} catch (RemotingSystemException x) {
if (x.getCause() instanceof InterruptedException) {
// pretend as if this operation is not interruptible.
// but we need to remember to set the interrupt flag back on
// before we leave this call.
interrupted = true;
continue; // JENKINS-19453: retry
}
throw x;
} }


ref.rememberIn(cn, ref.classLoader); // no code is allowed to reach here
} }
} finally {
// process the interrupt later.
if (interrupted)
Thread.currentThread().interrupt();
} }


assert cr != null; assert cr != null;
Expand Down Expand Up @@ -239,9 +272,8 @@ ClassReference toRef(ClassFile2 cf) {


while (true) { while (true) {
try { try {
if (TESTING) { if (TESTING_CLASS_LOAD != null) TESTING_CLASS_LOAD.run();
Thread.sleep(1000);
}
if (c!=null) return c; if (c!=null) return c;


// TODO: check inner class handling // TODO: check inner class handling
Expand Down Expand Up @@ -312,11 +344,12 @@ private static Object _getClassLoadingLock(RemoteClassLoader rcl, String name) {
return rcl; return rcl;
} }
/** /**
* Induce a large delay in {@link RemoteClassLoader#findClass(String)} to allow * Intercept {@link RemoteClassLoader#findClass(String)} to allow unittests to be written.
* *
* See JENKINS-6604 * See JENKINS-6604 and similar issues
*/ */
static boolean TESTING; static Runnable TESTING_CLASS_LOAD;
static Runnable TESTING_CLASS_REFERENCE_LOAD;


private Class<?> loadClassFile(String name, byte[] bytes) { private Class<?> loadClassFile(String name, byte[] bytes) {
if (bytes.length < 8) { if (bytes.length < 8) {
Expand Down
99 changes: 71 additions & 28 deletions src/test/java/hudson/remoting/ClassRemotingTest.java
View file
Expand Up @@ -29,9 +29,10 @@
import org.objectweb.asm.commons.EmptyVisitor; import org.objectweb.asm.commons.EmptyVisitor;


import java.io.IOException; import java.io.IOException;
import java.util.concurrent.CancellationException; import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService; import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors; import java.util.concurrent.Executors;
import java.util.concurrent.Future;


/** /**
* Test class image forwarding. * Test class image forwarding.
Expand Down Expand Up @@ -94,7 +95,7 @@ public void testRaceCondition() throws Throwable {
assertEquals(child2, c2.getClass().getClassLoader()); assertEquals(child2, c2.getClass().getClassLoader());
assertEquals(parent, c2.getClass().getSuperclass().getClassLoader()); assertEquals(parent, c2.getClass().getSuperclass().getClassLoader());
ExecutorService svc = Executors.newFixedThreadPool(2); ExecutorService svc = Executors.newFixedThreadPool(2);
RemoteClassLoader.TESTING = true; RemoteClassLoader.TESTING_CLASS_LOAD = new SleepForASec();
try { try {
java.util.concurrent.Future<Object> f1 = svc.submit(new java.util.concurrent.Callable<Object>() { java.util.concurrent.Future<Object> f1 = svc.submit(new java.util.concurrent.Callable<Object>() {
public Object call() throws Exception { public Object call() throws Exception {
Expand All @@ -109,49 +110,91 @@ public Object call() throws Exception {
f1.get(); f1.get();
f2.get(); f2.get();
} finally { } finally {
RemoteClassLoader.TESTING = false; RemoteClassLoader.TESTING_CLASS_LOAD = null;
}
}

private static final class SleepForASec implements Runnable {
@Override
public void run() {
try {
Thread.sleep(1000);
} catch (InterruptedException e) {
// Nothing
}
} }
} }


@Bug(19453) @Bug(19453)
public void testInterruption() throws Exception { public void testInterruptionOfClassCreation() throws Exception {
DummyClassLoader parent = new DummyClassLoader(TestLinkage.B.class); DummyClassLoader parent = new DummyClassLoader(TestLinkage.B.class);
final DummyClassLoader child1 = new DummyClassLoader(parent, TestLinkage.A.class); final DummyClassLoader child1 = new DummyClassLoader(parent, TestLinkage.A.class);
final DummyClassLoader child2 = new DummyClassLoader(child1, TestLinkage.class); final DummyClassLoader child2 = new DummyClassLoader(child1, TestLinkage.class);
final Callable<Object, Exception> c = (Callable) child2.load(TestLinkage.class); final Callable<Object, Exception> c = (Callable) child2.load(TestLinkage.class);
assertEquals(child2, c.getClass().getClassLoader()); assertEquals(child2, c.getClass().getClassLoader());
RemoteClassLoader.TESTING = true; RemoteClassLoader.TESTING_CLASS_LOAD = new InterruptThirdInvocation();
try { try {
ExecutorService svc = Executors.newSingleThreadExecutor(); java.util.concurrent.Future<Object> f1 = scheduleCallableLoad(c);
java.util.concurrent.Future<Object> f1 = svc.submit(new java.util.concurrent.Callable<Object>() {
public Object call() throws Exception { try {
try { f1.get();
return channel.call(c); } catch (ExecutionException ex) {
} catch (Throwable t) { // Expected
throw new Exception(t); }
}
} // verify that classes that we tried to load aren't irrevocably damaged and it's still available
}); assertEquals(String.class, channel.call(c).getClass());
} finally {
RemoteClassLoader.TESTING_CLASS_LOAD = null;
}
}

@Bug(36991)
public void testInterruptionOfClassReferenceCreation() throws Exception {
DummyClassLoader parent = new DummyClassLoader(TestLinkage.B.class);
final DummyClassLoader child1 = new DummyClassLoader(parent, TestLinkage.A.class);
final DummyClassLoader child2 = new DummyClassLoader(child1, TestLinkage.class);
final Callable<Object, Exception> c = (Callable) child2.load(TestLinkage.class);
assertEquals(child2, c.getClass().getClassLoader());
RemoteClassLoader.TESTING_CLASS_REFERENCE_LOAD = new InterruptThirdInvocation();


// This is so that the first two class loads succeed but the third fails. try {
// A better test would use semaphores rather than timing (cf. the test before this one). Future<Object> f1 = scheduleCallableLoad(c);
Thread.sleep(2500);


f1.cancel(true);
try { try {
Object o = f1.get(); f1.get();
assertEquals(String.class, o.getClass()); } catch (ExecutionException ex) {
/* TODO we really want to fail here, but this method gets run 4×, and the last 3× it gets to this point: // Expected
fail(o.toString());
*/
} catch (CancellationException x) {
// good
} }


// verify that classes that we tried to load aren't irrevocably damaged and it's still available // verify that classes that we tried to load aren't irrevocably damaged and it's still available
assertNotNull(channel.call(c)); assertEquals(String.class, channel.call(c).getClass());
} finally { } finally {
RemoteClassLoader.TESTING = false; RemoteClassLoader.TESTING_CLASS_REFERENCE_LOAD = null;
}
}

private Future<Object> scheduleCallableLoad(final Callable<Object, Exception> c) {
ExecutorService svc = Executors.newSingleThreadExecutor();
return svc.submit(new java.util.concurrent.Callable<Object>() {
public Object call() throws Exception {
try {
return channel.call(c);
} catch (Throwable t) {
throw new Exception(t);
}
}
});
}

private static class InterruptThirdInvocation implements Runnable {
private int invocationCount = 0;
@Override
public void run() {
invocationCount++;
if (invocationCount == 3) {
Thread.currentThread().interrupt();
}
} }
} }


Expand Down

0 comments on commit fe2feff

Please sign in to comment.