I have a potential fix for the problem, which I'll also attach to the Jira. IMO, the fix is as good as the current context-priority-classloader implementation.
In the way of background, the Geronimo classloaders for both Jetty and Tomcat have a number of packages that they exclude from being loaded from a web app's context (instead, they'll always be loaded from using the parent classloader). This behavior is as specified by the Servlet specification (which states that java, javax, and "container implementation" classes should not be loaded from a web app's context. The Jetty/Tomcat classloaders currently exclude java.*, javax.*, org.apache.geronimo.*, etc. Users currently have no way of predicting what packages will be excluded by the context-priority-classloader. My fix simply adds org.apache.commons.logging.* to this exclusion list (I think there are probably other "implementation classes" which ought to be excluded, but I'm not addressing these. Also, as Anita suggested, it would be useful to users to inform them when a class has been "excluded". There are some potential issues with this -- load times and potentially large number of messages -- I haven't attempted to address.
I've tested this change under Jetty and the test app deploys without error, and appears to work fine. Deployment under Tomcat fails with a NullPointerException. However, I don't believe this has anything to do with my fix. I'll pursue this problem separately.
I think the behavior of context-priority-classloader needs to be reviewed, the package exclusion lists for Tomcat and Jetty needs to be reviewed, and all of this needs to be documented for users. I think the current implementation/exclusion list gives users a reasonable chance of introducing deployment/runtime errors in their web app without a way of predicting this behavior.
Finally, It's possible that there's a fundamental problem which is causing the ClassCastException (I'm a relative novice when it comes to web containers/classloaders/commons logging). So, it's probably worth reviewing what's happening when we get the ClassCastException.
The code where the ClassCastException is occurring is in o.a.commons.logging.LogFactory.newFactory(factoryClassName, classloader). The stack trace line numbers don't exactly match with my source code, but I'm reasonably certain of the following:
LogFactory newFactory(String factoryClass, Classloader classLoader) {
...
try {
try {
return (LogFactory) classLoader.loadClass(factoryClass).newInstance(); // (1) the initial ClassCastException
}
catch (ClassCastException cce) {
if (classLoader == LogFactory.class.getClassLoader()) // (2) and yet this check succeeds
throw cce; // (3) so the exception is thrown again
}
return (LogFactory) Class.forName(factoryClass).newInstance(); // and we never get here...
}
catch (Exception e) {
throw new LogConfigurationException(); // (4) and ultimately fail here
}
}
I'm assuming that the initial ClassCastException (1) occurs because (LogFactory) was not loaded by the parent classloader, not the context classloader. Yet, I don't understand why (2), the subsequent classLoader == check, would then succeed. Thoughts anyone? I haven't debugged to the point of validating the factoryClass etc. But I'm assuming this is not a struts implementation/application issue.
--kevan
On 9/28/05, anita kulshreshtha <[EMAIL PROTECTED]> wrote:
--- Aaron Mulder <[EMAIL PROTECTED]>
wrote:
> On 9/28/05, Jeff Genender <[EMAIL PROTECTED] >
> wrote:
> > > GERONIMO-518 - Deploying Struts app fails on
> Logging ClassCastException
> > >
> > > Aaron, looking at your latest comments this
> doesn't seems so easy to
> > > solve. I wonder if we shouldn't add this to the
> "Significant Missing
> > > Features" section of our release notes.
> >
> > This is not a bug...I will probably close this out
> - unless others
> > object. This has to do with the commons logging
> jar in the web app.
>
> I think I object. If we feel that commons logging
> is part of the
> server implementation and cannot be redefined in the
> web app, then we
> should add it to the list of packages we refuse to
> load from the web
> app classloader (so if someone puts it in there,
> we'll just silently
> ignore it).
A warning will be even better. Eventually G will have
fixed version of all the jars. If a user needs to use
a later version of a jar, for example to include a
latest feature/bug fix important to their app), will
they be asked to upgrade G or be allowed to override
the jar.
It seems pretty obnoxious to take
> Struts WARs that work
> perfectly well with other app servers and claim that
> it's the user's
> fault that they blow up in Geronimo, you know?
I agree.
Thanks
Anita
>
> Aaron
>
__________________________________
Yahoo! Mail - PC Magazine Editors' Choice 2005
http://mail.yahoo.com
