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