From 774b9657eccbb70d2f20df7d6df6a12e096b7b45 Mon Sep 17 00:00:00 2001 From: markt Date: Thu, 25 Mar 2010 19:44:41 +0000 Subject: [PATCH] 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 --- .../apache/catalina/loader/WebappClassLoader.java | 191 ++++++++++----------- java/org/apache/jasper/servlet/JasperLoader.java | 2 +- 2 files changed, 96 insertions(+), 97 deletions(-) diff --git a/java/org/apache/catalina/loader/WebappClassLoader.java b/java/org/apache/catalina/loader/WebappClassLoader.java index 64a725963..a7527e117 100644 --- 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 826104068..731d3c4f7 100644 --- 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; -- 2.11.0