Review Request: 7193339 Prepare system classes be defined by a non-null module loader

[Mandy Chung]

On 8/23/2012 2:26 PM, Dmitry Samersoff wrote:
> Mandy,
>
> 1. You replace null with getClassLoader() calls in couple of places.
>     getClassLoader requires special permissions -
>     RuntimePermission("getClassLoader")
>
>     so probably you need to use doPrivileget() there.

No, the caller's class loader (all are system classes) is null and so no 
permission check is required [1].

>     Did you test your changes with SecurityManager/No permissions
>     for the test ?

I ran the jck tests that cover this permission check.  The jck tests 
uncovered these classes to be fixed during jigsaw development.

>
> 2. Did you consider moving
>
> sm.checkPermission(SecurityConstants.GET_CLASSLOADER_PERMISSION);
>
> inside ClassLoader.needsClassLoaderPermissionCheck(ccl, cl); ?

Yes I did and I decide to leave this sm.checkPermission call in the 
method body as it's closer to the spec and more readable.

>
> 3. ManagementFactory.java
>
> Could you explain the reason of changes (except 588) ?

The platform MXBeans (java.lang.management) are currently on the 
bootclasspath loaded by the null loader.  In the modular world, j.l.m. 
will possibly be put in the "management" module along with JMX so that 
an application only requires the management module to be present when it 
uses JMX and platform MXBean.  In that case, the management module will 
be loaded by a non-null module loader.  This check (loader != null) 
would fail in the modular world even loading platform MXBeans.   We 
changed this check and other places in the JDK to call 
VM.isSystemDomainLoader method and the real check for modules can be 
made in this single convenient method in the jigsaw repo.

> -Dmitry

Thanks
Mandy
  [1] 
http://docs.oracle.com/javase/7/docs/api/java/lang/Class.html#getClassLoader()

>
> On 2012-08-23 23:33, Mandy Chung wrote:
>> On 8/23/2012 11:58 AM, Alan Bateman wrote:
>>> On 23/08/2012 18:43, Mandy Chung wrote:
>>>> This change is to bring the jdk modularization changes from jigsaw
>>>> repo [1]
>>>> to jdk8.  This allows the jdk modularization changes to be exposed for
>>>> testing as early as possible and minimize the amount of changes carried
>>>> in the jigsaw repo that helps sync up jigsaw with jdk8/jdk9.
>>>>
>>>> Webrev at:
>>>>    http://cr.openjdk.java.net/~mchung/jdk8/webrevs/7193339/webrev.00/
>>>>
>>> This looks good to me and it's good to have these changes in jdk8.
>>>
>>> One suggestion for ReflectUtil is to add a private static boolean
>>> isAncestor method as I think that would make the checks in
>>> needsPackageAccessCheck easier to read. Also in ClassLoader you could
>>> use just use needsClassLoaderPermissionCheck(from,to).
>>>
>> Done.  This is a good cleanup I considered to do too but missed in the
>> previous webrev.
>>
>> http://cr.openjdk.java.net/~mchung/jdk8/webrevs/7193339/webrev.01/
>>
>> Thanks for the review.
>> Mandy
>