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.lev...@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.fu...@oracle.com>
>>>> <mailto: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
>>>> <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
>>>
>>
>
