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