Reviewer and committer request for 7198496

[David Holmes]

On 4/10/2012 6:52 PM, Paul Sandoz wrote:
> On Oct 4, 2012, at 10:14 AM, David Holmes<david.holmes at oracle.com>  wrote:
>>> I am not sure i agree. The TCCL can be set to null and that has significance as per the JavaDoc:
>>>
>>>       * @param  cl
>>>       *         the context ClassLoader for this Thread, or null  indicating the
>>>       *         system class loader (or, failing that, the bootstrap class loader)
>>>       *
>>>       * @throws  SecurityException
>>>       *          if the current thread cannot set the context ClassLoader
>>>       *
>>>       * @since 1.2
>>>       */
>>>      public void setContextClassLoader(ClassLoader cl) {
>>
>> Sorry but that is just the dual incorrect statement of what is in getContextClassLoader. It is not a meaningful description, in fact it is incorrect because null only ever means the bootstrap loader, never the system class loader - and the null can't mean two different things.
>
>> And what is "or failing that" supposed to mean?
>>
>
> I presume it means if the system CL is null.
>
> I would interpret the JavaDoc as follows: if I set the TCCL to null on a thread i would expect any consumer of the TCCL for that thread to first use the system CL. Which matches the behaviour of how SL is documented.
>
>
>> These parts of the javadoc for get/setContextClassLoader are simply wrong.
>>
>
> Wrong or not because of such JavaDoc developers have been coding using TCCL with certain expectations of "null" different to that of just meaning the bootstrap CL.
>
> How would you propose to fix it?

I don't see anything that actually needs fixing (apart from the 
javadoc). Anyone using getCCL has to expect null as a possibility and 
they are then free to proceed however they like. The obvious ways to 
proceed are as I outlined earlier:
- use current classloader
- use system loader
- use bootstrap loader

which is appropriate depends on the code that was using getCCL. Often 
the current loader will be either the system or bootloader so that 
narrows the options.

> Did i tell you i really dislike TCCL :-)

:) It was a necessary evil to handle dynamic loading from extensions.

Cheers,
David

> Paul.
>