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

[serguei.spitsyn at oracle.com]

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