You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by jf...@apache.org on 2005/09/28 01:42:53 UTC
cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/loader WebappClassLoader.java
jfarcand 2005/09/27 16:42:53
Modified: catalina/src/share/org/apache/catalina/loader
WebappClassLoader.java
Log:
Port fix from SJSAS.
Patch submitted by: Jan Luehe
Revision Changes Path
1.51 +16 -3 jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/loader/WebappClassLoader.java
Index: WebappClassLoader.java
===================================================================
RCS file: /home/cvs/jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/loader/WebappClassLoader.java,v
retrieving revision 1.50
retrieving revision 1.51
diff -u -r1.50 -r1.51
--- WebappClassLoader.java 8 Sep 2005 15:00:54 -0000 1.50
+++ WebappClassLoader.java 27 Sep 2005 23:42:53 -0000 1.51
@@ -1470,6 +1470,21 @@
*/
public void stop() throws LifecycleException {
+ /*
+ * Clear the IntrospectionUtils cache.
+ *
+ * Implementation note:
+ * Any reference to IntrospectionUtils which may cause the static
+ * initalizer of that class to be invoked must occur prior to setting
+ * the started flag to FALSE, because the static initializer of
+ * IntrospectionUtils makes a call to
+ * org.apache.commons.logging.LogFactory.getLog(), which ultimately
+ * calls the loadClass() method of the thread context classloader,
+ * which is the same as this classloader, whose impl throws a
+ * ThreadDeath if the started flag has been set to FALSE.
+ */
+ IntrospectionUtils.clear();
+
started = false;
int length = files.length;
@@ -1515,8 +1530,6 @@
org.apache.commons.logging.LogFactory.release(this);
// Clear the classloader reference in the VM's bean introspector
java.beans.Introspector.flushCaches();
- // Clear the IntrospectionUtils cache
- IntrospectionUtils.clear();
}
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/loader
WebappClassLoader.java
Posted by Costin Manolache <cm...@yahoo.com>.
Remy Maucherat wrote:
> Jan Luehe wrote:
>
> As a reminder, CVS shound't be used anymore.
I commited a bunch of small changes, don't know how easy it'll be to get
them in after the switch to svn. Let me know if there's a problem, I can
roll them back.
BTW - I had some of the changes in IntrospectionUtils in my workspace as
well, so I don't have to commit that :-)
Costin
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/loader
WebappClassLoader.java
Posted by Remy Maucherat <re...@apache.org>.
Jan Luehe wrote:
> No, I did.
Cool, there's one, at least :)
> Yes, but with lazy resolution, it will be loaded when the
> IntrospectionUtils symbol is first encountered, which may
> be inside WebappClassLoader.stop().
Normally, it's used by plenty of things, like the digester. Who knows
anyway, I find the classloading behavior to be weird sometimes.
>>So I am ok with your fix, but I don't understand how it can occur
>>in regular Tomcat.
>
> It's probably not occurring in standalone Tomcat, but only
> in "embedded" Tomcat. In standalone Tomcat, IntrospectionUtils is
> probably getting resolved (and its static initializer invoked)
> prior to calling WebappClassLoader.stop(),
> whereas in "embedded" mode, IntrospectionUtils is first referenced
> and loaded by WebappClassLoader.stop().
>
>>The big comment block is quite pointless, as it tries to be
>>informational, but doesn't correspond to reality (I am personally
>>against this kind of "commit message duplication" comment).
>
> Sure, I just thought this line might be an easy candidate for
> being moved around if there was no comment.
Stuff won't get moved around for no reason ;)
It would actually be good to move the 3 cleanup calls to
StandardContext.stop.
Rémy
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: cvs commit:
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/loader
WebappClassLoader.java
Posted by Jan Luehe <Ja...@Sun.COM>.
Remy,
Remy Maucherat wrote On 09/28/05 10:18,:
> Jan Luehe wrote:
>
>>We have seen the ThreadDeath in our callstacks, hence this fix.
>
>
> Nobody is reading what I am writing anymore ...
No, I did.
> I wrote:
> The static initializer is called when loading the class, and obviously
> the webapp CL is not going to load IntrospectionUtils.
>
> IntrospectionUtils will be loaded once, and its static initializer run
> once.
Yes, but with lazy resolution, it will be loaded when the
IntrospectionUtils symbol is first encountered, which may
be inside WebappClassLoader.stop().
IntrospectionUtils' static initializer will cause an invocation of
loadClass() on the thread's context classloader, which corresponds to
the WebappClassLoader, whose loadClass() throws ThreadDeath when
its "started" flag has been set to FALSE.
Therefore, we must avoid referencing IntrospectionUtils in
WebappClassLoader.stop() after the "started" flag has been
set to FALSE.
> So I am ok with your fix, but I don't understand how it can occur
> in regular Tomcat.
It's probably not occurring in standalone Tomcat, but only
in "embedded" Tomcat. In standalone Tomcat, IntrospectionUtils is
probably getting resolved (and its static initializer invoked)
prior to calling WebappClassLoader.stop(),
whereas in "embedded" mode, IntrospectionUtils is first referenced
and loaded by WebappClassLoader.stop().
> The big comment block is quite pointless, as it tries to be
> informational, but doesn't correspond to reality (I am personally
> against this kind of "commit message duplication" comment).
Sure, I just thought this line might be an easy candidate for
being moved around if there was no comment.
> As a reminder, CVS shound't be used anymore.
Yes.
Jan
>
> Rémy
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/loader
WebappClassLoader.java
Posted by Remy Maucherat <re...@apache.org>.
Jan Luehe wrote:
> We have seen the ThreadDeath in our callstacks, hence this fix.
Nobody is reading what I am writing anymore ...
I wrote:
The static initializer is called when loading the class, and obviously
the webapp CL is not going to load IntrospectionUtils.
IntrospectionUtils will be loaded once, and its static initializer run
once. So I am ok with your fix, but I don't understand how it can occur
in regular Tomcat.
The big comment block is quite pointless, as it tries to be
informational, but doesn't correspond to reality (I am personally
against this kind of "commit message duplication" comment).
As a reminder, CVS shound't be used anymore.
Rémy
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: cvs commit:
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/loader
WebappClassLoader.java
Posted by Jan Luehe <Ja...@Sun.COM>.
Remy,
Remy Maucherat wrote On 09/28/05 03:12,:
> jfarcand@apache.org wrote:
>
>> + /*
>> + * Clear the IntrospectionUtils cache.
>> + *
>> + * Implementation note:
>> + * Any reference to IntrospectionUtils which may cause the static
>> + * initalizer of that class to be invoked must occur prior to setting
>> + * the started flag to FALSE, because the static initializer of
>> + * IntrospectionUtils makes a call to
>> + * org.apache.commons.logging.LogFactory.getLog(), which ultimately
>> + * calls the loadClass() method of the thread context classloader,
>> + * which is the same as this classloader, whose impl throws a
>> + * ThreadDeath if the started flag has been set to FALSE.
>> + */
>> + IntrospectionUtils.clear();
>> +
>
>
> This commit does not make sense to me. The static initializer is called
> when loading the class, and obviously the webapp CL is not going to load
> IntrospectionUtils.
This code in IntrospectionUtils.java:
private static org.apache.commons.logging.Log log=
org.apache.commons.logging.LogFactory.getLog(
IntrospectionUtils.class );
will cause LogFactoryImpl.getLogConstructor() to be called, which
in turn will invoke LogFactoryImpl.loadClass(String name), which is
implemented as follows:
private static Class loadClass( final String name )
throws ClassNotFoundException
{
Object result = AccessController.doPrivileged(
new PrivilegedAction() {
public Object run() {
ClassLoader threadCL = getContextClassLoader();
if (threadCL != null) {
try {
return threadCL.loadClass(name);
} catch( ClassNotFoundException ex ) {
// ignore
}
}
try {
return Class.forName( name );
} catch (ClassNotFoundException e) {
return e;
}
}
});
if (result instanceof Class)
return (Class)result;
throw (ClassNotFoundException)result;
}
Notice the use of the thread context classloader (to load the class
with the given name), which is the same as the WebappClassLoader.
WebappClassLoader.loadClass() has this:
// Don't load classes if class loader is stopped
if (!started) {
log.info(sm.getString("webappClassLoader.stopped"));
throw new ThreadDeath();
}
We have seen the ThreadDeath in our callstacks, hence this fix.
Jan
> You should forget that the CVS exists, as we're in the middle of
> migrating to SVN (of course, losing that commit will not be a problem).
>
> Rémy
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/loader
WebappClassLoader.java
Posted by Remy Maucherat <re...@apache.org>.
jfarcand@apache.org wrote:
> + /*
> + * Clear the IntrospectionUtils cache.
> + *
> + * Implementation note:
> + * Any reference to IntrospectionUtils which may cause the static
> + * initalizer of that class to be invoked must occur prior to setting
> + * the started flag to FALSE, because the static initializer of
> + * IntrospectionUtils makes a call to
> + * org.apache.commons.logging.LogFactory.getLog(), which ultimately
> + * calls the loadClass() method of the thread context classloader,
> + * which is the same as this classloader, whose impl throws a
> + * ThreadDeath if the started flag has been set to FALSE.
> + */
> + IntrospectionUtils.clear();
> +
This commit does not make sense to me. The static initializer is called
when loading the class, and obviously the webapp CL is not going to load
IntrospectionUtils.
You should forget that the CVS exists, as we're in the middle of
migrating to SVN (of course, losing that commit will not be a problem).
Rémy
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org