RFR: 8248379: Handshake closures for JVMTI monitor functions lack of some validations

[serguei.spitsyn at oracle.com]

Hi David,

Thank you for clarification.

Thanks,
Serguei


On 6/26/20 15:50, David Holmes wrote:
> Hi Serguei,
>
> On 27/06/2020 2:40 am, serguei.spitsyn at oracle.com wrote:
>> Hi Yasumasa,
>>
>> I see, some VM_op's also have this check:
>>
>> 1546   ThreadsListHandle tlh;
>> 1547   if (jt != NULL && tlh.includes(jt)
>>
>>
>> I wonder if it make sense to add as well.
>
> If you are executing the handshake operation then you are in a 
> handshake with the target thread which means it must exist in some 
> ThreadsList.
>
> Cheers,
> David
> -----
>
>> Otherwise, it looks good to me.
>>
>> Thanks,
>> Serguei
>>
>> On 6/26/20 00:03, Yasumasa Suenaga wrote:
>>> Hi all,
>>>
>>> Please review this change.
>>>
>>>   JBS: https://bugs.openjdk.java.net/browse/JDK-8248379
>>>   webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8248379/webrev.00/
>>>
>>> JDK-8242425 introduces to migrate to thread local handshake from VM 
>>> operation for GetOwnedMonitorInfo, GetOwnedMonitorStackDepthInfo, 
>>> and GetCurrentContendedMonitor JVMTI functions. However it lacks of 
>>> validations for thread state and thread oop of the target.
>>>
>>> This change has been tested on submit repo and serviceability/jvmti, 
>>> serviceability/jdwp vmTestbase/nsk/jvmti, vmTestbase/nsk/jdi 
>>> vmTestbase/nsk/jdwp.
>>> On submit repo, tools/javac/7118412/ShadowingTest.java and 
>>> java/foreign/TestMismatch.java were failed 
>>> (mach5-one-ysuenaga-JDK-8248379-20200626-0503-12110818). However 
>>> they do not seems to be related to this change.
>>> (Both tests have been passed on my Linux AMD64)
>>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>