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

[Patricio Chilano Mateo]

On Thu, 30 May 2024 02:31:29 GMT, Serguei Spitsyn <sspitsyn 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
>
> Serguei Spitsyn has updated the pull request incrementally with one additional commit since the last revision:
> 
>   review: addressed nits in new test

Hi Serguei,

Thanks for fixing this one.

src/hotspot/share/prims/jvmtiThreadState.cpp line 674:

> 672:   }
> 673:   // enable interp_only_mode for carrier thread if it has pending bit
> 674:   process_pending_interp_only(thread);

So for the last unmount case we will call this before doing the JVMTI state rebinding, but shouldn't it be called after it in VTMS_vthread_end? Actually why not moving this call inside rebind_to_jvmti_thread_state_of()?

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

PR Review: https://git.openjdk.org/jdk/pull/19438#pullrequestreview-2089163431
PR Review Comment: https://git.openjdk.org/jdk/pull/19438#discussion_r1621298531