Re-rolled: Reduces chances of deadlock when CompilingClassLoader and MultiParentClassLoader are concurrently accessed from multiple threads.
Review by: zundel@google.com
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@10452 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/dev/shell/CompilingClassLoader.java b/dev/core/src/com/google/gwt/dev/shell/CompilingClassLoader.java
index 213b4dd..6b05280 100644
--- a/dev/core/src/com/google/gwt/dev/shell/CompilingClassLoader.java
+++ b/dev/core/src/com/google/gwt/dev/shell/CompilingClassLoader.java
@@ -70,6 +70,7 @@
import java.util.SortedSet;
import java.util.Stack;
import java.util.TreeSet;
+import java.util.concurrent.locks.ReentrantLock;
/**
* An isolated {@link ClassLoader} for running all user code. All user files are
@@ -353,23 +354,11 @@
public MultiParentClassLoader(ClassLoader parent, ClassLoader resources) {
super(parent);
+ assert parent != null;
this.resources = resources;
}
@Override
- public Class<?> loadClass(String name) throws ClassNotFoundException {
- try {
- return getParent().loadClass(name);
- } catch (Throwable t) {
- // Make a second attempt not only on ClassNotFoundExceptions, but also errors like
- // ClassCircularityError
- Class c = findClass(name);
- resolveClass(c);
- return c;
- }
- }
-
- @Override
protected synchronized Class<?> findClass(String name)
throws ClassNotFoundException {
String resourceName = name.replace('.', '/') + ".class";
@@ -380,6 +369,28 @@
byte[] bytes = Util.readURLAsBytes(url);
return defineClass(name, bytes, 0, bytes.length);
}
+
+ @Override
+ protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
+ try {
+ Class c = findLoadedClass(name);
+ if (c != null) {
+ if (resolve) {
+ resolveClass(c);
+ }
+ return c;
+ }
+ return getParent().loadClass(name);
+ } catch (Throwable t) {
+ // Make a second attempt not only on ClassNotFoundExceptions, but also errors like
+ // ClassCircularityError
+ Class c = findClass(name);
+ if (resolve) {
+ resolveClass(c);
+ }
+ return c;
+ }
+ }
}
/**
@@ -706,6 +717,11 @@
}
/**
+ * Only loads bootstrap classes, specifically excluding classes from the classpath.
+ */
+ private static final ClassLoader bootstrapClassLoader = new ClassLoader(null) { };
+
+ /**
* The names of the bridge classes.
*/
private static final Map<String, Class<?>> BRIDGE_CLASS_NAMES = new HashMap<String, Class<?>>();
@@ -902,6 +918,8 @@
*/
private boolean isInjectingClass = false;
+ private final ReentrantLock loadLock = new ReentrantLock();
+
private final TreeLogger logger;
private final Set<String> scriptOnlyClasses = new HashSet<String>();
@@ -1044,8 +1062,7 @@
}
@Override
- protected synchronized Class<?> findClass(String className)
- throws ClassNotFoundException {
+ protected Class<?> findClass(String className) throws ClassNotFoundException {
if (className == null) {
throw new ClassNotFoundException("null class name",
new NullPointerException());
@@ -1057,87 +1074,132 @@
// happens to look.
return ClassLoader.getSystemClassLoader().loadClass(className);
}
-
- if (scriptOnlyClasses.contains(className)) {
- // Allow the child ClassLoader to handle this
- throw new ClassNotFoundException();
- }
- // Check for a bridge class that spans hosted and user space.
- if (BRIDGE_CLASS_NAMES.containsKey(className)) {
- return BRIDGE_CLASS_NAMES.get(className);
- }
+ loadLock.lock();
+ try {
- // Get the bytes, compiling if necessary.
- byte[] classBytes = findClassBytes(className);
- if (classBytes == null) {
- throw new ClassNotFoundException(className);
- }
-
- if (HasAnnotation.hasAnnotation(classBytes, GwtScriptOnly.class)) {
- scriptOnlyClasses.add(className);
- maybeInitializeScriptOnlyClassLoader();
- return Class.forName(className, true, scriptOnlyClassLoader);
- }
-
- /*
- * Prevent reentrant problems where classes that need to be injected have
- * circular dependencies on one another via JSNI and inheritance. This check
- * ensures that a class's supertype can refer to the subtype (static
- * members, etc) via JSNI references by ensuring that the Class for the
- * subtype will have been defined before injecting the JSNI for the
- * supertype.
- */
- boolean localInjection;
- if (!isInjectingClass) {
- localInjection = isInjectingClass = true;
- } else {
- localInjection = false;
- }
-
- Class<?> newClass = defineClass(className, classBytes, 0, classBytes.length);
- if (className.equals(JavaScriptHost.class.getName())) {
- javaScriptHostClass = newClass;
- updateJavaScriptHost();
- }
-
- /*
- * We have to inject the JSNI code after defining the class, since dispId
- * assignment is based around reflection on Class objects. Don't inject JSNI
- * when loading a JSO interface class; just wait until the implementation
- * class is loaded.
- */
- if (!classRewriter.isJsoIntf(className)) {
- CompilationUnit unit = getUnitForClassName(canonicalizeClassName(className));
- if (unit != null) {
- toInject.push(unit);
+ if (scriptOnlyClasses.contains(className)) {
+ // Allow the child ClassLoader to handle this
+ throw new ClassNotFoundException();
}
- }
- if (localInjection) {
- try {
+ // Check for a bridge class that spans hosted and user space.
+ if (BRIDGE_CLASS_NAMES.containsKey(className)) {
+ return BRIDGE_CLASS_NAMES.get(className);
+ }
+
+ // Get the bytes, compiling if necessary.
+ byte[] classBytes = findClassBytes(className);
+ if (classBytes == null) {
+ throw new ClassNotFoundException(className);
+ }
+
+ if (HasAnnotation.hasAnnotation(classBytes, GwtScriptOnly.class)) {
+ scriptOnlyClasses.add(className);
+ maybeInitializeScriptOnlyClassLoader();
+
/*
- * Can't use an iterator here because calling injectJsniFor may cause
- * additional entries to be added.
+ * Release the lock before side-loading from scriptOnlyClassLoader. This prevents deadlock
+ * conditions when a class from scriptOnlyClassLoader ends up trying to call back into this
+ * classloader from another thread.
*/
- while (toInject.size() > 0) {
- CompilationUnit unit = toInject.remove(0);
- if (!alreadyInjected.contains(unit)) {
- injectJsniMethods(unit);
- alreadyInjected.add(unit);
- }
+ loadLock.unlock();
+
+ // Also don't run the static initializer to lower the risk of deadlock.
+ return Class.forName(className, false, scriptOnlyClassLoader);
+ }
+
+ /*
+ * Prevent reentrant problems where classes that need to be injected have
+ * circular dependencies on one another via JSNI and inheritance. This check
+ * ensures that a class's supertype can refer to the subtype (static
+ * members, etc) via JSNI references by ensuring that the Class for the
+ * subtype will have been defined before injecting the JSNI for the
+ * supertype.
+ */
+ boolean localInjection;
+ if (!isInjectingClass) {
+ localInjection = isInjectingClass = true;
+ } else {
+ localInjection = false;
+ }
+
+ Class<?> newClass = defineClass(className, classBytes, 0, classBytes.length);
+ if (className.equals(JavaScriptHost.class.getName())) {
+ javaScriptHostClass = newClass;
+ updateJavaScriptHost();
+ }
+
+ /*
+ * We have to inject the JSNI code after defining the class, since dispId
+ * assignment is based around reflection on Class objects. Don't inject JSNI
+ * when loading a JSO interface class; just wait until the implementation
+ * class is loaded.
+ */
+ if (!classRewriter.isJsoIntf(className)) {
+ CompilationUnit unit = getUnitForClassName(canonicalizeClassName(className));
+ if (unit != null) {
+ toInject.push(unit);
}
- } finally {
- isInjectingClass = false;
+ }
+
+ if (localInjection) {
+ try {
+ /*
+ * Can't use an iterator here because calling injectJsniFor may cause
+ * additional entries to be added.
+ */
+ while (toInject.size() > 0) {
+ CompilationUnit unit = toInject.remove(0);
+ if (!alreadyInjected.contains(unit)) {
+ injectJsniMethods(unit);
+ alreadyInjected.add(unit);
+ }
+ }
+ } finally {
+ isInjectingClass = false;
+ }
+ }
+
+ if (className.equals("com.google.gwt.core.client.GWT")) {
+ gwtClass = newClass;
+ updateGwtClass();
+ }
+
+ return newClass;
+ } finally {
+ if (loadLock.isLocked()) {
+ loadLock.unlock();
}
}
+ }
- if (className.equals("com.google.gwt.core.client.GWT")) {
- gwtClass = newClass;
- updateGwtClass();
+ /**
+ * Remove some of the excess locking that we'd normally inherit from loadClass.
+ */
+ @Override
+ protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
+ Class c = findLoadedClass(name);
+ if (c != null) {
+ if (resolve) {
+ resolveClass(c);
+ }
+ return c;
}
-
- return newClass;
+
+ assert getParent() == null;
+
+ try {
+ c = bootstrapClassLoader.loadClass(name);
+ } catch (ClassNotFoundException e) {
+ c = findClass(name);
+ }
+
+ if (resolve) {
+ resolveClass(c);
+ }
+
+ return c;
}
void clear() {
