RFR(S): 8185694: Replace SystemDictionaryShared::_java_platform_loader with SystemDictionary::is_platform_class_loader()
Calvin Cheung
calvin.cheung at oracle.com
Tue Oct 10 20:38:33 UTC 2017
Thanks Ioi.
Calvin
On 10/10/17, 1:02 PM, Ioi Lam wrote:
> Looks good. Thanks!
>
> - Ioi
>
>
> On 10/10/17 11:53 AM, Calvin Cheung wrote:
>> I ran into some runtime issue when creating the _java_platform_loader
>> before initPhase2.
>> I've filed the following to track the above issue:
>> https://bugs.openjdk.java.net/browse/JDK-8189120
>>
>> I'm going with the fix similar to version.02 - creating the system
>> and platform loaders after initPhase3.
>> updated webrev:
>> http://cr.openjdk.java.net/~ccheung/8185694/webrev.04/
>>
>> thanks,
>> Calvin
>>
>> On 10/5/17, 10:38 PM, David Holmes wrote:
>>> On 6/10/2017 3:28 PM, Calvin Cheung wrote:
>>>> On 10/5/17, 6: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 }
>>>>>
>>>>> 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.
>>>> We're obtaining the _java_system_loader after initPhase3 even
>>>> before this change. Roughly, the calling sequence of initPhase3 is
>>>> as follows:
>>>>
>>>> call_initPhase3()
>>>> -> ClassLoader.initPhase3()
>>>> -> ClassLoader.initSystemClassLoader() which contains the
>>>> following code:
>>>>
>>>> String cn = System.getProperty("java.system.class.loader");
>>>> if (cn != null) {
>>>> try {
>>>> // custom class loader is only supported to be
>>>> loaded from unnamed module
>>>> Constructor<?> ctor = Class.forName(cn, false,
>>>> builtinLoader)
>>>> .getDeclaredConstructor(ClassLoader.class);
>>>> scl = (ClassLoader) ctor.newInstance(builtinLoader);
>>>> } catch (Exception e) {
>>>> throw new Error(e);
>>>> }
>>>> } else {
>>>> scl = builtinLoader;
>>>> }
>>>> return scl;
>>>>
>>>> So initSystemClassLoader() will either return the built-in
>>>> system loader or a custom loader if it exists.
>>>>
>>>> We use the getSystemClassLoader API to obtain the _java_system_loader:
>>>>
>>>> public static ClassLoader getSystemClassLoader() {
>>>> switch (VM.initLevel()) {
>>>> case 0:
>>>> case 1:
>>>> case 2:
>>>> // the system class loader is the built-in app
>>>> class loader during startup
>>>> return getBuiltinAppClassLoader();
>>>> case 3:
>>>> String msg = "getSystemClassLoader should only be
>>>> called after VM booted";
>>>> throw new InternalError(msg);
>>>> case 4:
>>>> // system fully initialized
>>>> assert VM.isBooted() && scl != null;
>>>> SecurityManager sm = System.getSecurityManager();
>>>> if (sm != null) {
>>>> checkClassLoaderPermission(scl,
>>>> Reflection.getCallerClass());
>>>> }
>>>> return scl;
>>>> default:
>>>> throw new InternalError("should not reach here");
>>>> }
>>>> }
>>>>
>>>> So the _java_system_loader will either be the built-in system
>>>> loader or a custom loader. (case 4 in the above)
>>>>
>>>> I don't quite understand why the check in line 184 is required?
>>>> Is it for checking if a given class_loader is the same type
>>>> (like an instanceof) as the built-in system loader?
>>>
>>> I believe it is checking if the loader is the built-in default
>>> system loader, both to account for the case where/if
>>> SystemDictionary::is_system_class_loader is called prior to
>>> initPhase3 completing; and also to account for encountering the
>>> default-built-in loader when the custom system loader delegates to it.
>>>
>>> I'd have to examine every call path to
>>> SystemDictionary::is_system_class_loader to check all the details.
>>>
>>> David
>>> -----
>>>
>>>> thanks,
>>>> Calvin
>>>>>
>>>>> 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
>>>>>>
>
More information about the hotspot-runtime-dev
mailing list