On Fri, 1 Jul 2022 18:51:55 GMT, Alan Bateman <[email protected]> wrote:

>> Serguei Spitsyn has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   fix serviceability/jvmti/thread/thrstat03 to except correct GetThreadState 
>> result for terminated vthreads
>
> test/hotspot/jtreg/serviceability/jvmti/vthread/SelfSuspendDisablerTest/SelfSuspendDisablerTest.java
>  line 60:
> 
>> 58:     }
>> 59: 
>> 60:     private static void testJvmtiThreadState(Thread thread, int 
>> expectedState) throws RuntimeException {
> 
> Minor nit, "throws RuntimeException" is not needed here.

Ah, yes. I was thinking it is not needed but forgot to double-check.
Fixed now.

> The fix looks good but I'm wondering why the SelfSuspendDisabledTest is being 
> used to test this bug. I guess I expected to see a test for GetThreadState 
> instead.

The GetThreadState by its instrumental role can be used by many tests.
My initial intent was to check the terminated virtual thread case.
It looked as unneeded overhead to create a new test for this.
So, I've found this small test which already has convenient infrastructure to 
recreate needed conditions.
Then I decided to extend GetThreadState coverage in this test a little bit.
I can create a GetThreadState specific test if you think it is worth it.
 Interesting enough that I've found the existing test which already had needed 
coverage:
   ` test/hotspot/jtreg/serviceability/jvmti/thread/GetThreadState/thrstat03`
But this test was adjusted to adopt to incorrect GetThreadState result for 
virtual threads, so I had to fix it now.
Strongly speaking, the update of SelfSuspendDisabledTest is not needed.
But I feel it is worth to keep it.

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

PR: https://git.openjdk.org/jdk19/pull/102

Reply via email to