Author: markt Date: Mon Oct 30 19:51:44 2006 New Revision: 469360 URL: http://svn.apache.org/viewvc?view=rev&rev=469360 Log: Fix bug 37458 where concurrent attempts to load the same class could set entry.manifest to null in one thread whilst the other thread(s) were operating under the assumption it was still non-null. I had to add a largish sync block to do this (since the package sealing check also uses entry) but it is on <code>entry</code> rather than <code>this</code> so the impact should be minimal. I also fixed a rare but possible IAE in the package definition section. Looking at the dev archive, at least one user has seen this problem so it isn't entirely theoretical.
Modified: tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/loader/WebappClassLoader.java tomcat/container/tc5.5.x/webapps/docs/changelog.xml Modified: tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/loader/WebappClassLoader.java URL: http://svn.apache.org/viewvc/tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/loader/WebappClassLoader.java?view=diff&rev=469360&r1=469359&r2=469360 ============================================================================== --- tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/loader/WebappClassLoader.java (original) +++ tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/loader/WebappClassLoader.java Mon Oct 30 19:51:44 2006 @@ -1793,63 +1793,59 @@ if (clazz != null) return clazz; - synchronized (this) { + synchronized (entry) { if (entry.binaryContent == null && entry.loadedClass == null) throw new ClassNotFoundException(name); - } - // Looking up the package - String packageName = null; - int pos = name.lastIndexOf('.'); - if (pos != -1) - packageName = name.substring(0, pos); - - Package pkg = null; - - if (packageName != null) { - - pkg = getPackage(packageName); - - // Define the package (if null) - if (pkg == null) { - if (entry.manifest == null) { - definePackage(packageName, null, null, null, null, null, - null, null); - } else { - definePackage(packageName, entry.manifest, entry.codeBase); + // Looking up the package + String packageName = null; + int pos = name.lastIndexOf('.'); + if (pos != -1) + packageName = name.substring(0, pos); + + Package pkg = null; + + if (packageName != null) { + + synchronized(this) { + pkg = getPackage(packageName); + + // Define the package (if null) + if (pkg == null) { + if (entry.manifest == null) { + definePackage(packageName, null, null, null, null, + null, null, null); + } else { + definePackage(packageName, entry.manifest, + entry.codeBase); + } + } } } - } - - // Create the code source object - CodeSource codeSource = - new CodeSource(entry.codeBase, entry.certificates); - - if (securityManager != null) { - - // Checking sealing - if (pkg != null) { - boolean sealCheck = true; - if (pkg.isSealed()) { - sealCheck = pkg.isSealed(entry.codeBase); - } else { - sealCheck = (entry.manifest == null) - || !isPackageSealed(packageName, entry.manifest); + if (securityManager != null) { + + // Checking sealing + if (pkg != null) { + boolean sealCheck = true; + if (pkg.isSealed()) { + sealCheck = pkg.isSealed(entry.codeBase); + } else { + sealCheck = (entry.manifest == null) + || !isPackageSealed(packageName, entry.manifest); + } + if (!sealCheck) + throw new SecurityException + ("Sealing violation loading " + name + " : Package " + + packageName + " is sealed."); } - if (!sealCheck) - throw new SecurityException - ("Sealing violation loading " + name + " : Package " - + packageName + " is sealed."); + } - } - - synchronized (this) { if (entry.loadedClass == null) { clazz = defineClass(name, entry.binaryContent, 0, entry.binaryContent.length, - codeSource); + new CodeSource(entry.codeBase, entry.certificates)); entry.loadedClass = clazz; entry.binaryContent = null; entry.source = null; Modified: tomcat/container/tc5.5.x/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/container/tc5.5.x/webapps/docs/changelog.xml?view=diff&rev=469360&r1=469359&r2=469360 ============================================================================== --- tomcat/container/tc5.5.x/webapps/docs/changelog.xml (original) +++ tomcat/container/tc5.5.x/webapps/docs/changelog.xml Mon Oct 30 19:51:44 2006 @@ -38,6 +38,11 @@ the requirement that the name must be unique. (markt) </fix> <fix> + <bug>37458</bug>: Add syncs to the WebappClassloader to address + rare issues when multiple threads attempt to load the same class + concurrently. (markt) + </fix> + <fix> <bug>39436</bug>: Correct MIME type for SVG. (markt) </fix> <fix> --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]