A rollback of the following change (fails on some JVMs):
Reduces chances of deadlock when CompilingClassLoader and MultiParentClassLoader are concurrently accessed from multiple threads.
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@10449 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 cf509b9..213b4dd 100644
--- a/dev/core/src/com/google/gwt/dev/shell/CompilingClassLoader.java
+++ b/dev/core/src/com/google/gwt/dev/shell/CompilingClassLoader.java
@@ -70,7 +70,6 @@
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
@@ -354,11 +353,23 @@
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";
@@ -369,28 +380,6 @@
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;
- }
- }
}
/**
@@ -906,8 +895,6 @@
private final DispatchClassInfoOracle dispClassInfoOracle = new DispatchClassInfoOracle();
- private final Method findBootstrapClassMethod = getFindBootstrapClassMethod();
-
private Class<?> gwtClass, javaScriptHostClass;
/**
@@ -915,8 +902,6 @@
*/
private boolean isInjectingClass = false;
- private final ReentrantLock loadLock = new ReentrantLock();
-
private final TreeLogger logger;
private final Set<String> scriptOnlyClasses = new HashSet<String>();
@@ -1059,7 +1044,8 @@
}
@Override
- protected Class<?> findClass(String className) throws ClassNotFoundException {
+ protected synchronized Class<?> findClass(String className)
+ throws ClassNotFoundException {
if (className == null) {
throw new ClassNotFoundException("null class name",
new NullPointerException());
@@ -1071,148 +1057,87 @@
// happens to look.
return ClassLoader.getSystemClassLoader().loadClass(className);
}
+
+ if (scriptOnlyClasses.contains(className)) {
+ // Allow the child ClassLoader to handle this
+ throw new ClassNotFoundException();
+ }
- loadLock.lock();
- try {
+ // Check for a bridge class that spans hosted and user space.
+ if (BRIDGE_CLASS_NAMES.containsKey(className)) {
+ return BRIDGE_CLASS_NAMES.get(className);
+ }
- if (scriptOnlyClasses.contains(className)) {
- // Allow the child ClassLoader to handle this
- throw new ClassNotFoundException();
- }
+ // Get the bytes, compiling if necessary.
+ byte[] classBytes = findClassBytes(className);
+ if (classBytes == null) {
+ throw new ClassNotFoundException(className);
+ }
- // Check for a bridge class that spans hosted and user space.
- if (BRIDGE_CLASS_NAMES.containsKey(className)) {
- return BRIDGE_CLASS_NAMES.get(className);
- }
+ if (HasAnnotation.hasAnnotation(classBytes, GwtScriptOnly.class)) {
+ scriptOnlyClasses.add(className);
+ maybeInitializeScriptOnlyClassLoader();
+ return Class.forName(className, true, scriptOnlyClassLoader);
+ }
- // Get the bytes, compiling if necessary.
- byte[] classBytes = findClassBytes(className);
- if (classBytes == null) {
- throw new ClassNotFoundException(className);
- }
+ /*
+ * 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;
+ }
- if (HasAnnotation.hasAnnotation(classBytes, GwtScriptOnly.class)) {
- scriptOnlyClasses.add(className);
- maybeInitializeScriptOnlyClassLoader();
+ Class<?> newClass = defineClass(className, classBytes, 0, classBytes.length);
+ if (className.equals(JavaScriptHost.class.getName())) {
+ javaScriptHostClass = newClass;
+ updateJavaScriptHost();
+ }
- /*
- * 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.
- */
- 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);
- }
- }
-
- 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();
+ /*
+ * 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);
}
}
- }
- /**
- * 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;
- }
-
- assert getParent() == null;
-
- try {
+ if (localInjection) {
try {
- c = (Class) findBootstrapClassMethod.invoke(this, name);
- } catch (IllegalAccessException e) {
- throw new RuntimeException("Unexpected exception", e);
- } catch (InvocationTargetException e) {
- Throwable t = e.getCause();
- if (t instanceof ClassNotFoundException) {
- throw (ClassNotFoundException) t;
+ /*
+ * 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);
+ }
}
- if (t instanceof Error) {
- throw (Error) t;
- }
- if (t instanceof RuntimeException) {
- throw (RuntimeException) t;
- }
- throw new RuntimeException("Unexpected exception", t);
+ } finally {
+ isInjectingClass = false;
}
- } catch (ClassNotFoundException e) {
- c = findClass(name);
}
-
- if (resolve) {
- resolveClass(c);
+
+ if (className.equals("com.google.gwt.core.client.GWT")) {
+ gwtClass = newClass;
+ updateGwtClass();
}
-
- return c;
+
+ return newClass;
}
void clear() {
@@ -1362,16 +1287,6 @@
}
}
- private Method getFindBootstrapClassMethod() {
- try {
- Method m = ClassLoader.class.getDeclaredMethod("findBootstrapClass0", String.class);
- m.setAccessible(true);
- return m;
- } catch (Exception e) {
- throw new RuntimeException("Unable to find ClassLoader.findBootstrapClass0.", e);
- }
- }
-
/**
* Returns the compilationUnit corresponding to the className. For nested
* classes, the unit corresponding to the top level type is returned.
