RFR 8042901: Allow com.sun.management to be in a different module to java.lang.management

Mandy Chung mandy.chung at oracle.com
Wed Apr 1 21:41:54 UTC 2015


On 3/31/2015 9:39 AM, shanliang wrote:
> Please review this fix:
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8042901
> Webrev: http://cr.openjdk.java.net/~sjiang/JDK-8042901/00/

Thank you for doing this big change to separate com.sun.management
API from java.management module.   Looking really good.

Also thanks for fixing the tests to eliminate the unnecessary use of
JDK internal APIs.

modules.xml change looks good to me.

sun/management/ThreadImpl.java
   35 /**
   36  * Implementation class for the thread subsystem.
   37  * Standard and committed hotspot-specific metrics if any.
   38  *
   39  * ManagementFactory.getThreadMXBean() returns an instance
   40  * of this class.
   41  */
   42 // the implementation for com.sun.management.ThreadMXBean can
   43 // be moved to jdk.management in the future.

What about:
   Implementation for java.lang.management.ThreadMXBean as well as
   providing the supporting method for com.sun.management.ThreadMXBean.
   
   The supporting method for com.sun.management.ThreadMXBean can
   be moved to jdk.management in the future.

CheckSomeMXBeanImplPackage.java
    Thanks for adding this test.

    Iterating jrt file system to find jdk.management module is one way.
    Another simpler alternative is to call:
      Class.forName("com.sun.management.GarbageCollectorMXBean");
    and catch CNFE to determine if it's present or not.

    You should also call ManagementFactory.getPlatformMXBeans on
    com.sun.management.GarbageCollectorMXBean if present.

VMOptionOpenDataTest.java
    copyright header year is wrong.

PlatformMBeanProviderConstructorCheck.java
    The test needs to restore the original policy and security manager
    before the test exits in case this case runs in agent vm mode.
    The static permName and accepted variables are more appropriate
    in MyPolicy class.  Perhaps rename "accepted" to an instance
    "denied" or "allowed" variable of MyPolicy class to reflect
    what it intends to mean.

I'm okay if you make the change before the push.  No need for a new
webrev.

Mandy



More information about the jigsaw-dev mailing list