Address various class-loader deadlock / sync issues

https://issues.apache.org/bugzilla/show_bug.cgi?id=44041
https://issues.apache.org/bugzilla/show_bug.cgi?id=48694
https://issues.apache.org/bugzilla/show_bug.cgi?id=48903

Whilst parallel class-loading would be a nice feature, the various issues that have emerged have demonstrated that anything other than synchronized(this) is likely to cause issues.

Parallel class-loading will be explored for Tomcat 7 (disabled by default) and ported back to 6.0.x when proven to be stable.

git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@927565 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/java/org/apache/catalina/loader/WebappClassLoader.java b/java/org/apache/catalina/loader/WebappClassLoader.java
index 64a7259..a7527e1 100644 (file)
--- a/java/org/apache/catalina/loader/WebappClassLoader.java
+++ b/java/org/apache/catalina/loader/WebappClassLoader.java
@@ -1423,48 +1423,84 @@ public class WebappClassLoader
      * @exception ClassNotFoundException if the class was not found
      */
     @Override
-    public Class<?> loadClass(String name, boolean resolve)
+    public synchronized Class<?> loadClass(String name, boolean resolve)
         throws ClassNotFoundException {
 
-        synchronized (name.intern()) {
-            if (log.isDebugEnabled())
-                log.debug("loadClass(" + name + ", " + resolve + ")");
-            Class<?> clazz = null;
-    
-            // Log access to stopped classloader
-            if (!started) {
-                try {
-                    throw new IllegalStateException();
-                } catch (IllegalStateException e) {
-                    log.info(sm.getString("webappClassLoader.stopped", name), e);
-                }
+        if (log.isDebugEnabled())
+            log.debug("loadClass(" + name + ", " + resolve + ")");
+        Class<?> clazz = null;
+
+        // Log access to stopped classloader
+        if (!started) {
+            try {
+                throw new IllegalStateException();
+            } catch (IllegalStateException e) {
+                log.info(sm.getString("webappClassLoader.stopped", name), e);
             }
-    
-            // (0) Check our previously loaded local class cache
-            clazz = findLoadedClass0(name);
+        }
+
+        // (0) Check our previously loaded local class cache
+        clazz = findLoadedClass0(name);
+        if (clazz != null) {
+            if (log.isDebugEnabled())
+                log.debug("  Returning class from cache");
+            if (resolve)
+                resolveClass(clazz);
+            return (clazz);
+        }
+
+        // (0.1) Check our previously loaded class cache
+        clazz = findLoadedClass(name);
+        if (clazz != null) {
+            if (log.isDebugEnabled())
+                log.debug("  Returning class from cache");
+            if (resolve)
+                resolveClass(clazz);
+            return (clazz);
+        }
+
+        // (0.2) Try loading the class with the system class loader, to prevent
+        //       the webapp from overriding J2SE classes
+        try {
+            clazz = system.loadClass(name);
             if (clazz != null) {
-                if (log.isDebugEnabled())
-                    log.debug("  Returning class from cache");
                 if (resolve)
                     resolveClass(clazz);
                 return (clazz);
             }
-    
-            // (0.1) Check our previously loaded class cache
-            clazz = findLoadedClass(name);
-            if (clazz != null) {
-                if (log.isDebugEnabled())
-                    log.debug("  Returning class from cache");
-                if (resolve)
-                    resolveClass(clazz);
-                return (clazz);
+        } catch (ClassNotFoundException e) {
+            // Ignore
+        }
+
+        // (0.5) Permission to access this class when using a SecurityManager
+        if (securityManager != null) {
+            int i = name.lastIndexOf('.');
+            if (i >= 0) {
+                try {
+                    securityManager.checkPackageAccess(name.substring(0,i));
+                } catch (SecurityException se) {
+                    String error = "Security Violation, attempt to use " +
+                        "Restricted Class: " + name;
+                    log.info(error, se);
+                    throw new ClassNotFoundException(error, se);
+                }
             }
-    
-            // (0.2) Try loading the class with the system class loader, to prevent
-            //       the webapp from overriding J2SE classes
+        }
+
+        boolean delegateLoad = delegate || filter(name);
+
+        // (1) Delegate to our parent if requested
+        if (delegateLoad) {
+            if (log.isDebugEnabled())
+                log.debug("  Delegating to parent classloader1 " + parent);
+            ClassLoader loader = parent;
+            if (loader == null)
+                loader = system;
             try {
-                clazz = system.loadClass(name);
+                clazz = Class.forName(name, false, loader);
                 if (clazz != null) {
+                    if (log.isDebugEnabled())
+                        log.debug("  Loading class from parent");
                     if (resolve)
                         resolveClass(clazz);
                     return (clazz);
@@ -1472,53 +1508,36 @@ public class WebappClassLoader
             } catch (ClassNotFoundException e) {
                 // Ignore
             }
-    
-            // (0.5) Permission to access this class when using a SecurityManager
-            if (securityManager != null) {
-                int i = name.lastIndexOf('.');
-                if (i >= 0) {
-                    try {
-                        securityManager.checkPackageAccess(name.substring(0,i));
-                    } catch (SecurityException se) {
-                        String error = "Security Violation, attempt to use " +
-                            "Restricted Class: " + name;
-                        log.info(error, se);
-                        throw new ClassNotFoundException(error, se);
-                    }
-                }
-            }
-    
-            boolean delegateLoad = delegate || filter(name);
-    
-            // (1) Delegate to our parent if requested
-            if (delegateLoad) {
+        }
+
+        // (2) Search local repositories
+        if (log.isDebugEnabled())
+            log.debug("  Searching local repositories");
+        try {
+            clazz = findClass(name);
+            if (clazz != null) {
                 if (log.isDebugEnabled())
-                    log.debug("  Delegating to parent classloader1 " + parent);
-                ClassLoader loader = parent;
-                if (loader == null)
-                    loader = system;
-                try {
-                    clazz = Class.forName(name, false, loader);
-                    if (clazz != null) {
-                        if (log.isDebugEnabled())
-                            log.debug("  Loading class from parent");
-                        if (resolve)
-                            resolveClass(clazz);
-                        return (clazz);
-                    }
-                } catch (ClassNotFoundException e) {
-                    // Ignore
-                }
+                    log.debug("  Loading class from local repository");
+                if (resolve)
+                    resolveClass(clazz);
+                return (clazz);
             }
-    
-            // (2) Search local repositories
+        } catch (ClassNotFoundException e) {
+            // Ignore
+        }
+
+        // (3) Delegate to parent unconditionally
+        if (!delegateLoad) {
             if (log.isDebugEnabled())
-                log.debug("  Searching local repositories");
+                log.debug("  Delegating to parent classloader at end: " + parent);
+            ClassLoader loader = parent;
+            if (loader == null)
+                loader = system;
             try {
-                clazz = findClass(name);
+                clazz = Class.forName(name, false, loader);
                 if (clazz != null) {
                     if (log.isDebugEnabled())
-                        log.debug("  Loading class from local repository");
+                        log.debug("  Loading class from parent");
                     if (resolve)
                         resolveClass(clazz);
                     return (clazz);
@@ -1526,30 +1545,10 @@ public class WebappClassLoader
             } catch (ClassNotFoundException e) {
                 // Ignore
             }
-    
-            // (3) Delegate to parent unconditionally
-            if (!delegateLoad) {
-                if (log.isDebugEnabled())
-                    log.debug("  Delegating to parent classloader at end: " + parent);
-                ClassLoader loader = parent;
-                if (loader == null)
-                    loader = system;
-                try {
-                    clazz = Class.forName(name, false, loader);
-                    if (clazz != null) {
-                        if (log.isDebugEnabled())
-                            log.debug("  Loading class from parent");
-                        if (resolve)
-                            resolveClass(clazz);
-                        return (clazz);
-                    }
-                } catch (ClassNotFoundException e) {
-                    // Ignore
-                }
-            }
-    
-            throw new ClassNotFoundException(name);
         }
+
+        throw new ClassNotFoundException(name);
+
     }
 
 
@@ -2537,7 +2536,7 @@ public class WebappClassLoader
         if (clazz != null)
             return clazz;
 
-        synchronized (name.intern()) {
+        synchronized (this) {
             clazz = entry.loadedClass;
             if (clazz != null)
                 return clazz;

diff --git a/java/org/apache/jasper/servlet/JasperLoader.java b/java/org/apache/jasper/servlet/JasperLoader.java
index 8261040..731d3c4 100644 (file)
--- a/java/org/apache/jasper/servlet/JasperLoader.java
+++ b/java/org/apache/jasper/servlet/JasperLoader.java
@@ -89,7 +89,7 @@ public class JasperLoader extends URLClassLoader {
      * @exception ClassNotFoundException if the class was not found
      */                                    
     @Override
-    public Class<?> loadClass(final String name, boolean resolve)
+    public synchronized Class<?> loadClass(final String name, boolean resolve)
         throws ClassNotFoundException {
 
         Class<?> clazz = null;