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

classic Classic list List threaded Threaded
23 messages Options
12
Reply | Threaded
Open this post in threaded view
|

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

Yasumasa Suenaga-4
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
Reply | Threaded
Open this post in threaded view
|

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

Yasumasa Suenaga-4
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
>
Reply | Threaded
Open this post in threaded view
|

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

serguei.spitsyn@oracle.com
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


Reply | Threaded
Open this post in threaded view
|

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

Yasumasa Suenaga-4
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, [hidden email] 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
>>>
>
Reply | Threaded
Open this post in threaded view
|

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

serguei.spitsyn@oracle.com
Hi Yasumasa,

The changes looks good.
Thank you for making them!



On 11/3/17 05:10, Yasumasa Suenaga wrote:

> 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.


I'd expect a check for some exception name, not for details like: For
input string: "apa".



>> 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

There are more tests related to dynamic attach in closed,
nsk.aod.testlist and 30+ attach tests in nsk.jvmti.testlist.
I'm not sure, if they are included into any of the Mach5 testing levels.
Will need to check.
We have to make sure these tests are still passed.

Thanks,
Serguei

> Thanks,
>
> Yasumasa
>
>
> On 2017/11/03 16:31, [hidden email] 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
>>>>
>>

Reply | Threaded
Open this post in threaded view
|

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

Yasumasa Suenaga-4
Hi Serguei,

On 2017/11/06 20:06, [hidden email] wrote:
> Hi Yasumasa,
>
> The changes looks good.
> Thank you for making them!

Thanks!


> On 11/3/17 05:10, Yasumasa Suenaga wrote:
>> 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.
>
>
> I'd expect a check for some exception name, not for details like: For input string: "apa".

Should we remove this comparison?


>>> 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
>
> There are more tests related to dynamic attach in closed, nsk.aod.testlist and 30+ attach tests in nsk.jvmti.testlist.
> I'm not sure, if they are included into any of the Mach5 testing levels. Will need to check.
> We have to make sure these tests are still passed.

I cannot access JPRT and closed testcases because I'm not an Oracle employee.
Can you run them with this change?


Thanks,

Yasumasa


> Thanks,
> Serguei
>
>> Thanks,
>>
>> Yasumasa
>>
>>
>> On 2017/11/03 16:31, [hidden email] 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
>>>>>
>>>
>
Reply | Threaded
Open this post in threaded view
|

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

serguei.spitsyn@oracle.com
On 11/6/17 04:31, Yasumasa Suenaga wrote:

> Hi Serguei,
>
> On 2017/11/06 20:06, [hidden email] wrote:
>> Hi Yasumasa,
>>
>> The changes looks good.
>> Thank you for making them!
>
> Thanks!
>
>
>> On 11/3/17 05:10, Yasumasa Suenaga wrote:
>>> 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.
>>
>>
>> I'd expect a check for some exception name, not for details like: For
>> input string: "apa".
>
> Should we remove this comparison?

I don't understand. Why do remove?
Would it better to check for the exception name instead?


>>>> 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
>>
>> There are more tests related to dynamic attach in closed,
>> nsk.aod.testlist and 30+ attach tests in nsk.jvmti.testlist.
>> I'm not sure, if they are included into any of the Mach5 testing
>> levels. Will need to check.
>> We have to make sure these tests are still passed.
>
> I cannot access JPRT and closed testcases because I'm not an Oracle
> employee.
> Can you run them with this change?

Ok.
I will sponsor this fix and run these tests before the push.

It seems, another update and one more review is needed.

Thanks,
Serguei

>
>
> Thanks,
>
> Yasumasa
>
>
>> Thanks,
>> Serguei
>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>>
>>> On 2017/11/03 16:31, [hidden email] 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
>>>>>>
>>>>
>>

Reply | Threaded
Open this post in threaded view
|

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

Yasumasa Suenaga-4
Hi Serguei,

I uploaded new webrev:

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

>>> I'd expect a check for some exception name, not for details like: For
>>> input string: "apa".
>>
>>
>> Should we remove this comparison?
>
>
> I don't understand. Why do remove?
> Would it better to check for the exception name instead?

I've changed to check exception name (NumberFormatException) in
StartManagementAgent.java.

> I will sponsor this fix and run these tests before the push.

Thanks!
I'm waiting for second reviewer.


Yasumasa


2017-11-08 11:55 GMT+09:00 [hidden email]
<[hidden email]>:

> On 11/6/17 04:31, Yasumasa Suenaga wrote:
>>
>> Hi Serguei,
>>
>> On 2017/11/06 20:06, [hidden email] wrote:
>>>
>>> Hi Yasumasa,
>>>
>>> The changes looks good.
>>> Thank you for making them!
>>
>>
>> Thanks!
>>
>>
>>> On 11/3/17 05:10, Yasumasa Suenaga wrote:
>>>>
>>>> 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.
>>>
>>>
>>>
>>> I'd expect a check for some exception name, not for details like: For
>>> input string: "apa".
>>
>>
>> Should we remove this comparison?
>
>
> I don't understand. Why do remove?
> Would it better to check for the exception name instead?
>
>
>>>>> 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
>>>
>>>
>>> There are more tests related to dynamic attach in closed,
>>> nsk.aod.testlist and 30+ attach tests in nsk.jvmti.testlist.
>>> I'm not sure, if they are included into any of the Mach5 testing levels.
>>> Will need to check.
>>> We have to make sure these tests are still passed.
>>
>>
>> I cannot access JPRT and closed testcases because I'm not an Oracle
>> employee.
>> Can you run them with this change?
>
>
> Ok.
> I will sponsor this fix and run these tests before the push.
>
> It seems, another update and one more review is needed.
>
> Thanks,
> Serguei
>
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>>> Thanks,
>>> Serguei
>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>> On 2017/11/03 16:31, [hidden email] 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
>>>>>>>
>>>>>
>>>
>
Reply | Threaded
Open this post in threaded view
|

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

serguei.spitsyn@oracle.com
Hi Yasumasa,

It looks good to me.

Thanks,
Serguei


On 11/7/17 22:38, Yasumasa Suenaga wrote:

> Hi Serguei,
>
> I uploaded new webrev:
>
>    http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.02/
>
>>>> I'd expect a check for some exception name, not for details like: For
>>>> input string: "apa".
>>>
>>> Should we remove this comparison?
>>
>> I don't understand. Why do remove?
>> Would it better to check for the exception name instead?
> I've changed to check exception name (NumberFormatException) in
> StartManagementAgent.java.
>
>> I will sponsor this fix and run these tests before the push.
> Thanks!
> I'm waiting for second reviewer.
>
>
> Yasumasa
>
>
> 2017-11-08 11:55 GMT+09:00 [hidden email]
> <[hidden email]>:
>> On 11/6/17 04:31, Yasumasa Suenaga wrote:
>>> Hi Serguei,
>>>
>>> On 2017/11/06 20:06, [hidden email] wrote:
>>>> Hi Yasumasa,
>>>>
>>>> The changes looks good.
>>>> Thank you for making them!
>>>
>>> Thanks!
>>>
>>>
>>>> On 11/3/17 05:10, Yasumasa Suenaga wrote:
>>>>> 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.
>>>>
>>>>
>>>> I'd expect a check for some exception name, not for details like: For
>>>> input string: "apa".
>>>
>>> Should we remove this comparison?
>>
>> I don't understand. Why do remove?
>> Would it better to check for the exception name instead?
>>
>>
>>>>>> 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
>>>>
>>>> There are more tests related to dynamic attach in closed,
>>>> nsk.aod.testlist and 30+ attach tests in nsk.jvmti.testlist.
>>>> I'm not sure, if they are included into any of the Mach5 testing levels.
>>>> Will need to check.
>>>> We have to make sure these tests are still passed.
>>>
>>> I cannot access JPRT and closed testcases because I'm not an Oracle
>>> employee.
>>> Can you run them with this change?
>>
>> Ok.
>> I will sponsor this fix and run these tests before the push.
>>
>> It seems, another update and one more review is needed.
>>
>> Thanks,
>> Serguei
>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>>> Thanks,
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>> On 2017/11/03 16:31, [hidden email] 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
>>>>>>>>

Reply | Threaded
Open this post in threaded view
|

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

serguei.spitsyn@oracle.com
Hi Yasumasa,

Do the new tests pass in your runs?

In my runs 3 of 4 tests are failed with the errors like this:

  109 Running DCMD 'JVMTI.agent_load
/var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so'
through 'PidJcmdExecutor'
  110 Executing command
'[/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/jdk/bin/jcmd,
21951, JVMTI.agent_load
/var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so]'
  111 Command returned with exit code 0
  112 ---------------- stdout ----------------
  113 21951:
  114
/var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so
was not loaded.
  115
/var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so:
cannot open shared object file: No such file or directory
  116


Good news is that the attach-related tests from closed repository are
passed.


Thanks,
Serguei



On 11/14/17 16:40, [hidden email] wrote:

> Hi Yasumasa,
>
> It looks good to me.
>
> Thanks,
> Serguei
>
>
> On 11/7/17 22:38, Yasumasa Suenaga wrote:
>> Hi Serguei,
>>
>> I uploaded new webrev:
>>
>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.02/
>>
>>>>> I'd expect a check for some exception name, not for details like: For
>>>>> input string: "apa".
>>>>
>>>> Should we remove this comparison?
>>>
>>> I don't understand. Why do remove?
>>> Would it better to check for the exception name instead?
>> I've changed to check exception name (NumberFormatException) in
>> StartManagementAgent.java.
>>
>>> I will sponsor this fix and run these tests before the push.
>> Thanks!
>> I'm waiting for second reviewer.
>>
>>
>> Yasumasa
>>
>>
>> 2017-11-08 11:55 GMT+09:00 [hidden email]
>> <[hidden email]>:
>>> On 11/6/17 04:31, Yasumasa Suenaga wrote:
>>>> Hi Serguei,
>>>>
>>>> On 2017/11/06 20:06, [hidden email] wrote:
>>>>> Hi Yasumasa,
>>>>>
>>>>> The changes looks good.
>>>>> Thank you for making them!
>>>>
>>>> Thanks!
>>>>
>>>>
>>>>> On 11/3/17 05:10, Yasumasa Suenaga wrote:
>>>>>> 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.
>>>>>
>>>>>
>>>>> I'd expect a check for some exception name, not for details like: For
>>>>> input string: "apa".
>>>>
>>>> Should we remove this comparison?
>>>
>>> I don't understand. Why do remove?
>>> Would it better to check for the exception name instead?
>>>
>>>
>>>>>>> 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
>>>>>
>>>>> There are more tests related to dynamic attach in closed,
>>>>> nsk.aod.testlist and 30+ attach tests in nsk.jvmti.testlist.
>>>>> I'm not sure, if they are included into any of the Mach5 testing
>>>>> levels.
>>>>> Will need to check.
>>>>> We have to make sure these tests are still passed.
>>>>
>>>> I cannot access JPRT and closed testcases because I'm not an Oracle
>>>> employee.
>>>> Can you run them with this change?
>>>
>>> Ok.
>>> I will sponsor this fix and run these tests before the push.
>>>
>>> It seems, another update and one more review is needed.
>>>
>>> Thanks,
>>> Serguei
>>>
>>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>>> Thanks,
>>>>> Serguei
>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>> On 2017/11/03 16:31, [hidden email] 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
>>>>>>>>>
>

Reply | Threaded
Open this post in threaded view
|

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

Yasumasa Suenaga-4
Hi Serguei,

> Do the new tests pass in your runs?

Of course.
It seems not to exist jtreg native libraries.
I've tested as below:

   $ make build-test-hotspot-jtreg-native
   $ cd test
   $ $JT_HOME/bin/jtreg -ignore:quiet -nativepath:<builddir>/<confdir>/support/test/hotspot/jtreg/native/lib hotspot/jtreg/serviceability/attach hotspot/jtreg/serviceability/dcmd/jvmti jdk/com/sun/tools/attach


> Good news is that the attach-related tests from closed repository are passed.

Thanks!


Yasumasa


On 2017/11/15 16:38, [hidden email] wrote:

> Hi Yasumasa,
>
> Do the new tests pass in your runs?
>
> In my runs 3 of 4 tests are failed with the errors like this:
>
>   109 Running DCMD 'JVMTI.agent_load /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so' through 'PidJcmdExecutor'
>   110 Executing command '[/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/jdk/bin/jcmd, 21951, JVMTI.agent_load /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so]'
>   111 Command returned with exit code 0
>   112 ---------------- stdout ----------------
>   113 21951:
>   114 /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so was not loaded.
>   115 /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so: cannot open shared object file: No such file or directory
>   116
>
>
> Good news is that the attach-related tests from closed repository are passed.
>
>
> Thanks,
> Serguei
>
>
>
> On 11/14/17 16:40, [hidden email] wrote:
>> Hi Yasumasa,
>>
>> It looks good to me.
>>
>> Thanks,
>> Serguei
>>
>>
>> On 11/7/17 22:38, Yasumasa Suenaga wrote:
>>> Hi Serguei,
>>>
>>> I uploaded new webrev:
>>>
>>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.02/
>>>
>>>>>> I'd expect a check for some exception name, not for details like: For
>>>>>> input string: "apa".
>>>>>
>>>>> Should we remove this comparison?
>>>>
>>>> I don't understand. Why do remove?
>>>> Would it better to check for the exception name instead?
>>> I've changed to check exception name (NumberFormatException) in
>>> StartManagementAgent.java.
>>>
>>>> I will sponsor this fix and run these tests before the push.
>>> Thanks!
>>> I'm waiting for second reviewer.
>>>
>>>
>>> Yasumasa
>>>
>>>
>>> 2017-11-08 11:55 GMT+09:00 [hidden email]
>>> <[hidden email]>:
>>>> On 11/6/17 04:31, Yasumasa Suenaga wrote:
>>>>> Hi Serguei,
>>>>>
>>>>> On 2017/11/06 20:06, [hidden email] wrote:
>>>>>> Hi Yasumasa,
>>>>>>
>>>>>> The changes looks good.
>>>>>> Thank you for making them!
>>>>>
>>>>> Thanks!
>>>>>
>>>>>
>>>>>> On 11/3/17 05:10, Yasumasa Suenaga wrote:
>>>>>>> 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.
>>>>>>
>>>>>>
>>>>>> I'd expect a check for some exception name, not for details like: For
>>>>>> input string: "apa".
>>>>>
>>>>> Should we remove this comparison?
>>>>
>>>> I don't understand. Why do remove?
>>>> Would it better to check for the exception name instead?
>>>>
>>>>
>>>>>>>> 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
>>>>>>
>>>>>> There are more tests related to dynamic attach in closed,
>>>>>> nsk.aod.testlist and 30+ attach tests in nsk.jvmti.testlist.
>>>>>> I'm not sure, if they are included into any of the Mach5 testing levels.
>>>>>> Will need to check.
>>>>>> We have to make sure these tests are still passed.
>>>>>
>>>>> I cannot access JPRT and closed testcases because I'm not an Oracle
>>>>> employee.
>>>>> Can you run them with this change?
>>>>
>>>> Ok.
>>>> I will sponsor this fix and run these tests before the push.
>>>>
>>>> It seems, another update and one more review is needed.
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>>> Thanks,
>>>>>> Serguei
>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Yasumasa
>>>>>>>
>>>>>>>
>>>>>>> On 2017/11/03 16:31, [hidden email] 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
>>>>>>>>>>
>>
>
Reply | Threaded
Open this post in threaded view
|

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

David Holmes
On 15/11/2017 10:15 PM, Yasumasa Suenaga wrote:

> Hi Serguei,
>
>> Do the new tests pass in your runs?
>
> Of course.
> It seems not to exist jtreg native libraries.
> I've tested as below:
>
>    $ make build-test-hotspot-jtreg-native
>    $ cd test
>    $ $JT_HOME/bin/jtreg -ignore:quiet
> -nativepath:<builddir>/<confdir>/support/test/hotspot/jtreg/native/lib
> hotspot/jtreg/serviceability/attach
> hotspot/jtreg/serviceability/dcmd/jvmti jdk/com/sun/tools/attach

Please check that:

make test-image

followed by jtreg -nativepath:<build-dir>/images/test/hotspot/jtreg/native

also works.

Thanks,
David

>
>> Good news is that the attach-related tests from closed repository are
>> passed.
>
> Thanks!
>
>
> Yasumasa
>
>
> On 2017/11/15 16:38, [hidden email] wrote:
>> Hi Yasumasa,
>>
>> Do the new tests pass in your runs?
>>
>> In my runs 3 of 4 tests are failed with the errors like this:
>>
>>   109 Running DCMD 'JVMTI.agent_load
>> /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so'
>> through 'PidJcmdExecutor'
>>   110 Executing command
>> '[/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/jdk/bin/jcmd,
>> 21951, JVMTI.agent_load
>> /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so]'
>>
>>   111 Command returned with exit code 0
>>   112 ---------------- stdout ----------------
>>   113 21951:
>>   114
>> /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so
>> was not loaded.
>>   115
>> /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so:
>> cannot open shared object file: No such file or directory
>>   116
>>
>>
>> Good news is that the attach-related tests from closed repository are
>> passed.
>>
>>
>> Thanks,
>> Serguei
>>
>>
>>
>> On 11/14/17 16:40, [hidden email] wrote:
>>> Hi Yasumasa,
>>>
>>> It looks good to me.
>>>
>>> Thanks,
>>> Serguei
>>>
>>>
>>> On 11/7/17 22:38, Yasumasa Suenaga wrote:
>>>> Hi Serguei,
>>>>
>>>> I uploaded new webrev:
>>>>
>>>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.02/
>>>>
>>>>>>> I'd expect a check for some exception name, not for details like:
>>>>>>> For
>>>>>>> input string: "apa".
>>>>>>
>>>>>> Should we remove this comparison?
>>>>>
>>>>> I don't understand. Why do remove?
>>>>> Would it better to check for the exception name instead?
>>>> I've changed to check exception name (NumberFormatException) in
>>>> StartManagementAgent.java.
>>>>
>>>>> I will sponsor this fix and run these tests before the push.
>>>> Thanks!
>>>> I'm waiting for second reviewer.
>>>>
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>> 2017-11-08 11:55 GMT+09:00 [hidden email]
>>>> <[hidden email]>:
>>>>> On 11/6/17 04:31, Yasumasa Suenaga wrote:
>>>>>> Hi Serguei,
>>>>>>
>>>>>> On 2017/11/06 20:06, [hidden email] wrote:
>>>>>>> Hi Yasumasa,
>>>>>>>
>>>>>>> The changes looks good.
>>>>>>> Thank you for making them!
>>>>>>
>>>>>> Thanks!
>>>>>>
>>>>>>
>>>>>>> On 11/3/17 05:10, Yasumasa Suenaga wrote:
>>>>>>>> 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.
>>>>>>>
>>>>>>>
>>>>>>> I'd expect a check for some exception name, not for details like:
>>>>>>> For
>>>>>>> input string: "apa".
>>>>>>
>>>>>> Should we remove this comparison?
>>>>>
>>>>> I don't understand. Why do remove?
>>>>> Would it better to check for the exception name instead?
>>>>>
>>>>>
>>>>>>>>> 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
>>>>>>>
>>>>>>> There are more tests related to dynamic attach in closed,
>>>>>>> nsk.aod.testlist and 30+ attach tests in nsk.jvmti.testlist.
>>>>>>> I'm not sure, if they are included into any of the Mach5 testing
>>>>>>> levels.
>>>>>>> Will need to check.
>>>>>>> We have to make sure these tests are still passed.
>>>>>>
>>>>>> I cannot access JPRT and closed testcases because I'm not an Oracle
>>>>>> employee.
>>>>>> Can you run them with this change?
>>>>>
>>>>> Ok.
>>>>> I will sponsor this fix and run these tests before the push.
>>>>>
>>>>> It seems, another update and one more review is needed.
>>>>>
>>>>> Thanks,
>>>>> Serguei
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>>> Thanks,
>>>>>>> Serguei
>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> Yasumasa
>>>>>>>>
>>>>>>>>
>>>>>>>> On 2017/11/03 16:31, [hidden email] 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
>>>>>>>>>>>
>>>
>>
Reply | Threaded
Open this post in threaded view
|

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

serguei.spitsyn@oracle.com
Hi Yasumasa and David,


On 11/15/17 04:56, David Holmes wrote:

> On 15/11/2017 10:15 PM, Yasumasa Suenaga wrote:
>> Hi Serguei,
>>
>>> Do the new tests pass in your runs?
>>
>> Of course.
>> It seems not to exist jtreg native libraries.
>> I've tested as below:
>>
>>    $ make build-test-hotspot-jtreg-native
>>    $ cd test
>>    $ $JT_HOME/bin/jtreg -ignore:quiet
>> -nativepath:<builddir>/<confdir>/support/test/hotspot/jtreg/native/lib
>> hotspot/jtreg/serviceability/attach
>> hotspot/jtreg/serviceability/dcmd/jvmti jdk/com/sun/tools/attach

Thanks.
I missed to add the -nativepath flag, sorry.

> Please check that:
>
> make test-image
>
> followed by jtreg
> -nativepath:<build-dir>/images/test/hotspot/jtreg/native
>
> also works.

It fails with the error:

   63 Running DCMD 'JVMTI.agent_load
/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so'
through 'PidJcmdExecutor'
   64 Executing command
'[/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/jdk/bin/jcmd,
28407, JVMTI.agent_load
/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg
/native/lib/libException.so]'
   65 Command returned with exit code 0
   66 ---------------- stdout ----------------
   67 28407:
   68
/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so
was not loaded.
   69
/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so:
cannot open shared object file: No such file or directory
   70


It seems, the '/lib' folder is added to the nativepath.

Yasumasa, could you, double check it please?

I'm using the jtreg:
   /java/re/jtreg/4.2/promoted/latest/binaries/jtreg/bin/jtreg

which is:

% ls -l /java/re/jtreg/4.2/promoted/latest
lrwxrwxrwx 1 uucp 143 7 Nov  6 21:49 /java/re/jtreg/4.2/promoted/latest
-> fcs/b10/


Thanks,
Serguei


>
> Thanks,
> David
>
>>
>>> Good news is that the attach-related tests from closed repository
>>> are passed.
>>
>> Thanks!
>>
>>
>> Yasumasa
>>
>>
>> On 2017/11/15 16:38, [hidden email] wrote:
>>> Hi Yasumasa,
>>>
>>> Do the new tests pass in your runs?
>>>
>>> In my runs 3 of 4 tests are failed with the errors like this:
>>>
>>>   109 Running DCMD 'JVMTI.agent_load
>>> /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so'
>>> through 'PidJcmdExecutor'
>>>   110 Executing command
>>> '[/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/jdk/bin/jcmd,
>>> 21951, JVMTI.agent_load
>>> /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so]'
>>>
>>>   111 Command returned with exit code 0
>>>   112 ---------------- stdout ----------------
>>>   113 21951:
>>>   114
>>> /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so
>>> was not loaded.
>>>   115
>>> /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so:
>>> cannot open shared object file: No such file or directory
>>>   116
>>>
>>>
>>> Good news is that the attach-related tests from closed repository
>>> are passed.
>>>
>>>
>>> Thanks,
>>> Serguei
>>>
>>>
>>>
>>> On 11/14/17 16:40, [hidden email] wrote:
>>>> Hi Yasumasa,
>>>>
>>>> It looks good to me.
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>>
>>>> On 11/7/17 22:38, Yasumasa Suenaga wrote:
>>>>> Hi Serguei,
>>>>>
>>>>> I uploaded new webrev:
>>>>>
>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.02/
>>>>>
>>>>>>>> I'd expect a check for some exception name, not for details
>>>>>>>> like: For
>>>>>>>> input string: "apa".
>>>>>>>
>>>>>>> Should we remove this comparison?
>>>>>>
>>>>>> I don't understand. Why do remove?
>>>>>> Would it better to check for the exception name instead?
>>>>> I've changed to check exception name (NumberFormatException) in
>>>>> StartManagementAgent.java.
>>>>>
>>>>>> I will sponsor this fix and run these tests before the push.
>>>>> Thanks!
>>>>> I'm waiting for second reviewer.
>>>>>
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>> 2017-11-08 11:55 GMT+09:00 [hidden email]
>>>>> <[hidden email]>:
>>>>>> On 11/6/17 04:31, Yasumasa Suenaga wrote:
>>>>>>> Hi Serguei,
>>>>>>>
>>>>>>> On 2017/11/06 20:06, [hidden email] wrote:
>>>>>>>> Hi Yasumasa,
>>>>>>>>
>>>>>>>> The changes looks good.
>>>>>>>> Thank you for making them!
>>>>>>>
>>>>>>> Thanks!
>>>>>>>
>>>>>>>
>>>>>>>> On 11/3/17 05:10, Yasumasa Suenaga wrote:
>>>>>>>>> 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.
>>>>>>>>
>>>>>>>>
>>>>>>>> I'd expect a check for some exception name, not for details
>>>>>>>> like: For
>>>>>>>> input string: "apa".
>>>>>>>
>>>>>>> Should we remove this comparison?
>>>>>>
>>>>>> I don't understand. Why do remove?
>>>>>> Would it better to check for the exception name instead?
>>>>>>
>>>>>>
>>>>>>>>>> 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
>>>>>>>>
>>>>>>>> There are more tests related to dynamic attach in closed,
>>>>>>>> nsk.aod.testlist and 30+ attach tests in nsk.jvmti.testlist.
>>>>>>>> I'm not sure, if they are included into any of the Mach5
>>>>>>>> testing levels.
>>>>>>>> Will need to check.
>>>>>>>> We have to make sure these tests are still passed.
>>>>>>>
>>>>>>> I cannot access JPRT and closed testcases because I'm not an Oracle
>>>>>>> employee.
>>>>>>> Can you run them with this change?
>>>>>>
>>>>>> Ok.
>>>>>> I will sponsor this fix and run these tests before the push.
>>>>>>
>>>>>> It seems, another update and one more review is needed.
>>>>>>
>>>>>> Thanks,
>>>>>> Serguei
>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Yasumasa
>>>>>>>
>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Serguei
>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> Yasumasa
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 2017/11/03 16:31, [hidden email] 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
>>>>>>>>>>>>
>>>>
>>>

Reply | Threaded
Open this post in threaded view
|

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

David Holmes
Hi Serguei,

 >
/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so'


There should not be any "/lib/" in that path

David

On 16/11/2017 4:34 AM, [hidden email] wrote:

> Hi Yasumasa and David,
>
>
> On 11/15/17 04:56, David Holmes wrote:
>> On 15/11/2017 10:15 PM, Yasumasa Suenaga wrote:
>>> Hi Serguei,
>>>
>>>> Do the new tests pass in your runs?
>>>
>>> Of course.
>>> It seems not to exist jtreg native libraries.
>>> I've tested as below:
>>>
>>>    $ make build-test-hotspot-jtreg-native
>>>    $ cd test
>>>    $ $JT_HOME/bin/jtreg -ignore:quiet
>>> -nativepath:<builddir>/<confdir>/support/test/hotspot/jtreg/native/lib hotspot/jtreg/serviceability/attach
>>> hotspot/jtreg/serviceability/dcmd/jvmti jdk/com/sun/tools/attach
>
> Thanks.
> I missed to add the -nativepath flag, sorry.
>
>> Please check that:
>>
>> make test-image
>>
>> followed by jtreg
>> -nativepath:<build-dir>/images/test/hotspot/jtreg/native
>>
>> also works.
>
> It fails with the error:
>
>    63 Running DCMD 'JVMTI.agent_load
> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so'
> through 'PidJcmdExecutor'
>    64 Executing command
> '[/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/jdk/bin/jcmd,
> 28407, JVMTI.agent_load
> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg
> /native/lib/libException.so]'
>    65 Command returned with exit code 0
>    66 ---------------- stdout ----------------
>    67 28407:
>    68
> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so
> was not loaded.
>    69
> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so:
> cannot open shared object file: No such file or directory
>    70
>
>
> It seems, the '/lib' folder is added to the nativepath.
>
> Yasumasa, could you, double check it please?
>
> I'm using the jtreg:
>    /java/re/jtreg/4.2/promoted/latest/binaries/jtreg/bin/jtreg
>
> which is:
>
> % ls -l /java/re/jtreg/4.2/promoted/latest
> lrwxrwxrwx 1 uucp 143 7 Nov  6 21:49 /java/re/jtreg/4.2/promoted/latest
> -> fcs/b10/
>
>
> Thanks,
> Serguei
>
>
>>
>> Thanks,
>> David
>>
>>>
>>>> Good news is that the attach-related tests from closed repository
>>>> are passed.
>>>
>>> Thanks!
>>>
>>>
>>> Yasumasa
>>>
>>>
>>> On 2017/11/15 16:38, [hidden email] wrote:
>>>> Hi Yasumasa,
>>>>
>>>> Do the new tests pass in your runs?
>>>>
>>>> In my runs 3 of 4 tests are failed with the errors like this:
>>>>
>>>>   109 Running DCMD 'JVMTI.agent_load
>>>> /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so'
>>>> through 'PidJcmdExecutor'
>>>>   110 Executing command
>>>> '[/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/jdk/bin/jcmd,
>>>> 21951, JVMTI.agent_load
>>>> /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so]'
>>>>
>>>>   111 Command returned with exit code 0
>>>>   112 ---------------- stdout ----------------
>>>>   113 21951:
>>>>   114
>>>> /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so
>>>> was not loaded.
>>>>   115
>>>> /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so:
>>>> cannot open shared object file: No such file or directory
>>>>   116
>>>>
>>>>
>>>> Good news is that the attach-related tests from closed repository
>>>> are passed.
>>>>
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>>
>>>>
>>>> On 11/14/17 16:40, [hidden email] wrote:
>>>>> Hi Yasumasa,
>>>>>
>>>>> It looks good to me.
>>>>>
>>>>> Thanks,
>>>>> Serguei
>>>>>
>>>>>
>>>>> On 11/7/17 22:38, Yasumasa Suenaga wrote:
>>>>>> Hi Serguei,
>>>>>>
>>>>>> I uploaded new webrev:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.02/
>>>>>>
>>>>>>>>> I'd expect a check for some exception name, not for details
>>>>>>>>> like: For
>>>>>>>>> input string: "apa".
>>>>>>>>
>>>>>>>> Should we remove this comparison?
>>>>>>>
>>>>>>> I don't understand. Why do remove?
>>>>>>> Would it better to check for the exception name instead?
>>>>>> I've changed to check exception name (NumberFormatException) in
>>>>>> StartManagementAgent.java.
>>>>>>
>>>>>>> I will sponsor this fix and run these tests before the push.
>>>>>> Thanks!
>>>>>> I'm waiting for second reviewer.
>>>>>>
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>> 2017-11-08 11:55 GMT+09:00 [hidden email]
>>>>>> <[hidden email]>:
>>>>>>> On 11/6/17 04:31, Yasumasa Suenaga wrote:
>>>>>>>> Hi Serguei,
>>>>>>>>
>>>>>>>> On 2017/11/06 20:06, [hidden email] wrote:
>>>>>>>>> Hi Yasumasa,
>>>>>>>>>
>>>>>>>>> The changes looks good.
>>>>>>>>> Thank you for making them!
>>>>>>>>
>>>>>>>> Thanks!
>>>>>>>>
>>>>>>>>
>>>>>>>>> On 11/3/17 05:10, Yasumasa Suenaga wrote:
>>>>>>>>>> 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.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I'd expect a check for some exception name, not for details
>>>>>>>>> like: For
>>>>>>>>> input string: "apa".
>>>>>>>>
>>>>>>>> Should we remove this comparison?
>>>>>>>
>>>>>>> I don't understand. Why do remove?
>>>>>>> Would it better to check for the exception name instead?
>>>>>>>
>>>>>>>
>>>>>>>>>>> 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
>>>>>>>>>
>>>>>>>>> There are more tests related to dynamic attach in closed,
>>>>>>>>> nsk.aod.testlist and 30+ attach tests in nsk.jvmti.testlist.
>>>>>>>>> I'm not sure, if they are included into any of the Mach5
>>>>>>>>> testing levels.
>>>>>>>>> Will need to check.
>>>>>>>>> We have to make sure these tests are still passed.
>>>>>>>>
>>>>>>>> I cannot access JPRT and closed testcases because I'm not an Oracle
>>>>>>>> employee.
>>>>>>>> Can you run them with this change?
>>>>>>>
>>>>>>> Ok.
>>>>>>> I will sponsor this fix and run these tests before the push.
>>>>>>>
>>>>>>> It seems, another update and one more review is needed.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Serguei
>>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> Yasumasa
>>>>>>>>
>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Serguei
>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>>
>>>>>>>>>> Yasumasa
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 2017/11/03 16:31, [hidden email] 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
>>>>>>>>>>>>>
>>>>>
>>>>
>
Reply | Threaded
Open this post in threaded view
|

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

serguei.spitsyn@oracle.com
On 11/15/17 18:11, David Holmes wrote:
Hi Serguei,

> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so'

There should not be any "/lib/" in that path

Right, it should not be.

This is the script I'm using to run the tests:

#!/bin/sh

REPO=/var/tmp/sspitsyn/jdk.attach
IMAGES=${REPO}/build/linux-x86_64-normal-server-release/images
export JAVA_HOME=${IMAGES}/jdk
export NATIVE_PATH=${IMAGES}/../support/test/hotspot/jtreg/native
export NATIVE_PATH=${IMAGES}/test/hotspot/jtreg/native
echo "JAVA_HOME = $JAVA_HOME"

/java/re/jtreg/4.2/nightly/binaries/jtreg/bin/jtreg  -nativepath:${NATIVE_PATH} \
  $REPO/open/test/hotspot/jtreg/serviceability/dcmd/jvmti/AttachFailed


This is a part of log with the reported error from the AttachException.jtr:

[TestNG] Running:
  serviceability/dcmd/jvmti/AttachFailed/AttachException.java

Running DCMD 'JVMTI.agent_load /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so' through 'PidJcmdExecutor'
Executing command '[/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/jdk/bin/jcmd, 8689, JVMTI.agent_load /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so]'
Command returned with exit code 0
---------------- stdout ----------------
8689:
/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so was not loaded.
/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so: cannot open shared object file: No such file or directory


These are the locations of the libException.so:
  build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/libException.so
  build/linux-x86_64-normal-server-release/support/test/hotspot/jtreg/native/lib/libException.so


The tests fail with the "NATIVE_PATH=${IMAGES}/test/hotspot/jtreg/native"
but pass with the "export NATIVE_PATH=${IMAGES}/../support/test/hotspot/jtreg/native".


When the "export NATIVE_PATH=${IMAGES}/../support/test/hotspot/jtreg/native" is used
the log has this line:

Running DCMD 'JVMTI.agent_load /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/../support/test/hotspot/jtreg/native/lib/libException.so' through 'JMXExecutor'


Apparently, the sub-directory name "/lib" is added to the path.


Thanks,
Serguei


David

On 16/11/2017 4:34 AM, [hidden email] wrote:
Hi Yasumasa and David,


On 11/15/17 04:56, David Holmes wrote:
On 15/11/2017 10:15 PM, Yasumasa Suenaga wrote:
Hi Serguei,

Do the new tests pass in your runs?

Of course.
It seems not to exist jtreg native libraries.
I've tested as below:

   $ make build-test-hotspot-jtreg-native
   $ cd test
   $ $JT_HOME/bin/jtreg -ignore:quiet -nativepath:<builddir>/<confdir>/support/test/hotspot/jtreg/native/lib hotspot/jtreg/serviceability/attach hotspot/jtreg/serviceability/dcmd/jvmti jdk/com/sun/tools/attach

Thanks.
I missed to add the -nativepath flag, sorry.

Please check that:

make test-image

followed by jtreg -nativepath:<build-dir>/images/test/hotspot/jtreg/native

also works.

It fails with the error:

   63 Running DCMD 'JVMTI.agent_load /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so' through 'PidJcmdExecutor'
   64 Executing command '[/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/jdk/bin/jcmd, 28407, JVMTI.agent_load /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg /native/lib/libException.so]'
   65 Command returned with exit code 0
   66 ---------------- stdout ----------------
   67 28407:
   68 /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so was not loaded.
   69 /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so: cannot open shared object file: No such file or directory
   70


It seems, the '/lib' folder is added to the nativepath.

Yasumasa, could you, double check it please?

I'm using the jtreg:
   /java/re/jtreg/4.2/promoted/latest/binaries/jtreg/bin/jtreg

which is:

% ls -l /java/re/jtreg/4.2/promoted/latest
lrwxrwxrwx 1 uucp 143 7 Nov  6 21:49 /java/re/jtreg/4.2/promoted/latest -> fcs/b10/


Thanks,
Serguei



Thanks,
David


Good news is that the attach-related tests from closed repository are passed.

Thanks!


Yasumasa


On 2017/11/15 16:38, [hidden email] wrote:
Hi Yasumasa,

Do the new tests pass in your runs?

In my runs 3 of 4 tests are failed with the errors like this:

  109 Running DCMD 'JVMTI.agent_load /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so' through 'PidJcmdExecutor'
  110 Executing command '[/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/jdk/bin/jcmd, 21951, JVMTI.agent_load /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so]'
  111 Command returned with exit code 0
  112 ---------------- stdout ----------------
  113 21951:
  114 /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so was not loaded.
  115 /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so: cannot open shared object file: No such file or directory
  116


Good news is that the attach-related tests from closed repository are passed.


Thanks,
Serguei



On 11/14/17 16:40, [hidden email] wrote:
Hi Yasumasa,

It looks good to me.

Thanks,
Serguei


On 11/7/17 22:38, Yasumasa Suenaga wrote:
Hi Serguei,

I uploaded new webrev:

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

I'd expect a check for some exception name, not for details like: For
input string: "apa".

Should we remove this comparison?

I don't understand. Why do remove?
Would it better to check for the exception name instead?
I've changed to check exception name (NumberFormatException) in
StartManagementAgent.java.

I will sponsor this fix and run these tests before the push.
Thanks!
I'm waiting for second reviewer.


Yasumasa


2017-11-08 11:55 GMT+09:00 [hidden email]
[hidden email]:
On 11/6/17 04:31, Yasumasa Suenaga wrote:
Hi Serguei,

On 2017/11/06 20:06, [hidden email] wrote:
Hi Yasumasa,

The changes looks good.
Thank you for making them!

Thanks!


On 11/3/17 05:10, Yasumasa Suenaga wrote:
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.


I'd expect a check for some exception name, not for details like: For
input string: "apa".

Should we remove this comparison?

I don't understand. Why do remove?
Would it better to check for the exception name instead?


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

There are more tests related to dynamic attach in closed,
nsk.aod.testlist and 30+ attach tests in nsk.jvmti.testlist.
I'm not sure, if they are included into any of the Mach5 testing levels.
Will need to check.
We have to make sure these tests are still passed.

I cannot access JPRT and closed testcases because I'm not an Oracle
employee.
Can you run them with this change?

Ok.
I will sponsor this fix and run these tests before the push.

It seems, another update and one more review is needed.

Thanks,
Serguei


Thanks,

Yasumasa


Thanks,
Serguei

Thanks,

Yasumasa


On 2017/11/03 16:31, [hidden email] 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





Reply | Threaded
Open this post in threaded view
|

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

David Holmes
On 16/11/2017 4:43 PM, [hidden email] wrote:

> On 11/15/17 18:11, David Holmes wrote:
>> Hi Serguei,
>>
>> >
>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so'
>>
>>
>> There should not be any "/lib/" in that path
>
> Right, it should not be.

The test logic is adding it in AttachFailedTestBase.java:

   45         return Paths.get(System.getProperty("test.nativepath"),
"lib", libname)
   46                     .toAbsolutePath()
   47                     .toString();

but it shouldn't.

David
-----


> This is the script I'm using to run the tests:
>
> #!/bin/sh
>
> REPO=/var/tmp/sspitsyn/jdk.attach
> IMAGES=${REPO}/build/linux-x86_64-normal-server-release/images
> export JAVA_HOME=${IMAGES}/jdk
> export NATIVE_PATH=${IMAGES}/../support/test/hotspot/jtreg/native
> export NATIVE_PATH=${IMAGES}/test/hotspot/jtreg/native
> echo "JAVA_HOME = $JAVA_HOME"
>
> /java/re/jtreg/4.2/nightly/binaries/jtreg/bin/jtreg
> -nativepath:${NATIVE_PATH} \
> $REPO/open/test/hotspot/jtreg/serviceability/dcmd/jvmti/AttachFailed
>
>
> This is a part of log with the reported error from the AttachException.jtr:
>
> [TestNG] Running:
>    serviceability/dcmd/jvmti/AttachFailed/AttachException.java
>
> Running DCMD 'JVMTI.agent_load
> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so'
> through 'PidJcmdExecutor'
> Executing command
> '[/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/jdk/bin/jcmd,
> 8689, JVMTI.agent_load
> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so]'
> Command returned with exit code 0
> ---------------- stdout ----------------
> 8689:
> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native*/lib*/libException.so
> was not loaded.
> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native*/lib*/libException.so:
> cannot open shared object file: No such file or directory
>
>
> These are the locations of the libException.so:
> build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/libException.so
> build/linux-x86_64-normal-server-release/support/test/hotspot/jtreg/native/lib/libException.so
>
>
> The tests fail with the "NATIVE_PATH=${IMAGES}/test/hotspot/jtreg/native"
> but pass with the "export
> NATIVE_PATH=${IMAGES}/../support/test/hotspot/jtreg/native".
>
>
> When the "export
> NATIVE_PATH=${IMAGES}/../support/test/hotspot/jtreg/native" is used
> the log has this line:
>
> Running DCMD 'JVMTI.agent_load
> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/../support/test/hotspot/jtreg/native*/lib*/libException.so'
> through 'JMXExecutor'
>
>
> Apparently, the sub-directory name "/lib" is added to the path.
>
>
> Thanks,
> Serguei
>
>
>> David
>>
>> On 16/11/2017 4:34 AM, [hidden email] wrote:
>>> Hi Yasumasa and David,
>>>
>>>
>>> On 11/15/17 04:56, David Holmes wrote:
>>>> On 15/11/2017 10:15 PM, Yasumasa Suenaga wrote:
>>>>> Hi Serguei,
>>>>>
>>>>>> Do the new tests pass in your runs?
>>>>>
>>>>> Of course.
>>>>> It seems not to exist jtreg native libraries.
>>>>> I've tested as below:
>>>>>
>>>>>    $ make build-test-hotspot-jtreg-native
>>>>>    $ cd test
>>>>>    $ $JT_HOME/bin/jtreg -ignore:quiet
>>>>> -nativepath:<builddir>/<confdir>/support/test/hotspot/jtreg/native/lib
>>>>> hotspot/jtreg/serviceability/attach
>>>>> hotspot/jtreg/serviceability/dcmd/jvmti jdk/com/sun/tools/attach
>>>
>>> Thanks.
>>> I missed to add the -nativepath flag, sorry.
>>>
>>>> Please check that:
>>>>
>>>> make test-image
>>>>
>>>> followed by jtreg
>>>> -nativepath:<build-dir>/images/test/hotspot/jtreg/native
>>>>
>>>> also works.
>>>
>>> It fails with the error:
>>>
>>>    63 Running DCMD 'JVMTI.agent_load
>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so'
>>> through 'PidJcmdExecutor'
>>>    64 Executing command
>>> '[/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/jdk/bin/jcmd,
>>> 28407, JVMTI.agent_load
>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg
>>> /native/lib/libException.so]'
>>>    65 Command returned with exit code 0
>>>    66 ---------------- stdout ----------------
>>>    67 28407:
>>>    68
>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so
>>> was not loaded.
>>>    69
>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so:
>>> cannot open shared object file: No such file or directory
>>>    70
>>>
>>>
>>> It seems, the '/lib' folder is added to the nativepath.
>>>
>>> Yasumasa, could you, double check it please?
>>>
>>> I'm using the jtreg:
>>>    /java/re/jtreg/4.2/promoted/latest/binaries/jtreg/bin/jtreg
>>>
>>> which is:
>>>
>>> % ls -l /java/re/jtreg/4.2/promoted/latest
>>> lrwxrwxrwx 1 uucp 143 7 Nov  6 21:49
>>> /java/re/jtreg/4.2/promoted/latest -> fcs/b10/
>>>
>>>
>>> Thanks,
>>> Serguei
>>>
>>>
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>>
>>>>>> Good news is that the attach-related tests from closed repository
>>>>>> are passed.
>>>>>
>>>>> Thanks!
>>>>>
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>> On 2017/11/15 16:38, [hidden email] wrote:
>>>>>> Hi Yasumasa,
>>>>>>
>>>>>> Do the new tests pass in your runs?
>>>>>>
>>>>>> In my runs 3 of 4 tests are failed with the errors like this:
>>>>>>
>>>>>>   109 Running DCMD 'JVMTI.agent_load
>>>>>> /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so'
>>>>>> through 'PidJcmdExecutor'
>>>>>>   110 Executing command
>>>>>> '[/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/jdk/bin/jcmd,
>>>>>> 21951, JVMTI.agent_load
>>>>>> /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so]'
>>>>>>
>>>>>>   111 Command returned with exit code 0
>>>>>>   112 ---------------- stdout ----------------
>>>>>>   113 21951:
>>>>>>   114
>>>>>> /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so
>>>>>> was not loaded.
>>>>>>   115
>>>>>> /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so:
>>>>>> cannot open shared object file: No such file or directory
>>>>>>   116
>>>>>>
>>>>>>
>>>>>> Good news is that the attach-related tests from closed repository
>>>>>> are passed.
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Serguei
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 11/14/17 16:40, [hidden email] wrote:
>>>>>>> Hi Yasumasa,
>>>>>>>
>>>>>>> It looks good to me.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Serguei
>>>>>>>
>>>>>>>
>>>>>>> On 11/7/17 22:38, Yasumasa Suenaga wrote:
>>>>>>>> Hi Serguei,
>>>>>>>>
>>>>>>>> I uploaded new webrev:
>>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.02/
>>>>>>>>
>>>>>>>>>>> I'd expect a check for some exception name, not for details
>>>>>>>>>>> like: For
>>>>>>>>>>> input string: "apa".
>>>>>>>>>>
>>>>>>>>>> Should we remove this comparison?
>>>>>>>>>
>>>>>>>>> I don't understand. Why do remove?
>>>>>>>>> Would it better to check for the exception name instead?
>>>>>>>> I've changed to check exception name (NumberFormatException) in
>>>>>>>> StartManagementAgent.java.
>>>>>>>>
>>>>>>>>> I will sponsor this fix and run these tests before the push.
>>>>>>>> Thanks!
>>>>>>>> I'm waiting for second reviewer.
>>>>>>>>
>>>>>>>>
>>>>>>>> Yasumasa
>>>>>>>>
>>>>>>>>
>>>>>>>> 2017-11-08 11:55 GMT+09:00 [hidden email]
>>>>>>>> <[hidden email]>:
>>>>>>>>> On 11/6/17 04:31, Yasumasa Suenaga wrote:
>>>>>>>>>> Hi Serguei,
>>>>>>>>>>
>>>>>>>>>> On 2017/11/06 20:06, [hidden email] wrote:
>>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>>
>>>>>>>>>>> The changes looks good.
>>>>>>>>>>> Thank you for making them!
>>>>>>>>>>
>>>>>>>>>> Thanks!
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> On 11/3/17 05:10, Yasumasa Suenaga wrote:
>>>>>>>>>>>> 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.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> I'd expect a check for some exception name, not for details
>>>>>>>>>>> like: For
>>>>>>>>>>> input string: "apa".
>>>>>>>>>>
>>>>>>>>>> Should we remove this comparison?
>>>>>>>>>
>>>>>>>>> I don't understand. Why do remove?
>>>>>>>>> Would it better to check for the exception name instead?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>>>>> 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
>>>>>>>>>>>
>>>>>>>>>>> There are more tests related to dynamic attach in closed,
>>>>>>>>>>> nsk.aod.testlist and 30+ attach tests in nsk.jvmti.testlist.
>>>>>>>>>>> I'm not sure, if they are included into any of the Mach5
>>>>>>>>>>> testing levels.
>>>>>>>>>>> Will need to check.
>>>>>>>>>>> We have to make sure these tests are still passed.
>>>>>>>>>>
>>>>>>>>>> I cannot access JPRT and closed testcases because I'm not an
>>>>>>>>>> Oracle
>>>>>>>>>> employee.
>>>>>>>>>> Can you run them with this change?
>>>>>>>>>
>>>>>>>>> Ok.
>>>>>>>>> I will sponsor this fix and run these tests before the push.
>>>>>>>>>
>>>>>>>>> It seems, another update and one more review is needed.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Serguei
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>>
>>>>>>>>>> Yasumasa
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> Serguei
>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>
>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On 2017/11/03 16:31, [hidden email] 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
>>>>>>>>>>>>>>>
>>>>>>>
>>>>>>
>>>
>
Reply | Threaded
Open this post in threaded view
|

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

serguei.spitsyn@oracle.com
On 11/15/17 23:29, David Holmes wrote:

> On 16/11/2017 4:43 PM, [hidden email] wrote:
>> On 11/15/17 18:11, David Holmes wrote:
>>> Hi Serguei,
>>>
>>> >
>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so'
>>>
>>>
>>> There should not be any "/lib/" in that path
>>
>> Right, it should not be.
>
> The test logic is adding it in AttachFailedTestBase.java:
>
>   45         return Paths.get(System.getProperty("test.nativepath"),
> "lib", libname)
>   46                     .toAbsolutePath()
>   47                     .toString();
>
> but it shouldn't.

Nice catch!
I looked right to these lines and overlooked it. :)

Thanks,
Serguei

>
> David
> -----
>
>
>> This is the script I'm using to run the tests:
>>
>> #!/bin/sh
>>
>> REPO=/var/tmp/sspitsyn/jdk.attach
>> IMAGES=${REPO}/build/linux-x86_64-normal-server-release/images
>> export JAVA_HOME=${IMAGES}/jdk
>> export NATIVE_PATH=${IMAGES}/../support/test/hotspot/jtreg/native
>> export NATIVE_PATH=${IMAGES}/test/hotspot/jtreg/native
>> echo "JAVA_HOME = $JAVA_HOME"
>>
>> /java/re/jtreg/4.2/nightly/binaries/jtreg/bin/jtreg
>> -nativepath:${NATIVE_PATH} \
>> $REPO/open/test/hotspot/jtreg/serviceability/dcmd/jvmti/AttachFailed
>>
>>
>> This is a part of log with the reported error from the
>> AttachException.jtr:
>>
>> [TestNG] Running:
>>    serviceability/dcmd/jvmti/AttachFailed/AttachException.java
>>
>> Running DCMD 'JVMTI.agent_load
>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so'
>> through 'PidJcmdExecutor'
>> Executing command
>> '[/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/jdk/bin/jcmd,
>> 8689, JVMTI.agent_load
>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so]'
>> Command returned with exit code 0
>> ---------------- stdout ----------------
>> 8689:
>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native*/lib*/libException.so
>> was not loaded.
>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native*/lib*/libException.so:
>> cannot open shared object file: No such file or directory
>>
>>
>> These are the locations of the libException.so:
>> build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/libException.so
>>
>> build/linux-x86_64-normal-server-release/support/test/hotspot/jtreg/native/lib/libException.so
>>
>>
>>
>> The tests fail with the
>> "NATIVE_PATH=${IMAGES}/test/hotspot/jtreg/native"
>> but pass with the "export
>> NATIVE_PATH=${IMAGES}/../support/test/hotspot/jtreg/native".
>>
>>
>> When the "export
>> NATIVE_PATH=${IMAGES}/../support/test/hotspot/jtreg/native" is used
>> the log has this line:
>>
>> Running DCMD 'JVMTI.agent_load
>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/../support/test/hotspot/jtreg/native*/lib*/libException.so'
>> through 'JMXExecutor'
>>
>>
>> Apparently, the sub-directory name "/lib" is added to the path.
>>
>>
>> Thanks,
>> Serguei
>>
>>
>>> David
>>>
>>> On 16/11/2017 4:34 AM, [hidden email] wrote:
>>>> Hi Yasumasa and David,
>>>>
>>>>
>>>> On 11/15/17 04:56, David Holmes wrote:
>>>>> On 15/11/2017 10:15 PM, Yasumasa Suenaga wrote:
>>>>>> Hi Serguei,
>>>>>>
>>>>>>> Do the new tests pass in your runs?
>>>>>>
>>>>>> Of course.
>>>>>> It seems not to exist jtreg native libraries.
>>>>>> I've tested as below:
>>>>>>
>>>>>>    $ make build-test-hotspot-jtreg-native
>>>>>>    $ cd test
>>>>>>    $ $JT_HOME/bin/jtreg -ignore:quiet
>>>>>> -nativepath:<builddir>/<confdir>/support/test/hotspot/jtreg/native/lib
>>>>>> hotspot/jtreg/serviceability/attach
>>>>>> hotspot/jtreg/serviceability/dcmd/jvmti jdk/com/sun/tools/attach
>>>>
>>>> Thanks.
>>>> I missed to add the -nativepath flag, sorry.
>>>>
>>>>> Please check that:
>>>>>
>>>>> make test-image
>>>>>
>>>>> followed by jtreg
>>>>> -nativepath:<build-dir>/images/test/hotspot/jtreg/native
>>>>>
>>>>> also works.
>>>>
>>>> It fails with the error:
>>>>
>>>>    63 Running DCMD 'JVMTI.agent_load
>>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so'
>>>> through 'PidJcmdExecutor'
>>>>    64 Executing command
>>>> '[/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/jdk/bin/jcmd,
>>>> 28407, JVMTI.agent_load
>>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg
>>>> /native/lib/libException.so]'
>>>>    65 Command returned with exit code 0
>>>>    66 ---------------- stdout ----------------
>>>>    67 28407:
>>>>    68
>>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so
>>>> was not loaded.
>>>>    69
>>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so:
>>>> cannot open shared object file: No such file or directory
>>>>    70
>>>>
>>>>
>>>> It seems, the '/lib' folder is added to the nativepath.
>>>>
>>>> Yasumasa, could you, double check it please?
>>>>
>>>> I'm using the jtreg:
>>>> /java/re/jtreg/4.2/promoted/latest/binaries/jtreg/bin/jtreg
>>>>
>>>> which is:
>>>>
>>>> % ls -l /java/re/jtreg/4.2/promoted/latest
>>>> lrwxrwxrwx 1 uucp 143 7 Nov  6 21:49
>>>> /java/re/jtreg/4.2/promoted/latest -> fcs/b10/
>>>>
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>>
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>>
>>>>>>> Good news is that the attach-related tests from closed
>>>>>>> repository are passed.
>>>>>>
>>>>>> Thanks!
>>>>>>
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>> On 2017/11/15 16:38, [hidden email] wrote:
>>>>>>> Hi Yasumasa,
>>>>>>>
>>>>>>> Do the new tests pass in your runs?
>>>>>>>
>>>>>>> In my runs 3 of 4 tests are failed with the errors like this:
>>>>>>>
>>>>>>>   109 Running DCMD 'JVMTI.agent_load
>>>>>>> /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so'
>>>>>>> through 'PidJcmdExecutor'
>>>>>>>   110 Executing command
>>>>>>> '[/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/jdk/bin/jcmd,
>>>>>>> 21951, JVMTI.agent_load
>>>>>>> /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so]'
>>>>>>>
>>>>>>>   111 Command returned with exit code 0
>>>>>>>   112 ---------------- stdout ----------------
>>>>>>>   113 21951:
>>>>>>>   114
>>>>>>> /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so
>>>>>>> was not loaded.
>>>>>>>   115
>>>>>>> /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so:
>>>>>>> cannot open shared object file: No such file or directory
>>>>>>>   116
>>>>>>>
>>>>>>>
>>>>>>> Good news is that the attach-related tests from closed
>>>>>>> repository are passed.
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Serguei
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 11/14/17 16:40, [hidden email] wrote:
>>>>>>>> Hi Yasumasa,
>>>>>>>>
>>>>>>>> It looks good to me.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Serguei
>>>>>>>>
>>>>>>>>
>>>>>>>> On 11/7/17 22:38, Yasumasa Suenaga wrote:
>>>>>>>>> Hi Serguei,
>>>>>>>>>
>>>>>>>>> I uploaded new webrev:
>>>>>>>>>
>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.02/
>>>>>>>>>
>>>>>>>>>>>> I'd expect a check for some exception name, not for details
>>>>>>>>>>>> like: For
>>>>>>>>>>>> input string: "apa".
>>>>>>>>>>>
>>>>>>>>>>> Should we remove this comparison?
>>>>>>>>>>
>>>>>>>>>> I don't understand. Why do remove?
>>>>>>>>>> Would it better to check for the exception name instead?
>>>>>>>>> I've changed to check exception name (NumberFormatException) in
>>>>>>>>> StartManagementAgent.java.
>>>>>>>>>
>>>>>>>>>> I will sponsor this fix and run these tests before the push.
>>>>>>>>> Thanks!
>>>>>>>>> I'm waiting for second reviewer.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Yasumasa
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> 2017-11-08 11:55 GMT+09:00 [hidden email]
>>>>>>>>> <[hidden email]>:
>>>>>>>>>> On 11/6/17 04:31, Yasumasa Suenaga wrote:
>>>>>>>>>>> Hi Serguei,
>>>>>>>>>>>
>>>>>>>>>>> On 2017/11/06 20:06, [hidden email] wrote:
>>>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>>>
>>>>>>>>>>>> The changes looks good.
>>>>>>>>>>>> Thank you for making them!
>>>>>>>>>>>
>>>>>>>>>>> Thanks!
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>> On 11/3/17 05:10, Yasumasa Suenaga wrote:
>>>>>>>>>>>>> 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.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> I'd expect a check for some exception name, not for details
>>>>>>>>>>>> like: For
>>>>>>>>>>>> input string: "apa".
>>>>>>>>>>>
>>>>>>>>>>> Should we remove this comparison?
>>>>>>>>>>
>>>>>>>>>> I don't understand. Why do remove?
>>>>>>>>>> Would it better to check for the exception name instead?
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>>>>> 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
>>>>>>>>>>>>
>>>>>>>>>>>> There are more tests related to dynamic attach in closed,
>>>>>>>>>>>> nsk.aod.testlist and 30+ attach tests in nsk.jvmti.testlist.
>>>>>>>>>>>> I'm not sure, if they are included into any of the Mach5
>>>>>>>>>>>> testing levels.
>>>>>>>>>>>> Will need to check.
>>>>>>>>>>>> We have to make sure these tests are still passed.
>>>>>>>>>>>
>>>>>>>>>>> I cannot access JPRT and closed testcases because I'm not an
>>>>>>>>>>> Oracle
>>>>>>>>>>> employee.
>>>>>>>>>>> Can you run them with this change?
>>>>>>>>>>
>>>>>>>>>> Ok.
>>>>>>>>>> I will sponsor this fix and run these tests before the push.
>>>>>>>>>>
>>>>>>>>>> It seems, another update and one more review is needed.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Serguei
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>>
>>>>>>>>>>> Yasumasa
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> Serguei
>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 2017/11/03 16:31, [hidden email] 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
>>>>>>>>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>
>>

Reply | Threaded
Open this post in threaded view
|

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

Yasumasa Suenaga-4
Hi David, Serguei,

>> The test logic is adding it in AttachFailedTestBase.java:
>>
>>   45         return Paths.get(System.getProperty("test.nativepath"), "lib", libname)
>>   46                     .toAbsolutePath()
>>   47                     .toString();

Thanks!
I've fixed it in new webrev:

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


I've tested it as below. It works fine:

$ $JT_HOME/bin/jtreg -ignore:quiet -nativepath:$NATIVE_PATH hotspot/jtreg/serviceability/dcmd/jvmti/AttachFailed
$ echo $NATIVE_PATH
/<Path to configuration>/images/test/hotspot/jtreg/native


Thanks,

Yasumasa


On 2017/11/16 16:49, [hidden email] wrote:

> On 11/15/17 23:29, David Holmes wrote:
>> On 16/11/2017 4:43 PM, [hidden email] wrote:
>>> On 11/15/17 18:11, David Holmes wrote:
>>>> Hi Serguei,
>>>>
>>>> > /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so'
>>>>
>>>> There should not be any "/lib/" in that path
>>>
>>> Right, it should not be.
>>
>> The test logic is adding it in AttachFailedTestBase.java:
>>
>>   45         return Paths.get(System.getProperty("test.nativepath"), "lib", libname)
>>   46                     .toAbsolutePath()
>>   47                     .toString();
>>
>> but it shouldn't.
>
> Nice catch!
> I looked right to these lines and overlooked it. :)
>
> Thanks,
> Serguei
>
>>
>> David
>> -----
>>
>>
>>> This is the script I'm using to run the tests:
>>>
>>> #!/bin/sh
>>>
>>> REPO=/var/tmp/sspitsyn/jdk.attach
>>> IMAGES=${REPO}/build/linux-x86_64-normal-server-release/images
>>> export JAVA_HOME=${IMAGES}/jdk
>>> export NATIVE_PATH=${IMAGES}/../support/test/hotspot/jtreg/native
>>> export NATIVE_PATH=${IMAGES}/test/hotspot/jtreg/native
>>> echo "JAVA_HOME = $JAVA_HOME"
>>>
>>> /java/re/jtreg/4.2/nightly/binaries/jtreg/bin/jtreg -nativepath:${NATIVE_PATH} \
>>> $REPO/open/test/hotspot/jtreg/serviceability/dcmd/jvmti/AttachFailed
>>>
>>>
>>> This is a part of log with the reported error from the AttachException.jtr:
>>>
>>> [TestNG] Running:
>>>    serviceability/dcmd/jvmti/AttachFailed/AttachException.java
>>>
>>> Running DCMD 'JVMTI.agent_load /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so' through 'PidJcmdExecutor'
>>> Executing command '[/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/jdk/bin/jcmd, 8689, JVMTI.agent_load /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so]'
>>> Command returned with exit code 0
>>> ---------------- stdout ----------------
>>> 8689:
>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native*/lib*/libException.so was not loaded.
>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native*/lib*/libException.so: cannot open shared object file: No such file or directory
>>>
>>>
>>> These are the locations of the libException.so:
>>> build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/libException.so
>>> build/linux-x86_64-normal-server-release/support/test/hotspot/jtreg/native/lib/libException.so
>>>
>>>
>>> The tests fail with the "NATIVE_PATH=${IMAGES}/test/hotspot/jtreg/native"
>>> but pass with the "export NATIVE_PATH=${IMAGES}/../support/test/hotspot/jtreg/native".
>>>
>>>
>>> When the "export NATIVE_PATH=${IMAGES}/../support/test/hotspot/jtreg/native" is used
>>> the log has this line:
>>>
>>> Running DCMD 'JVMTI.agent_load /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/../support/test/hotspot/jtreg/native*/lib*/libException.so' through 'JMXExecutor'
>>>
>>>
>>> Apparently, the sub-directory name "/lib" is added to the path.
>>>
>>>
>>> Thanks,
>>> Serguei
>>>
>>>
>>>> David
>>>>
>>>> On 16/11/2017 4:34 AM, [hidden email] wrote:
>>>>> Hi Yasumasa and David,
>>>>>
>>>>>
>>>>> On 11/15/17 04:56, David Holmes wrote:
>>>>>> On 15/11/2017 10:15 PM, Yasumasa Suenaga wrote:
>>>>>>> Hi Serguei,
>>>>>>>
>>>>>>>> Do the new tests pass in your runs?
>>>>>>>
>>>>>>> Of course.
>>>>>>> It seems not to exist jtreg native libraries.
>>>>>>> I've tested as below:
>>>>>>>
>>>>>>>    $ make build-test-hotspot-jtreg-native
>>>>>>>    $ cd test
>>>>>>>    $ $JT_HOME/bin/jtreg -ignore:quiet -nativepath:<builddir>/<confdir>/support/test/hotspot/jtreg/native/lib hotspot/jtreg/serviceability/attach hotspot/jtreg/serviceability/dcmd/jvmti jdk/com/sun/tools/attach
>>>>>
>>>>> Thanks.
>>>>> I missed to add the -nativepath flag, sorry.
>>>>>
>>>>>> Please check that:
>>>>>>
>>>>>> make test-image
>>>>>>
>>>>>> followed by jtreg -nativepath:<build-dir>/images/test/hotspot/jtreg/native
>>>>>>
>>>>>> also works.
>>>>>
>>>>> It fails with the error:
>>>>>
>>>>>    63 Running DCMD 'JVMTI.agent_load /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so' through 'PidJcmdExecutor'
>>>>>    64 Executing command '[/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/jdk/bin/jcmd, 28407, JVMTI.agent_load /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg /native/lib/libException.so]'
>>>>>    65 Command returned with exit code 0
>>>>>    66 ---------------- stdout ----------------
>>>>>    67 28407:
>>>>>    68 /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so was not loaded.
>>>>>    69 /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so: cannot open shared object file: No such file or directory
>>>>>    70
>>>>>
>>>>>
>>>>> It seems, the '/lib' folder is added to the nativepath.
>>>>>
>>>>> Yasumasa, could you, double check it please?
>>>>>
>>>>> I'm using the jtreg:
>>>>> /java/re/jtreg/4.2/promoted/latest/binaries/jtreg/bin/jtreg
>>>>>
>>>>> which is:
>>>>>
>>>>> % ls -l /java/re/jtreg/4.2/promoted/latest
>>>>> lrwxrwxrwx 1 uucp 143 7 Nov  6 21:49 /java/re/jtreg/4.2/promoted/latest -> fcs/b10/
>>>>>
>>>>>
>>>>> Thanks,
>>>>> Serguei
>>>>>
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>>
>>>>>>>
>>>>>>>> Good news is that the attach-related tests from closed repository are passed.
>>>>>>>
>>>>>>> Thanks!
>>>>>>>
>>>>>>>
>>>>>>> Yasumasa
>>>>>>>
>>>>>>>
>>>>>>> On 2017/11/15 16:38, [hidden email] wrote:
>>>>>>>> Hi Yasumasa,
>>>>>>>>
>>>>>>>> Do the new tests pass in your runs?
>>>>>>>>
>>>>>>>> In my runs 3 of 4 tests are failed with the errors like this:
>>>>>>>>
>>>>>>>>   109 Running DCMD 'JVMTI.agent_load /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so' through 'PidJcmdExecutor'
>>>>>>>>   110 Executing command '[/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/jdk/bin/jcmd, 21951, JVMTI.agent_load /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so]'
>>>>>>>>   111 Command returned with exit code 0
>>>>>>>>   112 ---------------- stdout ----------------
>>>>>>>>   113 21951:
>>>>>>>>   114 /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so was not loaded.
>>>>>>>>   115 /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so: cannot open shared object file: No such file or directory
>>>>>>>>   116
>>>>>>>>
>>>>>>>>
>>>>>>>> Good news is that the attach-related tests from closed repository are passed.
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Serguei
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 11/14/17 16:40, [hidden email] wrote:
>>>>>>>>> Hi Yasumasa,
>>>>>>>>>
>>>>>>>>> It looks good to me.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Serguei
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 11/7/17 22:38, Yasumasa Suenaga wrote:
>>>>>>>>>> Hi Serguei,
>>>>>>>>>>
>>>>>>>>>> I uploaded new webrev:
>>>>>>>>>>
>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.02/
>>>>>>>>>>
>>>>>>>>>>>>> I'd expect a check for some exception name, not for details like: For
>>>>>>>>>>>>> input string: "apa".
>>>>>>>>>>>>
>>>>>>>>>>>> Should we remove this comparison?
>>>>>>>>>>>
>>>>>>>>>>> I don't understand. Why do remove?
>>>>>>>>>>> Would it better to check for the exception name instead?
>>>>>>>>>> I've changed to check exception name (NumberFormatException) in
>>>>>>>>>> StartManagementAgent.java.
>>>>>>>>>>
>>>>>>>>>>> I will sponsor this fix and run these tests before the push.
>>>>>>>>>> Thanks!
>>>>>>>>>> I'm waiting for second reviewer.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Yasumasa
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> 2017-11-08 11:55 GMT+09:00 [hidden email]
>>>>>>>>>> <[hidden email]>:
>>>>>>>>>>> On 11/6/17 04:31, Yasumasa Suenaga wrote:
>>>>>>>>>>>> Hi Serguei,
>>>>>>>>>>>>
>>>>>>>>>>>> On 2017/11/06 20:06, [hidden email] wrote:
>>>>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>>>>
>>>>>>>>>>>>> The changes looks good.
>>>>>>>>>>>>> Thank you for making them!
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks!
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>> On 11/3/17 05:10, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>> 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.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> I'd expect a check for some exception name, not for details like: For
>>>>>>>>>>>>> input string: "apa".
>>>>>>>>>>>>
>>>>>>>>>>>> Should we remove this comparison?
>>>>>>>>>>>
>>>>>>>>>>> I don't understand. Why do remove?
>>>>>>>>>>> Would it better to check for the exception name instead?
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>>>>> 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
>>>>>>>>>>>>>
>>>>>>>>>>>>> There are more tests related to dynamic attach in closed,
>>>>>>>>>>>>> nsk.aod.testlist and 30+ attach tests in nsk.jvmti.testlist.
>>>>>>>>>>>>> I'm not sure, if they are included into any of the Mach5 testing levels.
>>>>>>>>>>>>> Will need to check.
>>>>>>>>>>>>> We have to make sure these tests are still passed.
>>>>>>>>>>>>
>>>>>>>>>>>> I cannot access JPRT and closed testcases because I'm not an Oracle
>>>>>>>>>>>> employee.
>>>>>>>>>>>> Can you run them with this change?
>>>>>>>>>>>
>>>>>>>>>>> Ok.
>>>>>>>>>>> I will sponsor this fix and run these tests before the push.
>>>>>>>>>>>
>>>>>>>>>>> It seems, another update and one more review is needed.
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> Serguei
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>
>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>> Serguei
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 2017/11/03 16:31, [hidden email] 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
>>>>>>>>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>
>>>
>
Reply | Threaded
Open this post in threaded view
|

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

Yasumasa Suenaga-4
PING:

Could you review it?

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

I want to merge this change to jdk 10. So I need a second reviewer.


Yasumasa



On 2017/11/16 21:09, Yasumasa Suenaga wrote:

> Hi David, Serguei,
>
>>> The test logic is adding it in AttachFailedTestBase.java:
>>>
>>>   45         return Paths.get(System.getProperty("test.nativepath"), "lib", libname)
>>>   46                     .toAbsolutePath()
>>>   47                     .toString();
>
> Thanks!
> I've fixed it in new webrev:
>
>    http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.03/
>
>
> I've tested it as below. It works fine:
>
> $ $JT_HOME/bin/jtreg -ignore:quiet -nativepath:$NATIVE_PATH hotspot/jtreg/serviceability/dcmd/jvmti/AttachFailed
> $ echo $NATIVE_PATH
> /<Path to configuration>/images/test/hotspot/jtreg/native
>
>
> Thanks,
>
> Yasumasa
>
>
> On 2017/11/16 16:49, [hidden email] wrote:
>> On 11/15/17 23:29, David Holmes wrote:
>>> On 16/11/2017 4:43 PM, [hidden email] wrote:
>>>> On 11/15/17 18:11, David Holmes wrote:
>>>>> Hi Serguei,
>>>>>
>>>>> > /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so'
>>>>>
>>>>> There should not be any "/lib/" in that path
>>>>
>>>> Right, it should not be.
>>>
>>> The test logic is adding it in AttachFailedTestBase.java:
>>>
>>>   45         return Paths.get(System.getProperty("test.nativepath"), "lib", libname)
>>>   46                     .toAbsolutePath()
>>>   47                     .toString();
>>>
>>> but it shouldn't.
>>
>> Nice catch!
>> I looked right to these lines and overlooked it. :)
>>
>> Thanks,
>> Serguei
>>
>>>
>>> David
>>> -----
>>>
>>>
>>>> This is the script I'm using to run the tests:
>>>>
>>>> #!/bin/sh
>>>>
>>>> REPO=/var/tmp/sspitsyn/jdk.attach
>>>> IMAGES=${REPO}/build/linux-x86_64-normal-server-release/images
>>>> export JAVA_HOME=${IMAGES}/jdk
>>>> export NATIVE_PATH=${IMAGES}/../support/test/hotspot/jtreg/native
>>>> export NATIVE_PATH=${IMAGES}/test/hotspot/jtreg/native
>>>> echo "JAVA_HOME = $JAVA_HOME"
>>>>
>>>> /java/re/jtreg/4.2/nightly/binaries/jtreg/bin/jtreg -nativepath:${NATIVE_PATH} \
>>>> $REPO/open/test/hotspot/jtreg/serviceability/dcmd/jvmti/AttachFailed
>>>>
>>>>
>>>> This is a part of log with the reported error from the AttachException.jtr:
>>>>
>>>> [TestNG] Running:
>>>>    serviceability/dcmd/jvmti/AttachFailed/AttachException.java
>>>>
>>>> Running DCMD 'JVMTI.agent_load /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so' through 'PidJcmdExecutor'
>>>> Executing command '[/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/jdk/bin/jcmd, 8689, JVMTI.agent_load /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so]'
>>>> Command returned with exit code 0
>>>> ---------------- stdout ----------------
>>>> 8689:
>>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native*/lib*/libException.so was not loaded.
>>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native*/lib*/libException.so: cannot open shared object file: No such file or directory
>>>>
>>>>
>>>> These are the locations of the libException.so:
>>>> build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/libException.so
>>>> build/linux-x86_64-normal-server-release/support/test/hotspot/jtreg/native/lib/libException.so
>>>>
>>>>
>>>> The tests fail with the "NATIVE_PATH=${IMAGES}/test/hotspot/jtreg/native"
>>>> but pass with the "export NATIVE_PATH=${IMAGES}/../support/test/hotspot/jtreg/native".
>>>>
>>>>
>>>> When the "export NATIVE_PATH=${IMAGES}/../support/test/hotspot/jtreg/native" is used
>>>> the log has this line:
>>>>
>>>> Running DCMD 'JVMTI.agent_load /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/../support/test/hotspot/jtreg/native*/lib*/libException.so' through 'JMXExecutor'
>>>>
>>>>
>>>> Apparently, the sub-directory name "/lib" is added to the path.
>>>>
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>>
>>>>> David
>>>>>
>>>>> On 16/11/2017 4:34 AM, [hidden email] wrote:
>>>>>> Hi Yasumasa and David,
>>>>>>
>>>>>>
>>>>>> On 11/15/17 04:56, David Holmes wrote:
>>>>>>> On 15/11/2017 10:15 PM, Yasumasa Suenaga wrote:
>>>>>>>> Hi Serguei,
>>>>>>>>
>>>>>>>>> Do the new tests pass in your runs?
>>>>>>>>
>>>>>>>> Of course.
>>>>>>>> It seems not to exist jtreg native libraries.
>>>>>>>> I've tested as below:
>>>>>>>>
>>>>>>>>    $ make build-test-hotspot-jtreg-native
>>>>>>>>    $ cd test
>>>>>>>>    $ $JT_HOME/bin/jtreg -ignore:quiet -nativepath:<builddir>/<confdir>/support/test/hotspot/jtreg/native/lib hotspot/jtreg/serviceability/attach hotspot/jtreg/serviceability/dcmd/jvmti jdk/com/sun/tools/attach
>>>>>>
>>>>>> Thanks.
>>>>>> I missed to add the -nativepath flag, sorry.
>>>>>>
>>>>>>> Please check that:
>>>>>>>
>>>>>>> make test-image
>>>>>>>
>>>>>>> followed by jtreg -nativepath:<build-dir>/images/test/hotspot/jtreg/native
>>>>>>>
>>>>>>> also works.
>>>>>>
>>>>>> It fails with the error:
>>>>>>
>>>>>>    63 Running DCMD 'JVMTI.agent_load /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so' through 'PidJcmdExecutor'
>>>>>>    64 Executing command '[/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/jdk/bin/jcmd, 28407, JVMTI.agent_load /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg /native/lib/libException.so]'
>>>>>>    65 Command returned with exit code 0
>>>>>>    66 ---------------- stdout ----------------
>>>>>>    67 28407:
>>>>>>    68 /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so was not loaded.
>>>>>>    69 /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so: cannot open shared object file: No such file or directory
>>>>>>    70
>>>>>>
>>>>>>
>>>>>> It seems, the '/lib' folder is added to the nativepath.
>>>>>>
>>>>>> Yasumasa, could you, double check it please?
>>>>>>
>>>>>> I'm using the jtreg:
>>>>>> /java/re/jtreg/4.2/promoted/latest/binaries/jtreg/bin/jtreg
>>>>>>
>>>>>> which is:
>>>>>>
>>>>>> % ls -l /java/re/jtreg/4.2/promoted/latest
>>>>>> lrwxrwxrwx 1 uucp 143 7 Nov  6 21:49 /java/re/jtreg/4.2/promoted/latest -> fcs/b10/
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Serguei
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>> David
>>>>>>>
>>>>>>>>
>>>>>>>>> Good news is that the attach-related tests from closed repository are passed.
>>>>>>>>
>>>>>>>> Thanks!
>>>>>>>>
>>>>>>>>
>>>>>>>> Yasumasa
>>>>>>>>
>>>>>>>>
>>>>>>>> On 2017/11/15 16:38, [hidden email] wrote:
>>>>>>>>> Hi Yasumasa,
>>>>>>>>>
>>>>>>>>> Do the new tests pass in your runs?
>>>>>>>>>
>>>>>>>>> In my runs 3 of 4 tests are failed with the errors like this:
>>>>>>>>>
>>>>>>>>>   109 Running DCMD 'JVMTI.agent_load /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so' through 'PidJcmdExecutor'
>>>>>>>>>   110 Executing command '[/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/jdk/bin/jcmd, 21951, JVMTI.agent_load /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so]'
>>>>>>>>>   111 Command returned with exit code 0
>>>>>>>>>   112 ---------------- stdout ----------------
>>>>>>>>>   113 21951:
>>>>>>>>>   114 /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so was not loaded.
>>>>>>>>>   115 /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so: cannot open shared object file: No such file or directory
>>>>>>>>>   116
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Good news is that the attach-related tests from closed repository are passed.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Serguei
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 11/14/17 16:40, [hidden email] wrote:
>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>
>>>>>>>>>> It looks good to me.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Serguei
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 11/7/17 22:38, Yasumasa Suenaga wrote:
>>>>>>>>>>> Hi Serguei,
>>>>>>>>>>>
>>>>>>>>>>> I uploaded new webrev:
>>>>>>>>>>>
>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.02/
>>>>>>>>>>>
>>>>>>>>>>>>>> I'd expect a check for some exception name, not for details like: For
>>>>>>>>>>>>>> input string: "apa".
>>>>>>>>>>>>>
>>>>>>>>>>>>> Should we remove this comparison?
>>>>>>>>>>>>
>>>>>>>>>>>> I don't understand. Why do remove?
>>>>>>>>>>>> Would it better to check for the exception name instead?
>>>>>>>>>>> I've changed to check exception name (NumberFormatException) in
>>>>>>>>>>> StartManagementAgent.java.
>>>>>>>>>>>
>>>>>>>>>>>> I will sponsor this fix and run these tests before the push.
>>>>>>>>>>> Thanks!
>>>>>>>>>>> I'm waiting for second reviewer.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Yasumasa
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> 2017-11-08 11:55 GMT+09:00 [hidden email]
>>>>>>>>>>> <[hidden email]>:
>>>>>>>>>>>> On 11/6/17 04:31, Yasumasa Suenaga wrote:
>>>>>>>>>>>>> Hi Serguei,
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 2017/11/06 20:06, [hidden email] wrote:
>>>>>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> The changes looks good.
>>>>>>>>>>>>>> Thank you for making them!
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks!
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 11/3/17 05:10, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>> 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.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I'd expect a check for some exception name, not for details like: For
>>>>>>>>>>>>>> input string: "apa".
>>>>>>>>>>>>>
>>>>>>>>>>>>> Should we remove this comparison?
>>>>>>>>>>>>
>>>>>>>>>>>> I don't understand. Why do remove?
>>>>>>>>>>>> Would it better to check for the exception name instead?
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>>>>> 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
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> There are more tests related to dynamic attach in closed,
>>>>>>>>>>>>>> nsk.aod.testlist and 30+ attach tests in nsk.jvmti.testlist.
>>>>>>>>>>>>>> I'm not sure, if they are included into any of the Mach5 testing levels.
>>>>>>>>>>>>>> Will need to check.
>>>>>>>>>>>>>> We have to make sure these tests are still passed.
>>>>>>>>>>>>>
>>>>>>>>>>>>> I cannot access JPRT and closed testcases because I'm not an Oracle
>>>>>>>>>>>>> employee.
>>>>>>>>>>>>> Can you run them with this change?
>>>>>>>>>>>>
>>>>>>>>>>>> Ok.
>>>>>>>>>>>> I will sponsor this fix and run these tests before the push.
>>>>>>>>>>>>
>>>>>>>>>>>> It seems, another update and one more review is needed.
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> Serguei
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>> Serguei
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 2017/11/03 16:31, [hidden email] 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
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>
>>>>
>>
Reply | Threaded
Open this post in threaded view
|

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

David Holmes
Hi Yasumasa,

I've been trying to leave these reviews to serviceability folk ...

I've gone back through the original RFR from September last year to see
what we did and what was left.

The current proposal raises some concern for me - and IIRC Dmitry was
also concerned about it last time: printing of the pending exception. If
we print the pending exception we will report an error and throw
AgentLoadException, even if execution of the OnAttach function returned
JNI_OK. If that exception was not critical to the success of the loading
the agent, and the agent was just sloppy about clearing it, then it will
now fail to load - which would be a compatibility concern.

Further, if the exception indicates an error and the OnAttach function
returns JNI_ERR then we won't report that cleanly because the printing
of the exception will prevent matching with "return code: -1".

My own feeling is that it is up to the OnAttach function to properly
deal with pending exceptions: report and/or clear them. The VM side just
has to clear any pending exception to avoid it causing problems for
later code.

Some specific comments:

HotSpotVirtualMachine.java

The regex code seems overkill for the basic parsing you are doing. You
just need to see if the strings starts with "return code: " and then
parse the next bit as an integer to get the return code.

---

  test/jdk/com/sun/tools/attach/StartManagementAgent.java

The reporting of NumberFormatException may be accurate in terms of the
low-level exception, but "Invalid com.sun.management.jmxremote.port
number" was much clearer. This makes me wonder about whether the code
that previously produced "Invalid com.sun.management.jmxremote.port
number" needs updating if this change proceeds. (And alao makes me
wonder about the impact of the change in general.)

---

Sorry - not the quick second review you were looking for.

David
-----

On 19/11/2017 11:38 PM, Yasumasa Suenaga wrote:

> PING:
>
> Could you review it?
>
>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.03/
>
> I want to merge this change to jdk 10. So I need a second reviewer.
>
>
> Yasumasa
>
>
>
> On 2017/11/16 21:09, Yasumasa Suenaga wrote:
>> Hi David, Serguei,
>>
>>>> The test logic is adding it in AttachFailedTestBase.java:
>>>>
>>>>   45         return Paths.get(System.getProperty("test.nativepath"),
>>>> "lib", libname)
>>>>   46                     .toAbsolutePath()
>>>>   47                     .toString();
>>
>> Thanks!
>> I've fixed it in new webrev:
>>
>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.03/
>>
>>
>> I've tested it as below. It works fine:
>>
>> $ $JT_HOME/bin/jtreg -ignore:quiet -nativepath:$NATIVE_PATH
>> hotspot/jtreg/serviceability/dcmd/jvmti/AttachFailed
>> $ echo $NATIVE_PATH
>> /<Path to configuration>/images/test/hotspot/jtreg/native
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>> On 2017/11/16 16:49, [hidden email] wrote:
>>> On 11/15/17 23:29, David Holmes wrote:
>>>> On 16/11/2017 4:43 PM, [hidden email] wrote:
>>>>> On 11/15/17 18:11, David Holmes wrote:
>>>>>> Hi Serguei,
>>>>>>
>>>>>> >
>>>>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so'
>>>>>>
>>>>>>
>>>>>> There should not be any "/lib/" in that path
>>>>>
>>>>> Right, it should not be.
>>>>
>>>> The test logic is adding it in AttachFailedTestBase.java:
>>>>
>>>>   45         return Paths.get(System.getProperty("test.nativepath"),
>>>> "lib", libname)
>>>>   46                     .toAbsolutePath()
>>>>   47                     .toString();
>>>>
>>>> but it shouldn't.
>>>
>>> Nice catch!
>>> I looked right to these lines and overlooked it. :)
>>>
>>> Thanks,
>>> Serguei
>>>
>>>>
>>>> David
>>>> -----
>>>>
>>>>
>>>>> This is the script I'm using to run the tests:
>>>>>
>>>>> #!/bin/sh
>>>>>
>>>>> REPO=/var/tmp/sspitsyn/jdk.attach
>>>>> IMAGES=${REPO}/build/linux-x86_64-normal-server-release/images
>>>>> export JAVA_HOME=${IMAGES}/jdk
>>>>> export NATIVE_PATH=${IMAGES}/../support/test/hotspot/jtreg/native
>>>>> export NATIVE_PATH=${IMAGES}/test/hotspot/jtreg/native
>>>>> echo "JAVA_HOME = $JAVA_HOME"
>>>>>
>>>>> /java/re/jtreg/4.2/nightly/binaries/jtreg/bin/jtreg
>>>>> -nativepath:${NATIVE_PATH} \
>>>>> $REPO/open/test/hotspot/jtreg/serviceability/dcmd/jvmti/AttachFailed
>>>>>
>>>>>
>>>>> This is a part of log with the reported error from the
>>>>> AttachException.jtr:
>>>>>
>>>>> [TestNG] Running:
>>>>>    serviceability/dcmd/jvmti/AttachFailed/AttachException.java
>>>>>
>>>>> Running DCMD 'JVMTI.agent_load
>>>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so'
>>>>> through 'PidJcmdExecutor'
>>>>> Executing command
>>>>> '[/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/jdk/bin/jcmd,
>>>>> 8689, JVMTI.agent_load
>>>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so]'
>>>>>
>>>>> Command returned with exit code 0
>>>>> ---------------- stdout ----------------
>>>>> 8689:
>>>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native*/lib*/libException.so
>>>>> was not loaded.
>>>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native*/lib*/libException.so:
>>>>> cannot open shared object file: No such file or directory
>>>>>
>>>>>
>>>>> These are the locations of the libException.so:
>>>>> build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/libException.so
>>>>>
>>>>> build/linux-x86_64-normal-server-release/support/test/hotspot/jtreg/native/lib/libException.so
>>>>>
>>>>>
>>>>>
>>>>> The tests fail with the
>>>>> "NATIVE_PATH=${IMAGES}/test/hotspot/jtreg/native"
>>>>> but pass with the "export
>>>>> NATIVE_PATH=${IMAGES}/../support/test/hotspot/jtreg/native".
>>>>>
>>>>>
>>>>> When the "export
>>>>> NATIVE_PATH=${IMAGES}/../support/test/hotspot/jtreg/native" is used
>>>>> the log has this line:
>>>>>
>>>>> Running DCMD 'JVMTI.agent_load
>>>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/../support/test/hotspot/jtreg/native*/lib*/libException.so'
>>>>> through 'JMXExecutor'
>>>>>
>>>>>
>>>>> Apparently, the sub-directory name "/lib" is added to the path.
>>>>>
>>>>>
>>>>> Thanks,
>>>>> Serguei
>>>>>
>>>>>
>>>>>> David
>>>>>>
>>>>>> On 16/11/2017 4:34 AM, [hidden email] wrote:
>>>>>>> Hi Yasumasa and David,
>>>>>>>
>>>>>>>
>>>>>>> On 11/15/17 04:56, David Holmes wrote:
>>>>>>>> On 15/11/2017 10:15 PM, Yasumasa Suenaga wrote:
>>>>>>>>> Hi Serguei,
>>>>>>>>>
>>>>>>>>>> Do the new tests pass in your runs?
>>>>>>>>>
>>>>>>>>> Of course.
>>>>>>>>> It seems not to exist jtreg native libraries.
>>>>>>>>> I've tested as below:
>>>>>>>>>
>>>>>>>>>    $ make build-test-hotspot-jtreg-native
>>>>>>>>>    $ cd test
>>>>>>>>>    $ $JT_HOME/bin/jtreg -ignore:quiet
>>>>>>>>> -nativepath:<builddir>/<confdir>/support/test/hotspot/jtreg/native/lib
>>>>>>>>> hotspot/jtreg/serviceability/attach
>>>>>>>>> hotspot/jtreg/serviceability/dcmd/jvmti jdk/com/sun/tools/attach
>>>>>>>
>>>>>>> Thanks.
>>>>>>> I missed to add the -nativepath flag, sorry.
>>>>>>>
>>>>>>>> Please check that:
>>>>>>>>
>>>>>>>> make test-image
>>>>>>>>
>>>>>>>> followed by jtreg
>>>>>>>> -nativepath:<build-dir>/images/test/hotspot/jtreg/native
>>>>>>>>
>>>>>>>> also works.
>>>>>>>
>>>>>>> It fails with the error:
>>>>>>>
>>>>>>>    63 Running DCMD 'JVMTI.agent_load
>>>>>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so'
>>>>>>> through 'PidJcmdExecutor'
>>>>>>>    64 Executing command
>>>>>>> '[/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/jdk/bin/jcmd,
>>>>>>> 28407, JVMTI.agent_load
>>>>>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg
>>>>>>> /native/lib/libException.so]'
>>>>>>>    65 Command returned with exit code 0
>>>>>>>    66 ---------------- stdout ----------------
>>>>>>>    67 28407:
>>>>>>>    68
>>>>>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so
>>>>>>> was not loaded.
>>>>>>>    69
>>>>>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so:
>>>>>>> cannot open shared object file: No such file or directory
>>>>>>>    70
>>>>>>>
>>>>>>>
>>>>>>> It seems, the '/lib' folder is added to the nativepath.
>>>>>>>
>>>>>>> Yasumasa, could you, double check it please?
>>>>>>>
>>>>>>> I'm using the jtreg:
>>>>>>> /java/re/jtreg/4.2/promoted/latest/binaries/jtreg/bin/jtreg
>>>>>>>
>>>>>>> which is:
>>>>>>>
>>>>>>> % ls -l /java/re/jtreg/4.2/promoted/latest
>>>>>>> lrwxrwxrwx 1 uucp 143 7 Nov  6 21:49
>>>>>>> /java/re/jtreg/4.2/promoted/latest -> fcs/b10/
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Serguei
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> David
>>>>>>>>
>>>>>>>>>
>>>>>>>>>> Good news is that the attach-related tests from closed
>>>>>>>>>> repository are passed.
>>>>>>>>>
>>>>>>>>> Thanks!
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Yasumasa
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 2017/11/15 16:38, [hidden email] wrote:
>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>
>>>>>>>>>> Do the new tests pass in your runs?
>>>>>>>>>>
>>>>>>>>>> In my runs 3 of 4 tests are failed with the errors like this:
>>>>>>>>>>
>>>>>>>>>>   109 Running DCMD 'JVMTI.agent_load
>>>>>>>>>> /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so'
>>>>>>>>>> through 'PidJcmdExecutor'
>>>>>>>>>>   110 Executing command
>>>>>>>>>> '[/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/jdk/bin/jcmd,
>>>>>>>>>> 21951, JVMTI.agent_load
>>>>>>>>>> /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so]'
>>>>>>>>>>
>>>>>>>>>>   111 Command returned with exit code 0
>>>>>>>>>>   112 ---------------- stdout ----------------
>>>>>>>>>>   113 21951:
>>>>>>>>>>   114
>>>>>>>>>> /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so
>>>>>>>>>> was not loaded.
>>>>>>>>>>   115
>>>>>>>>>> /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so:
>>>>>>>>>> cannot open shared object file: No such file or directory
>>>>>>>>>>   116
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Good news is that the attach-related tests from closed
>>>>>>>>>> repository are passed.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Serguei
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 11/14/17 16:40, [hidden email] wrote:
>>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>>
>>>>>>>>>>> It looks good to me.
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> Serguei
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 11/7/17 22:38, Yasumasa Suenaga wrote:
>>>>>>>>>>>> Hi Serguei,
>>>>>>>>>>>>
>>>>>>>>>>>> I uploaded new webrev:
>>>>>>>>>>>>
>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.02/
>>>>>>>>>>>>
>>>>>>>>>>>>>>> I'd expect a check for some exception name, not for
>>>>>>>>>>>>>>> details like: For
>>>>>>>>>>>>>>> input string: "apa".
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Should we remove this comparison?
>>>>>>>>>>>>>
>>>>>>>>>>>>> I don't understand. Why do remove?
>>>>>>>>>>>>> Would it better to check for the exception name instead?
>>>>>>>>>>>> I've changed to check exception name (NumberFormatException) in
>>>>>>>>>>>> StartManagementAgent.java.
>>>>>>>>>>>>
>>>>>>>>>>>>> I will sponsor this fix and run these tests before the push.
>>>>>>>>>>>> Thanks!
>>>>>>>>>>>> I'm waiting for second reviewer.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> 2017-11-08 11:55 GMT+09:00 [hidden email]
>>>>>>>>>>>> <[hidden email]>:
>>>>>>>>>>>>> On 11/6/17 04:31, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>> Hi Serguei,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 2017/11/06 20:06, [hidden email] wrote:
>>>>>>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> The changes looks good.
>>>>>>>>>>>>>>> Thank you for making them!
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks!
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 11/3/17 05:10, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>>> 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.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I'd expect a check for some exception name, not for
>>>>>>>>>>>>>>> details like: For
>>>>>>>>>>>>>>> input string: "apa".
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Should we remove this comparison?
>>>>>>>>>>>>>
>>>>>>>>>>>>> I don't understand. Why do remove?
>>>>>>>>>>>>> Would it better to check for the exception name instead?
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> 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
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> There are more tests related to dynamic attach in closed,
>>>>>>>>>>>>>>> nsk.aod.testlist and 30+ attach tests in nsk.jvmti.testlist.
>>>>>>>>>>>>>>> I'm not sure, if they are included into any of the Mach5
>>>>>>>>>>>>>>> testing levels.
>>>>>>>>>>>>>>> Will need to check.
>>>>>>>>>>>>>>> We have to make sure these tests are still passed.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I cannot access JPRT and closed testcases because I'm not
>>>>>>>>>>>>>> an Oracle
>>>>>>>>>>>>>> employee.
>>>>>>>>>>>>>> Can you run them with this change?
>>>>>>>>>>>>>
>>>>>>>>>>>>> Ok.
>>>>>>>>>>>>> I will sponsor this fix and run these tests before the push.
>>>>>>>>>>>>>
>>>>>>>>>>>>> It seems, another update and one more review is needed.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>> Serguei
>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>> Serguei
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On 2017/11/03 16:31, [hidden email] 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
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>
>>>>>
>>>
12