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.