PING: RFR: 8165736: Error message should be shown when JVMTI agent cannot be attached

[Yasumasa Suenaga]

Hi Serguei,

Thank you for your comment!
I uploaded new webrev:

   http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.01/


> http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.00/test/jdk/com/sun/tools/attach/StartManagementAgent.java.udiff.html
> 
> - if (!ex.getMessage().contains("Invalid com.sun.management.jmxremote.port number")) {
> + if (!ex.getMessage().contains("For input string: \"apa\"")) {
> 
> 
>    What is the motivation for this change?
>    It seems, the original comparison is better.

"ex" is AttachOperationFailedException.
We can get the result as below when we run StartManagementAgent:

-------------
[runApplication] Error: Invalid com.sun.management.jmxremote.port number: apa
[runApplication] jdk.internal.agent.AgentConfigurationError: java.lang.NumberFormatException: For input string: "apa"
[runApplication]        at jdk.management.agent/sun.management.jmxremote.ConnectorBootstrap.startRemoteConnectorServer(ConnectorBootstrap.java:336)
-------------

I think we should check exception message in AttachOperationFailedException.
In fact, jtreg fails at this point in my environment.


> What tests did you run to make sure there are no regressions?

I've tested the following testcases:

   - hotspot/jtreg/serviceability/attach
   - hotspot/jtreg/serviceability/dcmd/jvmti
   - jdk/com/sun/tools/attach


Thanks,

Yasumasa


On 2017/11/03 16:31, serguei.spitsyn at oracle.com wrote:
> Hi Yasumasa,
> 
> Some comments.
> 
> http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.00/test/jdk/com/sun/tools/attach/StartManagementAgent.java.udiff.html
> 
> - if (!ex.getMessage().contains("Invalid com.sun.management.jmxremote.port number")) {
> + if (!ex.getMessage().contains("For input string: \"apa\"")) {
> 
> 
>    What is the motivation for this change?
>    It seems, the original comparison is better.
> 
> 
> http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.00/test/hotspot/jtreg/serviceability/dcmd/jvmti/AttachFailed/AttachException.java.html
> http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.00/test/hotspot/jtreg/serviceability/dcmd/jvmti/AttachFailed/AttachIncorrectLibrary.java.html
> http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.00/test/hotspot/jtreg/serviceability/dcmd/jvmti/AttachFailed/AttachNoEntry.java.html
> http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.00/test/hotspot/jtreg/serviceability/dcmd/jvmti/AttachFailed/AttachReturnError.java.html
> 
>    37     public void run(CommandExecutor executor)  {
>    38         try{
> 
>    A space is missed after 'try'.
> 
> 
>    It is odd that all test java classes define exactly the same methods: sharedObjectName(), jmx() and cli().
>    Would it better to defin a common base class with these methods?
> 
> 
> Otherwise, it looks good.
> Thank you for taking care about it!
> 
> What tests did you run to make sure there are no regressions?
> 
> Thanks,
> Serguei
> 
> 
> 
> On 11/1/17 05:59, Yasumasa Suenaga wrote:
>> PING: Could you review and sponsor it?
>>
>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.00/
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>> On 2017/09/29 13:24, Yasumasa Suenaga wrote:
>>> Hi all,
>>>
>>> If we try to attach invalid JVMTI agent via JVMTI.agent_load dcmd, we
>>> will get "Command executed successfully". However, it implies error in
>>> JVMTIAgentLoadDCmd.
>>> This message is from JCmd.java when jcmd does not receive any output
>>> from target VM.
>>>
>>> I think HotSopt/jcmd should return useful error message to users to
>>> understand the cause of failure.
>>>
>>> I uploaded webrev for this issue. Could you review it?
>>>
>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.00/
>>>
>>>
>>> This change is work fine on Fedora 26 x86_64 as following  jtreg testcases:
>>>
>>>    - hotspot/jtreg/serviceability/attach
>>>    - hotspot/jtreg/serviceability/dcmd/jvmti
>>>    - jdk/com/sun/tools/attach
>>>
>>> I cannot access JPRT. So I need a sponsor.
>>> (I cannot test it on other platforms.)
>>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>