RFR: 8311177: Switching to interpreter only mode in carrier thread can lead to crashes [v3]

[Serguei Spitsyn]

On Sat, 1 Jun 2024 00:22:45 GMT, Alex Menkov <amenkov at openjdk.org> wrote:

>> Serguei Spitsyn has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   review: refactored def and use of process_pending_interp_only()
>
> test/hotspot/jtreg/serviceability/jvmti/vthread/CarrierThreadEventNotification/libCarrierThreadEventNotification.cpp line 40:
> 
>> 38: 
>> 39: static const char* CTHREAD_NAME_START = "ForkJoinPool";
>> 40: static const size_t CTHREAD_NAME_START_LEN = (int)strlen("ForkJoinPool");
> 
> `(int)` cast is not needed

Thanks, fixed now.

> test/hotspot/jtreg/serviceability/jvmti/vthread/CarrierThreadEventNotification/libCarrierThreadEventNotification.cpp line 58:
> 
>> 56:       cthreads[ct_cnt++] = jni->NewGlobalRef(thread);
>> 57:     }
>> 58:     deallocate(jvmti, jni, (void*)tname);
> 
> cast to `void*` is not needed

Why do you think, the cast is not needed?

This is the `deallocate()` function in the `jvmti_common.hpp`:

static void
deallocate(jvmtiEnv *jvmti, JNIEnv* jni, void* ptr) {
  jvmtiError err = jvmti->Deallocate((unsigned char*)ptr);
  check_jvmti_status(jni, err, "deallocate: error in JVMTI Deallocate call");
}

> test/hotspot/jtreg/serviceability/jvmti/vthread/CarrierThreadEventNotification/libCarrierThreadEventNotification.cpp line 96:
> 
>> 94:     }
>> 95:     jvmtiError err = jvmti->Deallocate((unsigned char*)carrier_threads);
>> 96:     check_jvmti_status(jni, err, "deallocate: error in JVMTI Deallocate call");
> 
> replace with `deallocate(jvmti, jni, carrier_threads);` ?

Thanks, fixed now.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19438#discussion_r1624909747
PR Review Comment: https://git.openjdk.org/jdk/pull/19438#discussion_r1624911862
PR Review Comment: https://git.openjdk.org/jdk/pull/19438#discussion_r1624914083