On Wed, 28 Jun 2023 07:37:00 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:
> The test is great. I realize it is not easy to make it fully reliable though.
> So, will need another pass.
Good point about spurious wakeups.
Updated the test to handle them.
Now for WAITING scenarios the test suspends test thread and ensures it's
waiting. While the thread is suspended (either carrier or virtual) we can
safely verify thread states
> test/hotspot/jtreg/serviceability/jvmti/vthread/GetThreadStateMountedTest/GetThreadStateMountedTest.java
> line 117:
>
>> 115: } catch (InterruptedException ex) {
>> 116: // expected, ignore
>> 117: }
>
> Shout this code account for spurious wakeups?
Fixed
> test/hotspot/jtreg/serviceability/jvmti/vthread/GetThreadStateMountedTest/GetThreadStateMountedTest.java
> line 241:
>
>> 239: private static native void waitInNative();
>> 240: // Signals waitInNative() to exit.
>> 241: private static native void endWait();
>
> Q: Where is this native method invoked?
Good catch. Test thread inNative testcase never exits, it's terminated on the
test exit.
Fixed.
> test/hotspot/jtreg/serviceability/jvmti/vthread/GetThreadStateMountedTest/libGetThreadStateMountedTest.cpp
> line 73:
>
>> 71: jint unexpected = state - actual_full;
>> 72: LOG(" ERROR: some unexpected bits are set (%x): %s\n",
>> 73: unexpected, TranslateState(unexpected));
>
> Nit: lines 65, 73 have wrong indent.
Fixed.
> test/hotspot/jtreg/serviceability/jvmti/vthread/GetThreadStateMountedTest/libGetThreadStateMountedTest.cpp
> line 176:
>
>> 174: while (!time_to_exit) {
>> 175: sleep_ms(100);
>> 176: }
>
> Is this loop reliable in terms of thread states?
The code is pure native (sleep_ms uses native functions, it does not interact
with JVM), so there is no thread state changes
-------------
PR Comment: https://git.openjdk.org/jdk/pull/14622#issuecomment-1612423518
PR Review Comment: https://git.openjdk.org/jdk/pull/14622#discussion_r1246097094
PR Review Comment: https://git.openjdk.org/jdk/pull/14622#discussion_r1246096797
PR Review Comment: https://git.openjdk.org/jdk/pull/14622#discussion_r1246098484
PR Review Comment: https://git.openjdk.org/jdk/pull/14622#discussion_r1246098388
