You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Henri Yandell <ba...@generationjava.com> on 2002/11/24 03:17:31 UTC

[lang] reflect - Class.forName

I've not really been paying attention to the reflect stuff, and had missed
a question about it in a blog I usually read:

http://radio.weblogs.com/0112098/2002/09/24.html#a114

Rather than using:

Class theClass = Class.forName( className );

in reflect.ReflectionUtils, James suggests we should use:

****
Class theClass = null;

try {
 theClass = Thread.currentThread().getContextClassLoader().loadClass(
className );
} catch (ClassNotFoundException e) {
 theClass = getClass().getClassLoader().loadClass( className );
}
****

as "Class.forName is evil".

Any views on this??

Hen


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [lang] reflect - Class.forName

Posted by Juozas Baliuka <ba...@centras.lt>.
This way is "evil" too:

1. MyClass implements MyInterface
2. MyClass is visible for Thread.currentThread().getContextClassLoader();
3. MyInterface.class.getClassLoader() is not "parent" of
Thread.currentThread().getContextClassLoader();
4. MyInterface.class.getClassLoader() !=
Thread.currentThread().getContextClassLoader();

MyInterface var = null;
Class cls = Thread.currentThread().getContextClassLoader.loadClass(
"MyClass" );
var = (MyInterface)cls.newInstance();

the last line will throw ClassCastExeption.

----- Original Message -----
From: "Henri Yandell" <ba...@generationjava.com>
To: "Jakarta Commons Developers List" <co...@jakarta.apache.org>
Sent: Sunday, November 24, 2002 4:17 AM
Subject: [lang] reflect - Class.forName


>
> I've not really been paying attention to the reflect stuff, and had missed
> a question about it in a blog I usually read:
>
> http://radio.weblogs.com/0112098/2002/09/24.html#a114
>
> Rather than using:
>
> Class theClass = Class.forName( className );
>
> in reflect.ReflectionUtils, James suggests we should use:
>
> ****
> Class theClass = null;
>
> try {
>  theClass = Thread.currentThread().getContextClassLoader().loadClass(
> className );
> } catch (ClassNotFoundException e) {
>  theClass = getClass().getClassLoader().loadClass( className );
> }
> ****
>
> as "Class.forName is evil".
>
> Any views on this??
>
> Hen
>
>
> --
> To unsubscribe, e-mail:
<ma...@jakarta.apache.org>
> For additional commands, e-mail:
<ma...@jakarta.apache.org>
>


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [lang] reflect - Class.forName

Posted by Max Rydahl Andersen <ma...@eos.dk>.
Yes - forName is evil, especially when used in a J2EE container (both in web
and ejb containers).

An explanation and some sourcecode for it is shown in this thread:
http://sourceforge.net/forum/message.php?msg_id=1720229

The same issue is present in many libraries, and recently we (my company)
found a bug in Struts classloading that is similar - here they had done a
attempt to provide correct classloading, but did not get it completly right
:)
The above link and the discussion at
http://nagoya.apache.org/eyebrowse/BrowseList?listName=struts-dev@jakarta.ap
ache.org&by=thread&from=271341 should give some insight into the problem.

Note: Even though the problem could be solved in struts with a 3 line
codechange they decided against the change because it messes with the
classloading semantics. But I would just say that the code always fallsback
on to the class.forName semantics if everything else fails - thus it should
work as always in simple java app setups, and work as expected in more
advanced classssloading heavy apps.

/max

----- Original Message -----
From: "Henri Yandell" <ba...@generationjava.com>
To: "Jakarta Commons Developers List" <co...@jakarta.apache.org>
Sent: Sunday, November 24, 2002 3:17 AM
Subject: [lang] reflect - Class.forName


>
> I've not really been paying attention to the reflect stuff, and had missed
> a question about it in a blog I usually read:
>
> http://radio.weblogs.com/0112098/2002/09/24.html#a114
>
> Rather than using:
>
> Class theClass = Class.forName( className );
>
> in reflect.ReflectionUtils, James suggests we should use:
>
> ****
> Class theClass = null;
>
> try {
>  theClass = Thread.currentThread().getContextClassLoader().loadClass(
> className );
> } catch (ClassNotFoundException e) {
>  theClass = getClass().getClassLoader().loadClass( className );
> }
> ****
>
> as "Class.forName is evil".
>
> Any views on this??
>
> Hen
>
>
> --
> To unsubscribe, e-mail:
<ma...@jakarta.apache.org>
> For additional commands, e-mail:
<ma...@jakarta.apache.org>
>
>


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [lang] reflect - Class.forName

Posted by Juozas Baliuka <ba...@centras.lt>.
It is no good way to "discover" class loader,
but I prefer to try the most "specific" class loader first, like
java.beans.Introspector doe's to find "BeanInfo".
Thread context class loader is the "last".

javac generates code like this to implement "MyClass.class" :

 static Class class$(String name){
   try{
       return Class.forName(name);
   }catch( ClassNotFoundException cnfe ){
      throw new ClassNotFoundError(cnfe.getMessage());
  }
}

 static Class $MyClass = class$("MyClass");

I think this class loader must be used by library the first too, if possible
.


----- Original Message -----
From: "Max Rydahl Andersen" <ma...@eos.dk>
To: "Jakarta Commons Developers List" <co...@jakarta.apache.org>
Sent: Sunday, November 24, 2002 11:10 AM
Subject: Re: [lang] reflect - Class.forName


> hmm - I just came to think of another "issue".
>
> To have correct classloading semantics the class that is used to load
> classes dynamically (e.g. RequestUtils) needs to have been loaded by the
> same classloader as the caller of the function ......that actually means
it
> cannot be generalized to be a general function in a general library that
is
> just loaded by some "global" or "third-party" classloader...This should
not
> stop one from using the proposed load semantics, it should just be
> documented that that is the case (no one newer said that java classloading
> should be easy :)
>
> /max
>
> ----- Original Message -----
> From: "Steve Downey" <st...@netfolio.com>
> To: "Jakarta Commons Developers List" <co...@jakarta.apache.org>
> Sent: Sunday, November 24, 2002 6:46 AM
> Subject: Re: [lang] reflect - Class.forName
>
>
> +1, as the reasoning seems quite sane.
>
>
> On Saturday 23 November 2002 09:17 pm, Henri Yandell wrote:
> > I've not really been paying attention to the reflect stuff, and had
missed
> > a question about it in a blog I usually read:
> >
> > http://radio.weblogs.com/0112098/2002/09/24.html#a114
> >
> > Rather than using:
> >
> > Class theClass = Class.forName( className );
> >
> > in reflect.ReflectionUtils, James suggests we should use:
> >
> > ****
> > Class theClass = null;
> >
> > try {
> >  theClass = Thread.currentThread().getContextClassLoader().loadClass(
> > className );
> > } catch (ClassNotFoundException e) {
> >  theClass = getClass().getClassLoader().loadClass( className );
> > }
> > ****
> >
> > as "Class.forName is evil".
> >
> > Any views on this??
> >
> > Hen
>
>
> --
> To unsubscribe, e-mail:
> <ma...@jakarta.apache.org>
> For additional commands, e-mail:
> <ma...@jakarta.apache.org>
>
>
>
>
> --
> To unsubscribe, e-mail:
<ma...@jakarta.apache.org>
> For additional commands, e-mail:
<ma...@jakarta.apache.org>
>


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [lang] reflect - Class.forName

Posted by Juozas Baliuka <ba...@mwm.lt>.
----- Original Message -----
From: "Max Rydahl Andersen" <ma...@eos.dk>
To: "Jakarta Commons Developers List" <co...@jakarta.apache.org>
Sent: Tuesday, November 26, 2002 9:36 AM
Subject: Re: [lang] reflect - Class.forName


> Ok - now I've consulted my local java senator regarding class loading :)
>
> In RequestUtils it is necessary for RequestUtils to use the
> ContextClassLoader because the classloader that was used to load
> RequestUtils does NOT necessarily know the classes that is needed for
> loading (ofcourse!). Secondly, if the classloader for RequestUtils knows
> class X, it is not necesaarily the same version of class X that e.g. a
> webapp in Tomcat needs to load. Here class X might be an Xerces parser.
>
> Thus, if RequestUtils usage scenario is in an container then it has to
> consult the current threads contextclassloader as it is the responsibility
> for the container to provide the correct context for classloading - and if
> RequestUtils uses class.forname as its first choice the container has no
> possibility to control the classloading.
>
> Im a making any sense ? :)

It is problem on some tomcat and phoenix versions, I am not sure, but it
seems
 default is contextclassloader == SystemClassLoader, but not null. The same
problems
was reported for commons logging API  few moths ago, it seems it was problem
on JRun too.


>
> The issue on wether RequestUtils can be used in webapps running inside
> Tomcat, well, yes it can - and if the threads context classloader contains
> the needed and correct classloaders it should be no problem - as then
> class.forName would never be called :)
>
> /max
>
>
> --
> To unsubscribe, e-mail:
<ma...@jakarta.apache.org>
> For additional commands, e-mail:
<ma...@jakarta.apache.org>
>
>


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [lang] reflect - Class.forName

Posted by Max Rydahl Andersen <ma...@eos.dk>.
Yes, this is what im trying to argue for :)

The reason for my last post was that Jaouza suggested that one should use #3
as the first choice to avoid
his usecase with MyInterface and MyClass.

/max


----- Original Message -----
From: "bob mcwhirter" <bo...@werken.com>
To: "Jakarta Commons Developers List" <co...@jakarta.apache.org>
Sent: Tuesday, November 26, 2002 9:40 AM
Subject: Re: [lang] reflect - Class.forName


> > Im a making any sense ? :)
> >
> > The issue on wether RequestUtils can be used in webapps running inside
> > Tomcat, well, yes it can - and if the threads context classloader
contains
> > the needed and correct classloaders it should be no problem - as then
> > class.forName would never be called :)
>
> Right, which is why the algorithm typically is:
>
> 1) check for the thread's context classloader.
> 2) if not null, use to load class.
> 3) if null, use getClass().getClassLoader() to load class.
>
> #2 makes it safe for container usage, and #3 effectively falls
> back to Class.forName() functionality for those folks running
> in a simpler, non-container classloader environment.
>
> -bob
>
>
> --
> To unsubscribe, e-mail:
<ma...@jakarta.apache.org>
> For additional commands, e-mail:
<ma...@jakarta.apache.org>
>
>


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [lang] reflect - Class.forName

Posted by bob mcwhirter <bo...@werken.com>.
> Im a making any sense ? :)
> 
> The issue on wether RequestUtils can be used in webapps running inside
> Tomcat, well, yes it can - and if the threads context classloader contains
> the needed and correct classloaders it should be no problem - as then
> class.forName would never be called :)

Right, which is why the algorithm typically is:

1) check for the thread's context classloader.
2) if not null, use to load class.
3) if null, use getClass().getClassLoader() to load class.

#2 makes it safe for container usage, and #3 effectively falls
back to Class.forName() functionality for those folks running
in a simpler, non-container classloader environment.

	-bob


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [lang] reflect - Class.forName

Posted by Max Rydahl Andersen <ma...@eos.dk>.
Ok - now I've consulted my local java senator regarding class loading :)

In RequestUtils it is necessary for RequestUtils to use the
ContextClassLoader because the classloader that was used to load
RequestUtils does NOT necessarily know the classes that is needed for
loading (ofcourse!). Secondly, if the classloader for RequestUtils knows
class X, it is not necesaarily the same version of class X that e.g. a
webapp in Tomcat needs to load. Here class X might be an Xerces parser.

Thus, if RequestUtils usage scenario is in an container then it has to
consult the current threads contextclassloader as it is the responsibility
for the container to provide the correct context for classloading - and if
RequestUtils uses class.forname as its first choice the container has no
possibility to control the classloading.

Im a making any sense ? :)

The issue on wether RequestUtils can be used in webapps running inside
Tomcat, well, yes it can - and if the threads context classloader contains
the needed and correct classloaders it should be no problem - as then
class.forName would never be called :)

/max


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [lang] reflect - Class.forName

Posted by Max Rydahl Andersen <ma...@eos.dk>.
> > If the container is written "correctly" just having more or less
isolated
> > the webapp's classloading from
> > the containers own classloading then this would never be an problem (but
> > that is not the case
> > for many containers out there - and the "trick" of adding some jars to
the
> > "global" classpath of an container
> > is widely used as I understand it )
>
> Out of interest, do you mean adding this to the actual shellscript's
> classpath, or just placing things in the common/lib directory? [in say
> tomcat].
>
> If it's the former, then they [and I, yeah I've done it] deserve what they
> get.

Is there any difference between adding them to the shellscript or add it to
common/lib ?
Those placed in common/lib is just added to the global classpath, is it not
?

> > > Because first it would throw the class cast, and then forName would
fail?
> >
> > The utility should not throw class cast exceptions only classnotfound.
> > The class cast situation presented by Jousaz is something I'll have to
think
> > about some more -
> > i'll talk with my local classloading expert tomorrow, and get back to
you :)
> >
> > p.s. another note on James code suggestion is that it does not check if
the
> > current classloader is null - it can be, thus
> > James code can in some cases throw a NPE!
>
> Cool. Anyone feel like changing/submitting a patch? :) I'll add it to the
> STATUS.html for the moment.

A better code is available at the disscussion on hibernate and/or struts.

/max

>
> Hen
>
>
> --
> To unsubscribe, e-mail:
<ma...@jakarta.apache.org>
> For additional commands, e-mail:
<ma...@jakarta.apache.org>
>
>


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [lang] reflect - Class.forName

Posted by Henri Yandell <ba...@generationjava.com>.

On Sun, 24 Nov 2002, Max Rydahl Andersen wrote:

> If the container is written "correctly" just having more or less isolated
> the webapp's classloading from
> the containers own classloading then this would never be an problem (but
> that is not the case
> for many containers out there - and the "trick" of adding some jars to the
> "global" classpath of an container
> is widely used as I understand it )

Out of interest, do you mean adding this to the actual shellscript's
classpath, or just placing things in the common/lib directory? [in say
tomcat].

If it's the former, then they [and I, yeah I've done it] deserve what they
get.

> > Because first it would throw the class cast, and then forName would fail?
>
> The utility should not throw class cast exceptions only classnotfound.
> The class cast situation presented by Jousaz is something I'll have to think
> about some more -
> i'll talk with my local classloading expert tomorrow, and get back to you :)
>
> p.s. another note on James code suggestion is that it does not check if the
> current classloader is null - it can be, thus
> James code can in some cases throw a NPE!

Cool. Anyone feel like changing/submitting a patch? :) I'll add it to the
STATUS.html for the moment.

Hen


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [lang] reflect - Class.forName

Posted by Max Rydahl Andersen <ma...@eos.dk>.
> > To have correct classloading semantics the class that is used to load
> > classes dynamically (e.g. RequestUtils) needs to have been loaded by the
> > same classloader as the caller of the function ......that actually means
it
> > cannot be generalized to be a general function in a general library that
is
> > just loaded by some "global" or "third-party" classloader...
>
> Ouch. I'm writing a letter to my nearest Java senator.
>
> > This should not
> > stop one from using the proposed load semantics, it should just be
> > documented that that is the case (no one newer said that java
classloading
> > should be easy :)
>
> So the fix would be that in the class-loading aspects of Lang, we would
> use the code James Strachan points out, but if an exception is thrown as
> Juozas points out, we would catch it and run the tried and tested forName?
>
> And in some cases the code would never work, ie) in container where
> another classloader had the gall to load us first.
>
> So I can understand... do you mean that if Commons Lang becomes used in
> Tomcat, and Tomcat loads a webapp, would that webapp not be able to use
this
> code?

Something like that, yes :)

Except that in practice it might not be a problem - but that does not help
those 2% that
will run into this suttle "feature".

If the container is written "correctly" just having more or less isolated
the webapp's classloading from
the containers own classloading then this would never be an problem (but
that is not the case
for many containers out there - and the "trick" of adding some jars to the
"global" classpath of an container
is widely used as I understand it )

> Because first it would throw the class cast, and then forName would fail?

The utility should not throw class cast exceptions only classnotfound.
The class cast situation presented by Jousaz is something I'll have to think
about some more -
i'll talk with my local classloading expert tomorrow, and get back to you :)

p.s. another note on James code suggestion is that it does not check if the
current classloader is null - it can be, thus
James code can in some cases throw a NPE!







--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [lang] reflect - Class.forName

Posted by Henri Yandell <ba...@generationjava.com>.

On Sun, 24 Nov 2002, Max Rydahl Andersen wrote:

> To have correct classloading semantics the class that is used to load
> classes dynamically (e.g. RequestUtils) needs to have been loaded by the
> same classloader as the caller of the function ......that actually means it
> cannot be generalized to be a general function in a general library that is
> just loaded by some "global" or "third-party" classloader...

Ouch. I'm writing a letter to my nearest Java senator.

> This should not
> stop one from using the proposed load semantics, it should just be
> documented that that is the case (no one newer said that java classloading
> should be easy :)

So the fix would be that in the class-loading aspects of Lang, we would
use the code James Strachan points out, but if an exception is thrown as
Juozas points out, we would catch it and run the tried and tested forName?

And in some cases the code would never work, ie) in container where
another classloader had the gall to load us first.

So I can understand... do you mean that if Commons Lang becomes used in
Tomcat, and Tomcat loads a webapp, would that webapp not be able to use this
code?

Because first it would throw the class cast, and then forName would fail?

Hen



--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [lang] reflect - Class.forName

Posted by Max Rydahl Andersen <ma...@eos.dk>.
hmm - I just came to think of another "issue".

To have correct classloading semantics the class that is used to load
classes dynamically (e.g. RequestUtils) needs to have been loaded by the
same classloader as the caller of the function ......that actually means it
cannot be generalized to be a general function in a general library that is
just loaded by some "global" or "third-party" classloader...This should not
stop one from using the proposed load semantics, it should just be
documented that that is the case (no one newer said that java classloading
should be easy :)

/max

----- Original Message -----
From: "Steve Downey" <st...@netfolio.com>
To: "Jakarta Commons Developers List" <co...@jakarta.apache.org>
Sent: Sunday, November 24, 2002 6:46 AM
Subject: Re: [lang] reflect - Class.forName


+1, as the reasoning seems quite sane.


On Saturday 23 November 2002 09:17 pm, Henri Yandell wrote:
> I've not really been paying attention to the reflect stuff, and had missed
> a question about it in a blog I usually read:
>
> http://radio.weblogs.com/0112098/2002/09/24.html#a114
>
> Rather than using:
>
> Class theClass = Class.forName( className );
>
> in reflect.ReflectionUtils, James suggests we should use:
>
> ****
> Class theClass = null;
>
> try {
>  theClass = Thread.currentThread().getContextClassLoader().loadClass(
> className );
> } catch (ClassNotFoundException e) {
>  theClass = getClass().getClassLoader().loadClass( className );
> }
> ****
>
> as "Class.forName is evil".
>
> Any views on this??
>
> Hen


--
To unsubscribe, e-mail:
<ma...@jakarta.apache.org>
For additional commands, e-mail:
<ma...@jakarta.apache.org>




--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [lang] reflect - Class.forName

Posted by Steve Downey <st...@netfolio.com>.
+1, as the reasoning seems quite sane.


On Saturday 23 November 2002 09:17 pm, Henri Yandell wrote:
> I've not really been paying attention to the reflect stuff, and had missed
> a question about it in a blog I usually read:
>
> http://radio.weblogs.com/0112098/2002/09/24.html#a114
>
> Rather than using:
>
> Class theClass = Class.forName( className );
>
> in reflect.ReflectionUtils, James suggests we should use:
>
> ****
> Class theClass = null;
>
> try {
>  theClass = Thread.currentThread().getContextClassLoader().loadClass(
> className );
> } catch (ClassNotFoundException e) {
>  theClass = getClass().getClassLoader().loadClass( className );
> }
> ****
>
> as "Class.forName is evil".
>
> Any views on this??
>
> Hen


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>