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

[serguei.spitsyn at oracle.com]

Hi Yasumasa,

We are expected to reduce the risk at the RDP1, not to extend.
So that I'd suggest to reduce you fix instead of extending.
Is it possible to achieve your JVMTI.agent_load dcmd goal without
additional changes in the JvmtiExport::load_agent_library and
the HotSpotVirtualMachine.java ?
Or at least, could we minimize the impact?

I do not like the update in the HotSpotVirtualMachine.java
as it does not look as necessary to fix the bug 8164913.

Thanks,
Serguei


On 9/7/16 20:47, Yasumasa Suenaga wrote:
> Hi all,
>
> This changes has been approved. But it was not passed JPRT.
> It is caused by checking return value in 
> HotSpotVirtualMachine#loadAgentLibrary().
>
> I've fixed it and uploaded webrev. This changes has been passed JPRT. 
> (Thanks David!)
> Could you review again?
>
>   hotspot: 
> http://cr.openjdk.java.net/~ysuenaga/JDK-8164913/webrev.02/hotspot/
>       jdk: 
> http://cr.openjdk.java.net/~ysuenaga/JDK-8164913/webrev.02/jdk/
>
>
> Thanks,
>
> Yasumasa
>
>
> On 2016/09/07 16:47, serguei.spitsyn at oracle.com wrote:
>> On 9/7/16 00:45, Dmitry Samersoff wrote:
>>> Serguei,
>>>
>>> I'm OK with the fix and OK to be listed as a reviewer.
>>
>> Thanks, Dmitry!
>> Serguei
>>>
>>> -Dmitry
>>>
>>> On 2016-09-07 07:08, serguei.spitsyn at oracle.com wrote:
>>>> Hi Dmitry,
>>>>
>>>> As a sponsor I'm going to push this fix if you are Ok with the fix.
>>>> Please, let me know if you still have any concerns.
>>>> Also, confirm if you are Ok to be in the list of reviewers.
>>>>
>>>>
>>>> On 9/5/16 06:46, Dmitry Samersoff wrote:
>>>>> Yasumasa,
>>>>>
>>>>> I'll look closely to the fix.
>>>>>
>>>>> Please, notice:
>>>>>
>>>>> 1. We typically avoid printing attach error messages on
>>>>> the target VM side.
>>>> Some message was already printed:
>>>>
>>>> 2410 // Agent_OnAttach executed so completion status is JNI_OK
>>>> 2411 st->print_cr("%d", result);
>>>> 2412 result = JNI_OK;
>>>>
>>>> It is just a replacement. :)
>>>>
>>>>
>>>>> 2. We are in RDP1 for jdk9.
>>>>>
>>>>> http://mail.openjdk.java.net/pipermail/jdk9-dev/2016-August/004777.html 
>>>>>
>>>> I've set the priority to P3, so it can be pushed to the jdk9/hs.
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>>> -Dmitry
>>>>>
>>>>> On 2016-09-05 16:25, Yasumasa Suenaga wrote:
>>>>>> PING: Could you review and sponsor it?
>>>>>>
>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8164913/webrev.01/
>>>>>> Thanks,
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>> On 2016/09/01 12:47, Yasumasa Suenaga wrote:
>>>>>>> Hi all,
>>>>>>>
>>>>>>> I think RDP1 has been started.
>>>>>>> Cannot I fix this?
>>>>>>>
>>>>>>> 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 <http://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
>>>>>>>
>>>
>>