Reviewer and committer request for 7198496

[David Holmes]

On 3/10/2012 11:37 PM, Paul Sandoz wrote:
> On Oct 3, 2012, at 3:17 PM, David Holmes<david.holmes at oracle.com>  wrote:
>> On 3/10/2012 10:50 PM, Paul Sandoz wrote:
>>> For the benefit of others; for some context see this recent thread:
>>>
>>> http://markmail.org/search/?q=openjdk#query:openjdk%20list%3Anet.java.openjdk.core-libs-dev%20order%3Adate-backward+page:2+mid:inod424lqbey5fms+state:results
>>>
>>> Basically ServiceLoader is treating a null CL parameter as the system CL for loading the META-INF/services resources file and as the bootstrap CL for loading the classes of class names declared in those resource files.
>>>
>>> Which also means that calls to:
>>>
>>>    ServiceLoader.load(serviceInterface)
>>>    ServiceLoader.load(serviceInterface, Thread.currentThread().getContextClassLoader());
>>>
>>> Will behave oddly if the TCCL is null, from the JavaDoc of Thread.getContextClassLoader():
>>>
>>>    http://docs.oracle.com/javase/7/docs/api/java/lang/Thread.html#getContextClassLoader()
>>>    Returns:
>>>    the context ClassLoader for this Thread, or null indicating the system class loader (or, failing that, the bootstrap class loader)
>>
>> The @return doc for getContextClassLoader is wrong, or at best mis-leading. A null return does NOT indicate the system class loader (which is never null) - it simply means there is no CCL for that thread. If anything null would represent the bootstrap loader.
>
>>
>
> In the JDK i see code such as:
>
>                  Class c = Class.forName(className, true,Thread.currentThread().
>                                          getContextClassLoader());
>
> and:
>
>          ClassLoader contextClassLoader =
>              Thread.currentThread().getContextClassLoader();
>          if (contextClassLoader == null) {
>              contextClassLoader = ClassLoader.getSystemClassLoader();
>          }
>
> The only way i can interpret that TCCL JavaDoc sanely is to assume "null" is overloaded to mean the caller should try to use system or bootstrap CL if possible, otherwise failing that the bootstrap CL.

The TCCL doc for @return is simply confusing. The method will return the 
TCCL or it will return null. If it returns null it means the Thread has 
no CCL set. So what does a caller then do? They have a few choices:
a) use their own ClassLoader
b) use the system ClassLoader
c) use the the bootstrap loader

Your first example chooses (c) because the null arg to forName will 
indicate to use the bootstrap loader. Your second example chooses (b). 
This is not an arbitrary choice as it depends on the API - the three arg 
version of forName treats null as the bootstrap loader, but if you 
simply did getContextClassLoader().loadClass(...) then you would get NPE.

The point is that returning null in itself has no direct significance. 
The issue here is how ServiceLoader should attempt to load something if 
there is no CCL set for the thread.

David

> Paul.