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

[Serguei Spitsyn]

On Fri, 8 Sep 2023 02:46:36 GMT, Leonid Mesnik <lmes...@openjdk.org> wrote:

>> Serguei Spitsyn has updated the pull request with a new target base due to a 
>> merge or a rebase. The incremental webrev excludes the unrelated changes 
>> brought in by the merge/rebase. The pull request contains two additional 
>> commits since the last revision:
>> 
>>  - Merge
>>  - 8312174: missing JVMTI events from vthreads parked during JVMTI attach
>
> test/hotspot/jtreg/serviceability/jvmti/vthread/VThreadEventTest/libVThreadEventTest.cpp
>  line 30:
> 
>> 28: #include "jvmti_common.h"
>> 29: 
>> 30: #ifdef _WIN32
> 
> Do we need it here?

Thanks. Simplified now.

> test/hotspot/jtreg/serviceability/jvmti/vthread/VThreadEventTest/libVThreadEventTest.cpp
>  line 44:
> 
>> 42: 
>> 43:   void JNICALL VirtualThreadEnd(jvmtiEnv *jvmti, JNIEnv* jni, jthread 
>> virtual_thread) {
>> 44:     std::lock_guard<std::mutex> lockGuard(lock);
> 
> the atomic would be better for counters. It guarantees that counter is always 
> protected.

Thanks. Alex suggested the same. Fixed now.

> test/hotspot/jtreg/serviceability/jvmti/vthread/VThreadEventTest/libVThreadEventTest.cpp
>  line 62:
> 
>> 60: 
>> 61: void
>> 62: check_jvmti_err(jvmtiError err, const char* msg) {
> 
> This function could be moved into jvmti_common.h.

Good suggestion, thanks. Fixed now.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15467#discussion_r1320450305
PR Review Comment: https://git.openjdk.org/jdk/pull/15467#discussion_r1320450372
PR Review Comment: https://git.openjdk.org/jdk/pull/15467#discussion_r1320450217