On 06/11/2013 00:10, Konstantin Kolinko wrote: > 2013/11/6 <ma...@apache.org>: >> Author: markt >> Date: Tue Nov 5 23:06:33 2013 >> New Revision: 1539180 >> >> URL: http://svn.apache.org/r1539180 >> Log: >> Review of r1539036 by Nick Williams. >> >> copyWithoutTransformers(), as defined in the interface >> InstrumentableClassLoader, returns a ClassLoader. The start() method is not >> defined in ClassLoader, it is specific to WebappClassLoader. Furthermore, >> code calling copyWithoutTransformers() won't have access to >> WebappClassLoader to call start() reflectively if a SecurityManager is >> enabled. >> >> So, the copyWithoutTransformers() method needs to call start() before it >> returns the copied class loader. Otherwise, it will be useless to JPA >> providers and the like. >> >> Modified: >> tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java >> >> tomcat/trunk/test/org/apache/catalina/loader/TestWebappClassLoaderWeaving.java >> >> 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=1539180&r1=1539179&r2=1539180&view=diff >> ============================================================================== >> --- tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java >> (original) >> +++ tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java Tue >> Nov 5 23:06:33 2013 >> @@ -700,6 +700,12 @@ public class WebappClassLoader extends U >> result.permissionList.addAll(this.permissionList); >> result.loaderPC.putAll(this.loaderPC); >> >> + try { >> + result.start(); >> + } catch (LifecycleException e) { >> + throw new IllegalStateException(e); >> + } > > There is "result.started = this.started;" assignment a few lines above. > Shouldn't the above call be wrapped into if(!started){ ... } ?
Hmm. The Lifecycle of the web app class loader doesn't really apply to this copy so I suspect the copy always needs to be in the started. > LifecycleBase.start() logs an INFO message if start() is called repeatedly. > WebappClassLoader implements start() by itself and does some work > without checking for the "started" flag. There might be observable > consequences of that. I'm wondering if the started flag needs to be removed and replaced with the appropriate test of lifecycle state. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
