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
