Hi,

I have to ask the review again because I need to modify:
   langtools/src/jdk.dev/share/classes/com/sun/tools/jdeps/Profile.java

The issue was found when langtools tests were added into my test list.

The new version is:
   http://cr.openjdk.java.net/~sjiang/JDK-8042901/02/

which integrated also the Mandy's comments in the following mail.

Thanks,
Shanliang

Mandy Chung wrote:
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


Reply via email to