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

[David Holmes]

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 }

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.

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
>