RFR: 8312174: missing JVMTI events from vthreads parked during JVMTI attach [v5]

[Serguei Spitsyn]

On Mon, 11 Sep 2023 22:18:03 GMT, Alex Menkov <amenkov at openjdk.org> wrote:

>> Serguei Spitsyn has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   addressed second round of review comments on VThreadEventTest.java
>
> test/hotspot/jtreg/serviceability/jvmti/vthread/VThreadEventTest/VThreadEventTest.java line 45:
> 
>> 43:  * The test uses custom implementation of the CountDownLatch class.
>> 44:  * The reason is we want the state of tested thread to be predictable.
>> 45:  * With original CountDownLatch it is not clear what thread state is expected.
> 
> "original CountDownLatch" -> "java.util.concurrent.CountDownLatch"

Thanks - fixed now.

> test/hotspot/jtreg/serviceability/jvmti/vthread/VThreadEventTest/VThreadEventTest.java line 106:
> 
>> 104:         ready1.countDown(); // to guaranty state is not State.WAITING after await(mready)
>> 105:         try {
>> 106:             Thread.sleep(20000); // big timeout to keep unmounted untill interrupted
> 
> untill -> until

Thanks - fixed now.

> test/hotspot/jtreg/serviceability/jvmti/vthread/VThreadEventTest/VThreadEventTest.java line 132:
> 
>> 130:             // keep mounted
>> 131:         }
>> 132:         LockSupport.parkNanos(10 * TIMEOUT_BASE); // will cause extra mount and unmount
> 
> I don't see much sense in TIMEOUT_BASE constant (it's used only here and multiplied by 10)
> I think it would be clearer to
> Suggestion:
> 
>         // park for 10ms; causes extra unmount and mount
>         LockSupport.parkNanos(10_000_000L);

Thanks - fixed.

> test/hotspot/jtreg/serviceability/jvmti/vthread/VThreadEventTest/libVThreadEventTest.cpp line 24:
> 
>> 22:  */
>> 23: 
>> 24: #include <cstdlib>
> 
> I suppose this include was needed for abort() only and not needed anymore

Thanks - removed.

> test/hotspot/jtreg/serviceability/jvmti/vthread/VThreadEventTest/libVThreadEventTest.cpp line 27:
> 
>> 25: #include <cstring>
>> 26: #include <jvmti.h>
>> 27: #include <mutex>
> 
> not needed

Thanks - removed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15467#discussion_r1322195487
PR Review Comment: https://git.openjdk.org/jdk/pull/15467#discussion_r1322198339
PR Review Comment: https://git.openjdk.org/jdk/pull/15467#discussion_r1322201761
PR Review Comment: https://git.openjdk.org/jdk/pull/15467#discussion_r1322205612
PR Review Comment: https://git.openjdk.org/jdk/pull/15467#discussion_r1322204613