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

[David Holmes]

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
>