On Thu, 29 Jun 2023 18:40:50 GMT, Alex Menkov <amen...@openjdk.org> wrote:
>> This is follow-up JDK-8307153/JDK-8309612 (JVMTI GetThreadState on carrier >> should return STATE_WAITING) >> New test tests GetThreadState for different thread states. >> The test detected a bug in the implementation, new issue is created: >> JDK-8310584 >> Corresponding testcases are commented now in the test, fix for JDK-8310584 >> will uncomment them > > Alex Menkov has updated the pull request incrementally with one additional > commit since the last revision: > > updated comment Just a couple of comment suggestions. Otherwise looks good. What testing did you do? Any -Xcomp testing? test/hotspot/jtreg/serviceability/jvmti/vthread/GetThreadStateMountedTest/GetThreadStateMountedTest.java line 41: > 39: /** > 40: * The test implements different scenarios to get desired JVMTI thread > states. > 41: * For each scenario test also checks states after carrier and virtual > threads suspend/resume Suggestion: * For each scenario the test also checks states after carrier and virtual threads suspend/resume test/hotspot/jtreg/serviceability/jvmti/vthread/GetThreadStateMountedTest/GetThreadStateMountedTest.java line 45: > 43: * Special handling is required for WAITING state scenarios: > 44: * Spurious wakeups may cause unexpected thread state change and this > causes test failure. > 45: * To avoid this test thread should be suspended (i.e. carrier and/or > mounted virtual thread is suspended). Suggestion: * To avoid this, the test thread should be suspended (i.e. carrier and/or mounted virtual thread is suspended). ------------- Marked as reviewed by cjplummer (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14622#pullrequestreview-1523188533 PR Review Comment: https://git.openjdk.org/jdk/pull/14622#discussion_r1259012273 PR Review Comment: https://git.openjdk.org/jdk/pull/14622#discussion_r1259012879
