Revision: 10449
Author:   to...@google.com
Date:     Wed Jul 13 06:37:30 2011
Log:      A rollback of the following change (fails on some JVMs):

Reduces chances of deadlock when CompilingClassLoader and MultiParentClassLoader are concurrently accessed from multiple threads.

http://code.google.com/p/google-web-toolkit/source/detail?r=10449

Modified:
 /trunk/dev/core/src/com/google/gwt/dev/shell/CompilingClassLoader.java

=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/shell/CompilingClassLoader.java Tue Jul 12 13:27:01 2011 +++ /trunk/dev/core/src/com/google/gwt/dev/shell/CompilingClassLoader.java Wed Jul 13 06:37:30 2011
@@ -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,10 +353,22 @@

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 {
@@ -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);
     }
-
-    loadLock.lock();
-    try {
-
-      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);
-      }
-
-      // 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();
-
-        /*
- * 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();
+
+    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);
+    }
+
+    // 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 (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;
       }
     }
-  }
-
-  /**
- * 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 {
-      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;
-        }
-        if (t instanceof Error) {
-          throw (Error) t;
-        }
-        if (t instanceof RuntimeException) {
-          throw (RuntimeException) t;
-        }
-        throw new RuntimeException("Unexpected exception", t);
-      }
-    } catch (ClassNotFoundException e) {
-      c = findClass(name);
-    }
-
-    if (resolve) {
-      resolveClass(c);
-    }
-
-    return c;
+
+    if (className.equals("com.google.gwt.core.client.GWT")) {
+      gwtClass = newClass;
+      updateGwtClass();
+    }
+
+    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.

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Reply via email to