On Fri, 12 May 2023 03:54:46 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:

>> [JDK-8306034](https://bugs.openjdk.org/browse/JDK-8306034) (#13546) added 
>> some virtual thread support to JVMTI StopThread. This allows JDWP 
>> ThreadReference.Stop and JDI ThreadReference.stop() to have the same level 
>> support for virtual threads.
>> 
>> For your reference, here are the JVMTI spec changes that this pr is using as 
>> the basis for the JDWP and JDI spec updates.
>> 
>> https://github.com/openjdk/jdk/pull/13546/files#diff-cab9797fde25f344784e4473778cd35bfd9e19ba36f82c11b1a0674636958339
>> 
>> Mostly this is a spec update for JDWP and JDI, but there are also some minor 
>> changes needed to the ThreadReference.stop() handling of JDWP errors, and 
>> throwing the appropriate exceptions. Also some minor cleanup in jdb. The 
>> debug agent doesn't need changes since JVMTI errors are just passed through 
>> as the corresponding JDWP errors, and the code for this is already in place. 
>> These errors are not new nor need any special handling.
>> 
>> Our existing testing for ThreadReference.stop() is fairly weak:
>> 
>> - nsk/jdb/kill/kill001, which tests stop() when the thread is suspended at a 
>> breakpoint. It will get problem listed by 
>> [JDK-8306034](https://bugs.openjdk.org/browse/JDK-8306034). I have fixes for 
>> it already working and will push it separately.
>> - nsk/jdi/stop/stop001, which is problem listed and only tests when the 
>> thread is blocked in Object.wait()
>> - nsk/jdi/stop/stop002, which only tests that throwing an invalid exception 
>> fails properly
>> 
>> I decided to take stop002 and make it a more thorough test of 
>> ThreadReference.stop(). See the comment at the top of the test for a list of 
>> what is tested. As for reviewing this test, it's probably best to look at it 
>> as a completely new test rather than looking at diffs since so much has been 
>> changed and added.
>
> Chris Plummer has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix error message to print correct exception name

Marked as reviewed by sspitsyn (Reviewer).

-------------

PR Review: https://git.openjdk.org/jdk/pull/13548#pullrequestreview-1423732446

Reply via email to