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

[Serguei Spitsyn]

On Mon, 11 Sep 2023 09:08:26 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> This update fixes two important issues:
>>  - Issue reported by a bug submitter about missing JVMTI events on virtual threads after an a JVMTI agent dynamic attach
>>  -  Known scalability/performance issue: a need to lazily create `JvmtiThreadState's` for virtual threads
>> 
>> The issue is tricky to fix because the existing mechanism of the JVMTI event management does not support unmounted virtual threads. The JVMTI `SetEventNotificationMode()` calls the function `JvmtiEventControllerPrivate::recompute_enabled()`
>> which inspects a `JavaThread's` list and for each thread in the list recomputes enabled event bits with the function `JvmtiEventControllerPrivate::recompute_thread_enabled()`.  The `JvmtiThreadState` of each thread is created but only when it is really needed, eg, if any of the thread filtered events is enabled. There was an initial adjustment of this mechanism for virtual threads which accounted for both carrier and virtual threads when a virtual thread is mounted. However, it does not work for unmounted virtual threads. A temporary work around was to always create `JvmtiThreadState` for each virtual thread eagerly at a thread starting point.
>> 
>> This fix introduces new function `JvmtiExport::get_jvmti_thread_state()` which checks if thread is virtual and there is a thread filtered event enabled globally, and if so, forces a creation of the `JvmtiThreadState`. Another adjustment was needed because the function `state_for_while_locked()` can be called directly in some contexts. New function `JvmtiEventController::recompute_thread_filtered()` was introduced to make necessary corrections.
>> 
>> Testing:
>>  - new test from the bug report was adopted: `test/hotspot/jtreg/serviceability/jvmti/vthread/VThreadEventTest`
>>  - ran mach5 tiers 1-6: all are passed
>
> Serguei Spitsyn has updated the pull request incrementally with one additional commit since the last revision:
> 
>   removed JavaThread::is_virtual()

I've pushed an update.
It includes:
 - addressed review comments on new test `test/hotspot/jtreg/serviceability/jvmti/vthread/VThreadEventTest` (some details are listed below)
 - added comments for `state_for()` and `state_for_while_locked()` to `src/hotspot/share/prims/jvmtiThreadState.hpp` as Alex suggested
 - moved the call to `JvmtiEventController::recompute_thread_filtered(state)` from `state_for_while_locked()` to `state_for()`
 - removed newly added function `JavaThread::is_virtual()` and replaced its use in `jvmtiExport.cpp` with `is_vthread_mounted()`

Some of new test updates:
 - Renamed `check_jvmti_err()` to `check_jvmti_error()` and moved it to `test/lib/jdk/test/lib/jvmti/jvmti_common.h` as Leonid suggested
 - Removed VARIADICJNI and `namespace` in the native agent
 - Removed `std::mutex lock` and used atomic counter instead
 - Added `@requires vm.compMode != "Xcomp"` as the test execution time with `-Xcomp` sometimes is not enough
 - I did not replace the test `Counter` class with the use `CountDownLatch` because it caused deadlocks while the test never deadlocks with the `Counter` class. Instead I've renamed `Counter` to `CountDownLatch` so that it can be easy to remove this custom implementation of `CountDownLatch` with the one from the `java.util.concorrent`. I'm not sure yet why use of original `CountDownLatch` class causes deadlocks.
 - Refactored Java part of the test by introducing methods `test1()`, `test2()` and `test3()`
 - Added code to wait for `test1()` to reach the execution of `Thread.sleep(big-timeout)` before the agent dynamic attach. One problem is that the thread state in `sleep()` is `WAITING` but not `TIMED_WAITING` (this looks like a bug: will need to follow up. So, there was a need to distinguish if the `test1()` does not execute the await code. It is why one more `CountDownLatch` object has been added.

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

PR Comment: https://git.openjdk.org/jdk/pull/15467#issuecomment-1714362970