You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2010/03/25 20:44:41 UTC

svn commit: r927565 - in /tomcat/trunk/java/org/apache: catalina/loader/WebappClassLoader.java jasper/servlet/JasperLoader.java

Author: markt
Date: Thu Mar 25 19:44:41 2010
New Revision: 927565

URL: http://svn.apache.org/viewvc?rev=927565&view=rev
Log:
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.

Modified:
    tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java
    tomcat/trunk/java/org/apache/jasper/servlet/JasperLoader.java

Modified: tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java?rev=927565&r1=927564&r2=927565&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java (original)
+++ tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java Thu Mar 25 19:44:41 2010
@@ -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;

Modified: tomcat/trunk/java/org/apache/jasper/servlet/JasperLoader.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/servlet/JasperLoader.java?rev=927565&r1=927564&r2=927565&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/servlet/JasperLoader.java (original)
+++ tomcat/trunk/java/org/apache/jasper/servlet/JasperLoader.java Thu Mar 25 19:44:41 2010
@@ -89,7 +89,7 @@ public class JasperLoader extends URLCla
      * @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;                



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org