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]

Reply via email to