Reviewer and committer request for 7198496

[Paul Sandoz]

On Oct 4, 2012, at 12:37 PM, David Holmes <david.holmes at oracle.com> wrote:

> Paul,
> 
> On 4/10/2012 7:53 PM, Paul Sandoz wrote:
>> On Oct 4, 2012, at 11:34 AM, David Holmes<david.holmes at oracle.com>  wrote:
>>>> 
>>>>> 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
>>> 
>> 
>> So you are proposing to widen the scope? since i interpret the current JavaDoc to correspond to only the latter two options.
> 
> I think you are missing the point. If getCCL returns null it returns null - end of story.

> What the caller of getCCL does after that is their business, it has nothing to do with the spec for getCCL.

I think we may be misunderstanding and/or talking over each other. IIUC you are saying what behaviour you would like and i am saying how i interpret the current behaviour.

I interpret the current JavaDoc as normatively stating what the caller of getCCL *must* do if null a value is returned.

A little tweak to the doc might help:

From:

"@return  the context ClassLoader for this Thread, or {@code null} indicating the system class loader (or, failing that, the bootstrap class loader)"

To:

"@return  the context ClassLoader for this Thread, or {@code null} indicating the caller shall use the system class loader (or, failing that, the bootstrap class loader) in place of what would otherwise be the non-null context ClassLoader."

Because TCCL has the annoying property of decoupling behaviour (or "action at a distance") i think i can see why it was documented in such a way.


So from my perspective you are proposing a change in the specified behaviour, perhaps to something like:

@return  the context ClassLoader for this Thread, or {@code null} indicating the context ClassLoader is undefined. It is recommended that ...

Which might be fine, from a backwards compatibility perspective, i am not sure, but i have been on the receiving end of some very tricky bugs due to TCCL, hence a conservative position.


<snip>
>  loader = (cl == null) ? ClassLoader.getSystemClassLoader() : cl;
> 
> which has the same semantic effect, but leaves some "dead" code internally as now loader can not be null.
> 

It can if the system CL is null.

Paul.