Review Request: 7193339 Prepare system classes be defined by a non-null module loader
mandy.chung at oracle.com
Thu Aug 23 23:22:56 UTC 2012
On 8/23/2012 2:26 PM, Dmitry Samersoff wrote:
> 1. You replace null with getClassLoader() calls in couple of places.
> getClassLoader requires special permissions -
> 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 .
> 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
> 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.
> 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 
>>>> 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:
>>> 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.
>> Thanks for the review.
More information about the core-libs-dev