RFR: JDK-8164913: JVMTI.agent_load dcmd should show useful error message

[Yasumasa Suenaga]

Hi,

>> Then you have to add this justification to the bug report and set the priority to P3.

I've updated JBS.


> BTW, the fix looks good to me.
> I can sponsor the fix if you have no sponsor yet.

Thanks! I've not had a sponsor for this issue.
Please sponsor for it.

David, can I list you as a reviewer?
Dmitry, have you reviewed this change?


Thanks,

Yasumasa


On 2016/09/07 8:47, serguei.spitsyn at oracle.com wrote:
>
>
> On 9/6/16 15:46, serguei.spitsyn at oracle.com wrote:
>> Hi Yasumasa,
>>
>>
>> On 9/6/16 15:28, Yasumasa Suenaga wrote:
>>> Hi Serguei,
>>>
>>>>>> I think you would have to justify why this should be a P3 rather than P4 issue.
>>>>
>>>>
>>>> This needs to be answered as well.
>>>
>>> I've sent it in previous email as below:
>>>
>>>>>>> This problem is that jcmd shows incorrect status when JVMTI agent cannot
>>>>>>> be attached.
>>>>>>> I think this problem should be fixed in 9 GA.
>>>>>>> The users who want.to attach JVMTI agent want to know
>>>>>>> whether it succeed.
>>
>>
>> Then you have to add this justification to the bug report and set the priority to P3.
>> Not sure, what else is required by the RDP1 process.
>
> BTW, the fix looks good to me.
> I can sponsor the fix if you have no sponsor yet.
>
> Thanks,
> Serguei
>
>>
>> Thanks,
>> Serguei
>>
>>
>>>
>>>
>>> Yasumasa
>>>
>>>
>>> On 2016/09/07 3:48, serguei.spitsyn at oracle.com wrote:
>>>> Hi Yasumasa,
>>>>
>>>> Please, find my comment below.
>>>>
>>>>
>>>> On 9/6/16 06:06, Yasumasa Suenaga wrote:
>>>>> Hi David,
>>>>>
>>>>>> Is this something specific to code added in 9? If so that is motivation as we should not have a new feature with known bugs if we can avoid it. But if this already exists in JDK 8 and is a long standing "bug" ...
>>>>>
>>>>> The change for jvmtiExport.cpp can apply jdk8u:
>>>>>
>>>>> http://hg.openjdk.java.net/jdk8u/jdk8u-dev/hotspot/file/d55ea2916525/src/share/vm/prims/jvmtiExport.cpp#l2270
>>>>>
>>>>> I think HotSpot in jdk8u ignores exception and return JNI_OK even if Agent_OnAttach() fails.
>>>>> OTOH, the change for diagnosticCommand.cpp affects jdk9 only.
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>> On 2016/09/06 9:54, David Holmes wrote:
>>>>>> Hi Yasumasa,
>>>>>>
>>>>>> On 1/09/2016 1:47 PM, Yasumasa Suenaga wrote:
>>>>>>> Hi all,
>>>>>>>
>>>>>>> I think RDP1 has been started.
>>>>>>> Cannot I fix this?
>>>>>>
>>>>>> "P4-P5 bugs should, in general, be left to future releases unless they
>>>>>> only affect documentation, demos, or tests, in which case they should be
>>>>>> identified as such with the "noreg-doc", "noreg-demo", and "noreg-self"
>>>>>> labels, respectively." [1]
>>>>>>
>>>>>> This is due to the need to stabilize changes as we head towards GA.
>>>>>>
>>>>>> I think you would have to justify why this should be a P3 rather than P4 issue.
>>>>
>>>>
>>>> This needs to be answered as well.
>>>>
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>>
>>>>>> Is this something specific to code added in 9? If so that is motivation as we should not have a new feature with known bugs if we can avoid it. But if this already exists in JDK 8 and is a long standing "bug" ...
>>>>>>
>>>>>> David
>>>>>>
>>>>>> [1] http://mail.openjdk.java.net/pipermail/jdk9-dev/2016-August/004777.html
>>>>>>
>>>>>>
>>>>>>> This problem is that jcmd shows incorrect status when JVMTI agent cannot
>>>>>>> be attached.
>>>>>>> I think this problem should be fixed in 9 GA.
>>>>>>> The users who want.to attach JVMTI agent want to know
>>>>>>> whether it succeed.
>>>>>>>
>>>>>>> Yasumasa
>>>>>>>
>>>>>>>
>>>>>>> 2016/08/29 15:42 "Yasumasa Suenaga" <yasuenag at gmail.com
>>>>>>> <mailto:yasuenag at gmail.com>>:
>>>>>>>
>>>>>>>         This comment no longer matches the code and should be deleted:
>>>>>>>
>>>>>>>         2412       // Agent_OnAttach executed so completion status is JNI_OK
>>>>>>>         2413       st->print_cr("return code: %d", result);
>>>>>>>
>>>>>>>
>>>>>>>     Thanks David!
>>>>>>>     I removed it in new webrev.
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8164913/webrev.01/
>>>>>>> <http://cr.openjdk.java.net/~ysuenaga/JDK-8164913/webrev.01/>
>>>>>>>
>>>>>>>
>>>>>>>     Yasumasa
>>>>>>>
>>>>>>>
>>>>>>>     On 2016/08/29 12:59, David Holmes wrote:
>>>>>>>
>>>>>>>         Hi Yasumasa,
>>>>>>>
>>>>>>>         On 28/08/2016 10:47 PM, 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
>>>>>>>             output from
>>>>>>>             target VM.
>>>>>>>             So we should send error message from JVMTIAgentLoadDCmd.
>>>>>>>
>>>>>>>             I uploaded a webrev for it. Could you review it?
>>>>>>>
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8164913/webrev.00/
>>>>>>> <http://cr.openjdk.java.net/~ysuenaga/JDK-8164913/webrev.00/>
>>>>>>>
>>>>>>>
>>>>>>>         This seems reasonable.
>>>>>>>
>>>>>>>         src/share/vm/prims/jvmtiExport.cpp
>>>>>>>
>>>>>>>         This comment no longer matches the code and should be deleted:
>>>>>>>
>>>>>>>         2412       // Agent_OnAttach executed so completion status is JNI_OK
>>>>>>>         2413       st->print_cr("return code: %d", result);
>>>>>>>
>>>>>>>         Thanks,
>>>>>>>         David
>>>>>>>
>>>>>>>             I cannot access JPRT.
>>>>>>>             So I need a sponsor.
>>>>>>>
>>>>>>>
>>>>>>>             Thanks,
>>>>>>>
>>>>>>>             Yasumasa
>>>>>>>
>>>>
>>
>