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