On 19/05/2014 22:41, Konstantin Kolinko wrote: > 2014-05-19 23:38 GMT+04:00 <ma...@apache.org>: >> Author: markt >> Date: Mon May 19 19:38:13 2014 >> New Revision: 1596004 >> >> URL: http://svn.apache.org/r1596004 >> Log: >> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56387 >> Refactor handling of class loading attempts after web application stop. >> >> Modified: >> tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java >> tomcat/trunk/webapps/docs/changelog.xml >> >> Modified: tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java >> URL: >> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java?rev=1596004&r1=1596003&r2=1596004&view=diff >> ============================================================================== >> --- tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java >> (original) >> +++ tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java Mon >> May 19 19:38:13 2014 >> @@ -852,10 +852,7 @@ public class WebappClassLoader extends U >> if (log.isDebugEnabled()) >> log.debug(" findClass(" + name + ")"); >> >> - // Cannot load anything from local repositories if class loader is >> stopped >> - if (!state.isAvailable()) { >> - throw new ClassNotFoundException(name); >> - } >> + checkStateForClassLoading(name); > > The old code threw a CNFE here. The new code throws nothing. > >> // (1) Permission to define this class when using a SecurityManager >> if (securityManager != null) { >> @@ -1215,14 +1212,8 @@ public class WebappClassLoader extends U >> log.debug("loadClass(" + name + ", " + resolve + ")"); >> Class<?> clazz = null; >> >> - // Log access to stopped classloader >> - if (!state.isAvailable()) { >> - try { >> - throw new IllegalStateException(); >> - } catch (IllegalStateException e) { >> - log.info(sm.getString("webappClassLoader.stopped", name), >> e); >> - } >> - } >> + // Log access to stopped class loader >> + checkStateForClassLoading(name); >> >> // (0) Check our previously loaded local class cache >> clazz = findLoadedClass0(name); >> @@ -1331,7 +1322,19 @@ public class WebappClassLoader extends U >> } >> >> throw new ClassNotFoundException(name); >> + } >> + >> >> + protected void checkStateForClassLoading(String className) { >> + // It is not permitted to load new classes once the web application >> has >> + // been stopped. >> + if (!state.isAvailable()) { >> + String msg = sm.getString("webappClassLoader.stopped", >> className); >> + IllegalStateException cause = new IllegalStateException(msg); >> + ClassNotFoundException cnfe = new ClassNotFoundException(); > > It might be better to use "new ClassNotFoundException(className)". > Though from the second look I see that the log message already > mentions the class name. > >> + cnfe.initCause(cause); >> + log.info(msg, cnfe); > > This method does not throw anything... Thus it is not really a > "ClassNotFoundException", because a class is not really "not found", > but execution continues further and it may occasionally find the > class.
bah. I meant to throw CNFE at the end of that but got distracted thinking about logging. I'll fix that in a sec. >> + } >> } >> > > As a test case it looks that > org.apache.catalina.valves.TestStuckThreadDetectionValve > occasionally causes this exception, as was noted by Buildbot > (though that buildbot run was before this change, so it is not illustrative). > http://ci.apache.org/projects/tomcat/tomcat8/1595898/ Lets see what happens when I fix the above. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
