commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From robert burrell donkin <robertburrelldon...@blueyonder.co.uk>
Subject Re: [logging] r375631 - systemClassloaderTried patch
Date Mon, 13 Feb 2006 22:33:31 GMT
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 :)

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.

i'd be equally happy to revert and go with just a dianostic message for
now, though.

- robert


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Mime
View raw message