Hi Yasumasa,
Yes, you can list me as a reviewer.
Thanks,
Serguei
On 6/26/20 18:54, Yasumasa Suenaga wrote:
Hi Serguei, David,
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.
Yes, it is checked at Handshake::execute_direct().
```
349 ThreadsListHandle tlh;
350 if (tlh.includes(target)) {
351 target->set_handshake_operation(&op);
352 } else {
353 log_handshake_info(start_time_ns, op.name(), 0, 0, "(thread
dead)");
354 return false;
355 }
```
Serguei, can I list you as a Reviewer? If so, I will push this change
when I got second reviewer.
Thanks,
Yasumasa
On 2020/06/27 8:51, [email protected] wrote:
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, [email protected] 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