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

Mandy Chung mandy.chung at oracle.com
Thu Aug 23 23:22:56 UTC 2012

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.GETCLASSLOADERPERMISSION); 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

More information about the core-libs-dev mailing list