Hi Daniel, Mandy,

It seems that this change has caused regression in jconsole tool. It doesn't want to list local JVMs to attach to any more. The following fixes the issue:

--- old/src/jdk.jconsole/share/classes/sun/tools/jconsole/JConsole.java 2017-02-05 16:37:26.768771419 +0100 +++ new/src/jdk.jconsole/share/classes/sun/tools/jconsole/JConsole.java 2017-02-05 16:37:26.597774358 +0100
@@ -954,7 +954,7 @@
         boolean supported;
         try {
             Class.forName("com.sun.tools.attach.VirtualMachine");
-            Class.forName("sun.management.ConnectorAddressLink");
+ Class.forName("jdk.internal.agent.ConnectorAddressLink");
             supported = true;
         } catch (NoClassDefFoundError x) {
             supported = false;


Regards, Peter

On 02/01/2017 04:29 PM, Daniel Fuchs wrote:
Hi Mandy,

On 01/02/17 05:11, Mandy Chung wrote:

On Jan 31, 2017, at 10:26 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote:

Hi,

Please find below a patch for:

8173607: JMX RMI connector should be in its own module
https://bugs.openjdk.java.net/browse/JDK-8173607

webrev:
http://cr.openjdk.java.net/~dfuchs/webrev_8173607/webrev.05


This mostly looks good. I’d like to see an updated webrev after you sync with JDK-8173608. I believe you can revert test/sun/management/jdp and test/sun/management/jmxremote tests change and ConnectorBootstrap.java as you noted. Also you can run jdeps —-check on java.rmi, java.management, and jdk.management.agent to update its requires and qualified exports. java.management should no longer require java.rmi and the qualified exports from java.rmi is not needed.

Here is the updated webrev, rebased after pulling JDK-8173608, and with
your feedback below integrated.

I am pleased to report that java.management no longer requires
java.rmi or java.naming :-)

Compared to previous version, then RMIExporter has been moved
to java.management.rmi, various module-info.java have been
cleaned up, some @modules in tests have been updated (mostly
due to the RMIExporter move).

I have also improved some javadoc comments in JMXConnectorFactory.java
No changes in build files compared to webrev.05

http://cr.openjdk.java.net/~dfuchs/webrev_8173607/webrev.06
http://cr.openjdk.java.net/~dfuchs/webrev_8173607/webrev.06/java.management.rmi-summary.html

best regards,

-- daniel


java.management.rmi module-info.java
  32  * @see javax.management.remote.rmi

This @see is not needed. This package is listed in the exported packages table.

JMXConnectorFactory.java
I like the ProviderFinder approach to look up the custom providers and platform providers; and shared code used by both JMXConnectorFactory and JMXConnectorServerFactory which is good.

Nit: line 481-491 - this is javadoc and probably /* .. */ comment block is more appropriate here.


Some further cleanup of java.base and java.rmi module-info.java
should become possible once JDK-8173608 is in (as well as moving
RMIExporter to java.management.rmi - which is not possible yet).


Yes.

Further cleanup of @modules in tests might become possible as
well.


Hope there will be a bulk @modules cleanup some time.

Thanks
Mandy



Reply via email to