WebappClassLoader may fail to load class from local repository when multiple threads are trying to load same class concurrently. Need to change method findClassInternal (line 1577 ), if ((entry == null) || (entry.binaryContent == null)) throw new ClassNotFoundException(name); to if ((entry == null) || (entry.binaryContent == null && entry.loadedClass==null)) throw new ClassNotFoundException(name); since after one thread calls defineClass() on the entry, the entry's binaryContent is set to null, but in this case, the class is already loaded, so the code should not throw ClassNotFoundException. Also in method findClassInternal, following code still use double checked locking, which is proven broken - if (entry.loadedClass == null) { synchronized (this) { if (entry.loadedClass == null) { clazz = defineClass(name, entry.binaryContent, 0, entry.binaryContent.length, codeSource); entry.loadedClass = clazz; entry.binaryContent = null; entry.source = null; entry.codeBase = null; entry.manifest = null; entry.certificates = null; } else { clazz = entry.loadedClass; } } } else { clazz = entry.loadedClass; }
Are you reporting an actual problem, or is it just reading of the source code ? The first part could make sense: I added entry.binaryContent = null after the rest, so the condition might be wrong.
This is a reproducible real problem. Our server uses tomcat 5.0 as a servlet engine. When it runs multiple worker threads to execute a JavaScritpt (using Rhino) on the server side, it will occasionally but consistently throw NoClassDefFoundError on a class in Rhino library, which is located in the WEB-INF\lib directory. We eventually figure out if two threads start to load same class concurrently for WEB-INF/lib, one of them will fail if a) the first thread manages to call defineClass on the resource entry and set the binaryContent to null; b) when the second adds its entry to the HashMap, it will find an entry for that class already exists, so it will return the existing entry (the one whose binaryContent is null); For the second part, it has been reported by some other people. Please see the following link http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html It would be better to replace the code with single checked locking and make it thread-safe.
You're not supposed to fork threads in your webapp. As for the double check thing, try to post productive stuff, thanks.
It's well and good that you're "not supposed to fork threads", but let's get real -- this is something that code needs to do more often than the strict letter of J2EE lets on.
"You're not supposed to fork threads in your webapp." Oh, this is the simpler fix, thanks.
The J2EE spec forbids it, and experience shows it can cause many tricky problems. The change for entry.loadedClass == null is good, since I added the entry.binaryContent = null without seeing that the check would need an update. This was introduced in 5.0.3.
Thanks Remy. It is forbidden to create threads in ejb container, and it is not recommended to spawn threads in servlet container, but don't recall this is strictly forbidden by the servlet spec (I might be wrong here).