Author: markt
Date: Wed Nov 18 12:25:41 2009
New Revision: 881748

URL: http://svn.apache.org/viewvc?rev=881748&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=44041
Threading issue in class loading

Modified:
    tomcat/tc6.0.x/trunk/STATUS.txt
    tomcat/tc6.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoader.java
    tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml

Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL: 
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=881748&r1=881747&r2=881748&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Wed Nov 18 12:25:41 2009
@@ -142,13 +142,6 @@
   +1: markt
   -1:
 
-* Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=44041
-  Threading issue in classloading. Adds a sync so please check performance
-  Updated to use Filip's suggestion
-  http://people.apache.org/~markt/patches/2009-11-05-bug44041.patch
-  +1: markt, fhanik, kkolinko
-  -1:
-
 * Allow per instance configuration of JULI or log4j for core Tomcat logging
   Updated patch with the suggested tweak for 6.0.x so we don't break Eclipse
   integration

Modified: 
tomcat/tc6.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoader.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoader.java?rev=881748&r1=881747&r2=881748&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoader.java 
(original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoader.java 
Wed Nov 18 12:25:41 2009
@@ -1296,118 +1296,99 @@
     public Class loadClass(String name, boolean resolve)
         throws ClassNotFoundException {
 
-        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);
-        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) {
+        synchronized (name.intern()) {
             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);
+                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);
             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.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);
             }
-        }
-
-        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;
+    
+            // (0.2) Try loading the class with the system class loader, to 
prevent
+            //       the webapp from overriding J2SE classes
             try {
-                clazz = loader.loadClass(name);
+                clazz = system.loadClass(name);
                 if (clazz != null) {
-                    if (log.isDebugEnabled())
-                        log.debug("  Loading class from parent");
                     if (resolve)
                         resolveClass(clazz);
                     return (clazz);
                 }
             } catch (ClassNotFoundException e) {
-                ;
+                // Ignore
             }
-        }
-
-        // (2) Search local repositories
-        if (log.isDebugEnabled())
-            log.debug("  Searching local repositories");
-        try {
-            clazz = findClass(name);
-            if (clazz != null) {
+    
+            // (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) {
                 if (log.isDebugEnabled())
-                    log.debug("  Loading class from local repository");
-                if (resolve)
-                    resolveClass(clazz);
-                return (clazz);
+                    log.debug("  Delegating to parent classloader1 " + parent);
+                ClassLoader loader = parent;
+                if (loader == null)
+                    loader = system;
+                try {
+                    clazz = loader.loadClass(name);
+                    if (clazz != null) {
+                        if (log.isDebugEnabled())
+                            log.debug("  Loading class from parent");
+                        if (resolve)
+                            resolveClass(clazz);
+                        return (clazz);
+                    }
+                } catch (ClassNotFoundException e) {
+                    ;
+                }
             }
-        } catch (ClassNotFoundException e) {
-            ;
-        }
-
-        // (3) Delegate to parent unconditionally
-        if (!delegateLoad) {
+    
+            // (2) Search local repositories
             if (log.isDebugEnabled())
-                log.debug("  Delegating to parent classloader at end: " + 
parent);
-            ClassLoader loader = parent;
-            if (loader == null)
-                loader = system;
+                log.debug("  Searching local repositories");
             try {
-                clazz = loader.loadClass(name);
+                clazz = findClass(name);
                 if (clazz != null) {
                     if (log.isDebugEnabled())
-                        log.debug("  Loading class from parent");
+                        log.debug("  Loading class from local repository");
                     if (resolve)
                         resolveClass(clazz);
                     return (clazz);
@@ -1415,9 +1396,30 @@
             } catch (ClassNotFoundException e) {
                 ;
             }
+    
+            // (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 = loader.loadClass(name);
+                    if (clazz != null) {
+                        if (log.isDebugEnabled())
+                            log.debug("  Loading class from parent");
+                        if (resolve)
+                            resolveClass(clazz);
+                        return (clazz);
+                    }
+                } catch (ClassNotFoundException e) {
+                    ;
+                }
+            }
+    
+            throw new ClassNotFoundException(name);
         }
-
-        throw new ClassNotFoundException(name);
     }
 
 

Modified: tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?rev=881748&r1=881747&r2=881748&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Wed Nov 18 12:25:41 2009
@@ -81,6 +81,10 @@
         persistent session manager. (markt)
       </fix>
       <fix>
+        <bug>44041</bug>: Fix threading issue in WebappClassLoader that can 
lead
+        to duplicate class definition under high load. (markt/fhanik)
+      </fix>
+      <fix>
         <bug>44943</bug>: Use the same engine name in server.xml comments to
         reduce copy and pastes issues. (markt, kkolinko)
       </fix>



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

Reply via email to