RFR: 8173607: JMX RMI connector should be in its own module

[Mandy Chung]

Thanks for catching this.  I’ll file a bug and fix it.  We can drop Class.forName since jdk.jconsole requires jdk.attach and jdk.management.agent.

Mandy

> On Feb 5, 2017, at 7:41 AM, Peter Levart <peter.levart at gmail.com> wrote:
> 
> 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.fuchs at oracle.com> <mailto:daniel.fuchs at 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 <https://bugs.openjdk.java.net/browse/JDK-8173607> 
>>>> 
>>>> webrev: 
>>>> http://cr.openjdk.java.net/~dfuchs/webrev_8173607/webrev.05 <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> 
>> http://cr.openjdk.java.net/~dfuchs/webrev_8173607/webrev.06/java.management.rmi-summary.html <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 
>>> 
>> 
>