RFR(S): 8185694: Replace SystemDictionaryShared::_java_platform_loader with SystemDictionary::is_platform_class_loader()

[coleen.phillimore at oracle.com]

On 10/5/17 9:33 PM, David Holmes wrote:
> Hi Coleen, Calvin,
>
> On 6/10/2017 6:54 AM, coleen.phillimore at oracle.com wrote:
>> So if you use -Djava.system.loader=myLoader on the command line, 
>> setting _java_system_loader, then does that mean that the classes 
>> loaded by 
>> SystemDictionary::jdk_internal_loader_ClassLoaders_AppClassLoader_klass() 
>> are not in the system loader?  ie. they can be unloaded?  What is the 
>> result of the call to SystemDictionary::is_system_class_loader() used 
>> for?   I guess same question applies to the platform class loader.
>
> The classloading delegation hierarchy (as of JDK 9) is:
> - boot loader (native)
>   - platform loader (built-in)
>     - system (aka application) loader (built-in)
>
> If the user specifies a custom class for the system loader then it is 
> loaded by an instance of the default system loader and becomes a 
> fourth level in the hierarchy, and it is then technically the "system 
> loader". None of these loaders, or the classes they load can be unloaded.
>
> Which is presumably why the code checks both:
>
>  180 bool SystemDictionary::is_system_class_loader(oop class_loader) {
>  181   if (class_loader == NULL) {
>  182     return false;
>  183   }
>  184   return (class_loader->klass() == 
> SystemDictionary::jdk_internal_loader_ClassLoaders_AppClassLoader_klass() 
> ||
>  185           class_loader == _java_system_loader);
>  186 }
>

Okay then this code shouldn't change.
> because we need to treat both of these instances as the "system 
> loader" from a VM perspective? The same does not apply to the platform 
> loader.

I don't think we have good tests for this.

Coleen
>
> David
> -----
>
>> thanks,
>> Coleen
>>
>>>
>>>> The implementation is in closed source.
>>> Please clean up the closed code to remove those.
>>>
>>>
>>> Thanks,
>>>
>>> Jiangli
>>>
>>>>> Is this new java_platform_loader function used anywhere?
>>>> Yes, it is being used in closed source.
>>>>> Currently 
>>>>> SystemDictionary::jdk_internal_loader_ClassLoaders_PlatformClassLoader_klass 
>>>>> is preloaded.  Shouldn't this be removed?  What about 
>>>>> jdk_internal_loader_ClassLoaders_AppClassLoader?
>>>> They're being used in lines 184 and 193 in systemDictionary.cpp and 
>>>> also in closed source.
>>>>> thread.cpp
>>>>>
>>>>> 3752 SystemDictionary::compute_java_loaders(CHECK_(JNI_ERR));
>>>>>
>>>>> What is the difference between CHECK_(JNI_ERR) vs CHECK_JNI_ERR?  
>>>>> Should it simply use CHECK_JNI_ERR as in other places?
>>>> They are the same, in utilities/exceptions.hpp:
>>>> #define CHECK_JNI_ERR CHECK_(JNI_ERR)
>>>>
>>>> and it expands to the following:
>>>> __the_thread__); if 
>>>> ((((ThreadShadow*)__the_thread__)->has_pending_exception())) return 
>>>> (-1); (void)(0
>>>>
>>>> I can change it to CHECK_JNI_ERR.
>>>>
>>>> thanks,
>>>> Calvin
>>>>> Mandy
>>