RFR: 8311177: Switching to interpreter only mode in carrier thread can lead to crashes
Serguei Spitsyn
sspitsyn at openjdk.org
Thu May 30 02:04:02 UTC 2024
On Thu, 30 May 2024 00:41:28 GMT, Alex Menkov <amenkov at openjdk.org> wrote:
>> Please, review the following `interp-only` issue related to carrier threads.
>> There are 3 problems fixed here:
>> - The `EnterInterpOnlyModeClosure::do_threads` is taking the `JvmtiThreadState` with the `jt->jvmti_thread_state()` which is incorrect when we have a deal with a carrier thread. The target state is known at the point when the `HandshakeClosure` is set, so the fix is to pass it as a constructor parameter.
>> - The `state->is_pending_interp_only_mode())` was processed at mounts only but it has to be processed for unmounts as well.
>> - The test `test/hotspot/jtreg/serviceability/jvmti/vthread/MethodExitTest/libMethodExitTest.cpp` has a wrong assumption that there can't be `MethodExit` event on the carrier thread when the function `breakpoint_hit1` is being executed. However, it can happen if the virtual thread gets unmounted.
>>
>> The fix also includes new test case `vthread/CarrierThreadEventNotification` developed by Patricio.
>>
>> Testing:
>> - Ran new test case locally
>> - Ran mach5 tiers 1-6
>
> test/hotspot/jtreg/serviceability/jvmti/vthread/CarrierThreadEventNotification/CarrierThreadEventNotification.java line 2:
>
>> 1: /*
>> 2: * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
>
> (c) 2024
Fixed, thanks.
> test/hotspot/jtreg/serviceability/jvmti/vthread/CarrierThreadEventNotification/libCarrierThreadEventNotification.cpp line 2:
>
>> 1: /*
>> 2: * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
>
> (c) 2024
Fixed, thanks.
> test/hotspot/jtreg/serviceability/jvmti/vthread/CarrierThreadEventNotification/libCarrierThreadEventNotification.cpp line 40:
>
>> 38:
>> 39: static const char* CTHREAD_NAME_START = "ForkJoinPool";
>> 40: static const int CTHREAD_NAME_START_LEN = (int)strlen("ForkJoinPool");
>
> should be `size_t` (the value is used for `strncmp` which expects `size_t`)
Fixed, thanks.
> test/hotspot/jtreg/serviceability/jvmti/vthread/CarrierThreadEventNotification/libCarrierThreadEventNotification.cpp line 44:
>
>> 42: static jint
>> 43: get_cthreads(JNIEnv* jni, jthread** cthreads_p) {
>> 44: jthread* tested_cthreads = NULL;
>
> Suggestion:
>
> jthread* tested_cthreads = nullptr;
Fixed, thanks.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19438#discussion_r1619716426
PR Review Comment: https://git.openjdk.org/jdk/pull/19438#discussion_r1619716604
PR Review Comment: https://git.openjdk.org/jdk/pull/19438#discussion_r1619717881
PR Review Comment: https://git.openjdk.org/jdk/pull/19438#discussion_r1619718490
More information about the serviceability-dev
mailing list