On Mon, 2006-02-13 at 22:33 +0000, robert burrell donkin wrote:
> On Mon, 2006-02-13 at 14:21 +1300, Simon Kitching wrote:
> > Hi Robert,
> >
> > I've had a look at this patch and it doesn't seem quite right to me.
>
> i had the same feeling :)
>
> but that might just be the specifications being unintuitive again...
>
> > Presumably this is to handle the case where the system classloader is
> > not in the chain leading from base classloader to boot classloader. This
> > is a rather odd situation, but I guess that in the new principle of "try
> > everything before failing" we could have a go with that when all else
> > fails.
>
> not quite
>
> this patch is intended to address allowed variability in the classloader
> class implementation noted in the javadocs for ClassLoader#getParent. it
> is possible that some JRE implementations "may use null to represent the
> bootstrap class loader. This method will return null in such
> implementations if this class loader's parent is the bootstrap class
> loader."
>
> the term bootstrap classloader is difficult. i suspect that this javadoc
> is prior to the introduction of boot, extension and system classloaders.
> i elected to assume that using the system classloader would usually be
> good enough (providing that recursive loops are avoided) since i don't
> know of any cross-JRE way of accessing the boot classloader. (if anyone
> knows, please jump in :)
I would interpret this differently. I think "bootstrap class loader" is
the same thing as "boot classloader", which is indeed represented by
"null" in the Sun JVM. A classloader for which getParent returns null is
a child of the boot classloader.
Classes in the boot classloader can be accessed via
ClassLoader loader = null;
Class.forName(className, true, loader);
By passing null as the loader to use, the boot classloader is used. This
is exactly what the code in method createLogFromClass is already doing;
because null is tested for only at the end of the loop, an attempt is
made to use the null (boot) classloader already.
In early JVMs, I believe ClassLoader.getSystemClassLoader returns null,
ie the "system" and "boot" classloaders are the same. I believe that
some embedded JVMs do the same thing, even if they support java > 1.1
Am I mistaken in thinking "bootstrap classloader" and "boot classloader"
are the same thing?
>
> but it seems like a lot of complex code to address a risk something this
> obscure...
>
> > However this is currently hooked in to fire when parentCL is null, ie
> > *before* we try the bootloader. I believe that in simple JVMs the
> > bootloader IS also the system classloader. So I think it would be better
> > as:
> >
> > final ClassLoader systemCL = ClassLoader.getSystemClassLoader();
> > boolean systemCLTried = false;
> > for(;;) {
> > // checks as currently done
> >
> > if (currentCL == systemCL) {
> > // the CL we just tested was the systemCL
> > systemCLTried = true;
> > }
> >
> > if (currentCL == null) {
> > if (systemCLTried == true) {
> > break;
> > }
> >
> > // We've walked up the CL tree, but never visited the systemCL.
> > // Before giving up, let's try walking up from the system CL too..
> > currentCL = systemCL;
> > } else {
> > currentCL = currentCL.getParent();
> > }
> > }
> >
> > Thoughts?
>
> this seems like a better formulation with equivalent effect.
Almost the same. However the above code effectively "inserts" the system
classloader only *after* trying the logic with the null classloader. The
recently committed patch inserts it *before* trying the null loader, and
never tries the null loader at all.
>
> i'd be equally happy to revert and go with just a dianostic message for
> now, though.
Lets do that then. The situation where the system classloader is not in
the chain from TCCL to boot is possible, but very weird.
Cheers,
Simon
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]