You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cocoon.apache.org by David Haraburda <da...@haraburda.com> on 2002/06/09 20:50:15 UTC

ClassLoader issues

Hi all,

This is a second attempt to raise what I feel to be a rather important
issue, and that is the way CocoonServlet sets up the ContextClassLoader
for each thread on startup.  I think this is bad because (1) It breaks
JNDI in Tomcat and (2) it conflicts with both the Java API docs and the
J2EE spec.  Take a look at the beginning of the service method in
CocoonServlet -- it overrides the ContextClassLoader without even
checking the init-classloader param.  I don't think Cocoon should be
setting the classloader for user-level code at all.  Obviously there was
something that inspired this behavior. What was it?  Maybe I'm just
really confused, if so, please flame away -- however, I'm more than
happy to submit patches to fix this (I've already submitted what I feel
is a good temporary fix - bug 9684), but I'd like to get some more
insight/feedback on why Cocoon behaves this way.

David


---------------------------------------------------------------------
To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
For additional commands, email: cocoon-dev-help@xml.apache.org


Re: ClassLoader issues

Posted by Peter Royal <pr...@apache.org>.
On Monday 10 June 2002 02:38 pm, Sylvain Wallez wrote:
> So in short, my proposal is : remove classloading stuff in CocoonServlet
> and move it to ParanoidCocoonServlet. Of course, classpath computation
> is left unchanged in CocoonServlet.

Go for it. Sounds like you have done your research and are aware of the 
implications :)
-pete

-- 
peter royal -> proyal@apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
For additional commands, email: cocoon-dev-help@xml.apache.org


Re: ClassLoader issues

Posted by David Haraburda <da...@haraburda.com>.
On Mon, 2002-06-10 at 13:38, Sylvain Wallez wrote:
> 
> After some investigation in CocoonServlet's classloading stuff (which 
> has a loooong history), it appears to me that classpath and classloader 
> are different issues and that we don't really need the 
> RepositoryClassloader there. Discussion follows...

I agree.  It looks like RepositoryClassLoader is needed to compile
things like XSPs and the sitemap and such, but this is internal to
Cocoon, so there isn't a problem there.  My arguments for having it
removed from CocoonServlet (besides the fact that it breaks JNDI in
Tomcat) are:

1 - J2SE API Docs (take a look at java.lang.Thread javadoc) imply that
it is a Thread's creator's responsibility to set the
ContextClassLoader.  Since the servlet engine (Tomcat in this case)
creates the Threads, it is responsible for this
2 - The J2EE spec (1.3) in section 6.2.4.8 further elaborates on the
needs for a ContextClassLoader (that is set by the J2EE container)

(I know I've said these two things before -- just re-iterating :-)

> 
> The classpath is used to instruct the Java compiler used for XSPs where 
> it should find the classes, since it (unfortunately) doesn't care about 
> the current classloader. So the extra-classpath parameter is IMO usefull 
> only when there are some classes visible from the webapp that are 
> neither in the system classpath, neither in the WEB-INF/lib or 
> WEB-INF/classes directory. This case may arise when some libraries 
> common to several webapp contexts are deployed in tomcat/lib (see 
> http://jakarta.apache.org/tomcat/tomcat-4.0-doc/class-loader-howto.html 
> for a clear explanation of this).

And this behavior I believe, is Tomcat-specific.  Really, web
application developers should follow the spec guidelines and package
their web-app with all required librares and classes in WEB-INF/lib and
WEB-INF/classes -- I think the extra-classpath parameter could be
removed also.

> Now Cocoon relies heavily on dynamic loading of classes named in 
> cocoon.xconf and sitemap.xmap through Avalon, and to avoid classloading 
> problems when a class defined by a higher-lever classloader (such as 
> tomcat/lib) loads a class defined by a lower-level class loader (such as 
> WEB-INF/lib), all classloading is made using the current thread's 
> context classloader.

Yup.  That's basically why the ContextClassLoader exists.  The J2EE spec
says essentially the same thing.

> 
> The purpose of setting the context classloader in CocoonServlet is to 
> ensure that the webapp classloader (i.e. the lowest level) will 
> effectively be used by Cocoon and Avalon. Now this could be limited to 
> CocoonServlet's classloader (i.e. the webapp classloader) and not it's 
> child RepositoryClassloader.
> 
> My opinion is that the RepositoryClassloader in CocoonServlet is 
> actually not needed if we rely on normal classloading provided by the 
> container. If the container doesn't provide the right classloader, then 
> we should use the ParanoidCocoonServlet that totally shields the 
> upper-level classloaders with its own ParanoidClassloader, which for 
> sure here must be the context classloader.

I agree.  Hopefully, most containers can "do the right thing", and those
that need this non-standard behavior can set the appropriate settings in
web.xml.  Throwing JAR files and classes "wherever" and then tweaking
Cocoon's options to make it work will certainly only create more
problems.

> 
> So in short, my proposal is : remove classloading stuff in CocoonServlet 
> and move it to ParanoidCocoonServlet. Of course, classpath computation 
> is left unchanged in CocoonServlet.
> 
> Thoughts ?

I think this is a good idea.

David


---------------------------------------------------------------------
To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
For additional commands, email: cocoon-dev-help@xml.apache.org


RE: ClassLoader issues

Posted by Vadim Gritsenko <va...@verizon.net>.
> From: Sylvain Wallez [mailto:sylvain.wallez@anyware-tech.com]

...
 
> After some investigation in CocoonServlet's classloading stuff (which
> has a loooong history), it appears to me that classpath and
classloader
> are different issues and that we don't really need the
> RepositoryClassloader there. Discussion follows...
> 
> The classpath is used to instruct the Java compiler used for XSPs
where
> it should find the classes, since it (unfortunately) doesn't care
about
> the current classloader. So the extra-classpath parameter is IMO
usefull
> only when there are some classes visible from the webapp that are
> neither in the system classpath, neither in the WEB-INF/lib or
> WEB-INF/classes directory. This case may arise when some libraries
> common to several webapp contexts are deployed in tomcat/lib (see
>
http://jakarta.apache.org/tomcat/tomcat-4.0-doc/class-loader-howto.html
> for a clear explanation of this).
> 
> Now Cocoon relies heavily on dynamic loading of classes named in
> cocoon.xconf and sitemap.xmap through Avalon, and to avoid
classloading
> problems when a class defined by a higher-lever classloader (such as
> tomcat/lib) loads a class defined by a lower-level class loader (such
as
> WEB-INF/lib), all classloading is made using the current thread's
> context classloader.
> 
> The purpose of setting the context classloader in CocoonServlet is to
> ensure that the webapp classloader (i.e. the lowest level) will
> effectively be used by Cocoon and Avalon. Now this could be limited to
> CocoonServlet's classloader (i.e. the webapp classloader) and not it's
> child RepositoryClassloader.
> 
> My opinion is that the RepositoryClassloader in CocoonServlet is
> actually not needed if we rely on normal classloading provided by the
> container. If the container doesn't provide the right classloader,
then
> we should use the ParanoidCocoonServlet that totally shields the
> upper-level classloaders with its own ParanoidClassloader, which for
> sure here must be the context classloader.
> 
> So in short, my proposal is : remove classloading stuff in
CocoonServlet
> and move it to ParanoidCocoonServlet. Of course, classpath computation
> is left unchanged in CocoonServlet.
> 
> Thoughts ?

No objections if it works.

I think this hack was done long before ParanoidCocoonServlet was
created, and now it seems it should be moved to ParanoidCocoonServlet.

Vadim


> Sylvain
> 
> --
> Sylvain Wallez
>  Anyware Technologies                  Apache Cocoon
>  http://www.anyware-tech.com           mailto:sylvain@apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
For additional commands, email: cocoon-dev-help@xml.apache.org


Re: ClassLoader issues

Posted by Sylvain Wallez <sy...@anyware-tech.com>.
David Haraburda wrote:

>Hi,
>
>On Sun, 2002-06-09 at 16:28, Nicola Ken Barozzi wrote:
>  
>
>>The real reason: Cocoon is not a Servlet.
>>Cocoon conceptually lives in the same space of Tomcat, not on top of it.
>>    
>>
>
>I certainly agree that conceptually Cocoon is much more than a simple
>servlet -- it is a large and complex system.  However in actuality it
>runs as a web application, so I think it is only appropriate that it
>acts in compliance with the necessary specifications.
>
>  
>
>>Cocoon is generally a servlet out of need, since it eases deployment on
>>existing systems.
>>
>
>I can understand with a large project like Cocoon, many things will have
>to be done that a normal web application wouldn't do -- the main issue I
>am trying to raise here is that I feel it is unnecessary for Cocoon to
>ever override the ContextClassLoader -- and I appreciate any and all
>feedback on this.
>

After some investigation in CocoonServlet's classloading stuff (which 
has a loooong history), it appears to me that classpath and classloader 
are different issues and that we don't really need the 
RepositoryClassloader there. Discussion follows...

The classpath is used to instruct the Java compiler used for XSPs where 
it should find the classes, since it (unfortunately) doesn't care about 
the current classloader. So the extra-classpath parameter is IMO usefull 
only when there are some classes visible from the webapp that are 
neither in the system classpath, neither in the WEB-INF/lib or 
WEB-INF/classes directory. This case may arise when some libraries 
common to several webapp contexts are deployed in tomcat/lib (see 
http://jakarta.apache.org/tomcat/tomcat-4.0-doc/class-loader-howto.html 
for a clear explanation of this).

Now Cocoon relies heavily on dynamic loading of classes named in 
cocoon.xconf and sitemap.xmap through Avalon, and to avoid classloading 
problems when a class defined by a higher-lever classloader (such as 
tomcat/lib) loads a class defined by a lower-level class loader (such as 
WEB-INF/lib), all classloading is made using the current thread's 
context classloader.

The purpose of setting the context classloader in CocoonServlet is to 
ensure that the webapp classloader (i.e. the lowest level) will 
effectively be used by Cocoon and Avalon. Now this could be limited to 
CocoonServlet's classloader (i.e. the webapp classloader) and not it's 
child RepositoryClassloader.

My opinion is that the RepositoryClassloader in CocoonServlet is 
actually not needed if we rely on normal classloading provided by the 
container. If the container doesn't provide the right classloader, then 
we should use the ParanoidCocoonServlet that totally shields the 
upper-level classloaders with its own ParanoidClassloader, which for 
sure here must be the context classloader.

So in short, my proposal is : remove classloading stuff in CocoonServlet 
and move it to ParanoidCocoonServlet. Of course, classpath computation 
is left unchanged in CocoonServlet.

Thoughts ?

Sylvain

-- 
Sylvain Wallez
 Anyware Technologies                  Apache Cocoon
 http://www.anyware-tech.com           mailto:sylvain@apache.org




---------------------------------------------------------------------
To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
For additional commands, email: cocoon-dev-help@xml.apache.org


RE: ClassLoader issues

Posted by John Morrison <jo...@ntlworld.com>.
> From: David Haraburda [mailto:david-cocoon@haraburda.com]
>
> On Sun, 2002-06-09 at 16:28, Nicola Ken Barozzi wrote:
> > The real reason: Cocoon is not a Servlet.
> > Cocoon conceptually lives in the same space of Tomcat, not on top of it.
>
> I certainly agree that conceptually Cocoon is much more than a simple
> servlet -- it is a large and complex system.  However in actuality it
> runs as a web application, so I think it is only appropriate that it
> acts in compliance with the necessary specifications.

It also runs straight from a java command line.  See CLI.

I'm +1 as long as there's always the possibility of running it
in a backward compatible way.

J.


---------------------------------------------------------------------
To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
For additional commands, email: cocoon-dev-help@xml.apache.org


Re: ClassLoader issues

Posted by David Haraburda <da...@haraburda.com>.
Hi,

On Sun, 2002-06-09 at 16:28, Nicola Ken Barozzi wrote:
> The real reason: Cocoon is not a Servlet.
> Cocoon conceptually lives in the same space of Tomcat, not on top of it.

I certainly agree that conceptually Cocoon is much more than a simple
servlet -- it is a large and complex system.  However in actuality it
runs as a web application, so I think it is only appropriate that it
acts in compliance with the necessary specifications.

> 
> Cocoon is generally a servlet out of need, since it eases deployment on
> existing systems.
> 

I can understand with a large project like Cocoon, many things will have
to be done that a normal web application wouldn't do -- the main issue I
am trying to raise here is that I feel it is unnecessary for Cocoon to
ever override the ContextClassLoader -- and I appreciate any and all
feedback on this.

> I'm +1 for having this behaviour being toggeled effectively by the param, so
> I will commit your patch if there are no -1s.
> 

Thanks for the vote and for your time, it is appreciated.

David


---------------------------------------------------------------------
To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
For additional commands, email: cocoon-dev-help@xml.apache.org


Re: ClassLoader issues

Posted by Nicola Ken Barozzi <ni...@apache.org>.
From: "David Haraburda" <da...@haraburda.com>

> Hi all,
>
> This is a second attempt to raise what I feel to be a rather important
> issue, and that is the way CocoonServlet sets up the ContextClassLoader
> for each thread on startup.  I think this is bad because (1) It breaks
> JNDI in Tomcat and (2) it conflicts with both the Java API docs and the
> J2EE spec.  Take a look at the beginning of the service method in
> CocoonServlet -- it overrides the ContextClassLoader without even
> checking the init-classloader param.  I don't think Cocoon should be
> setting the classloader for user-level code at all.  Obviously there was
> something that inspired this behavior. What was it?  Maybe I'm just
> really confused, if so, please flame away -- however, I'm more than
> happy to submit patches to fix this (I've already submitted what I feel
> is a good temporary fix - bug 9684), but I'd like to get some more
> insight/feedback on why Cocoon behaves this way.

The real reason: Cocoon is not a Servlet.
Cocoon conceptually lives in the same space of Tomcat, not on top of it.

Cocoon is generally a servlet out of need, since it eases deployment on
existing systems.

I'm +1 for having this behaviour being toggeled effectively by the param, so
I will commit your patch if there are no -1s.

--
Nicola Ken Barozzi                   nicolaken@apache.org
            - verba volant, scripta manent -
   (discussions get forgotten, just code remains)
---------------------------------------------------------------------


---------------------------------------------------------------------
To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
For additional commands, email: cocoon-dev-help@xml.apache.org