RFR: 8303086: SIGSEGV in JavaThread::is_interp_only_mode()

[Serguei Spitsyn]

On Sun, 2 Jul 2023 22:39:46 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> The JVMTI function `SetEventNotificationMode` can set notification mode globally (`event_thread == nullptr`) for all threads or for a specific thread (`event_thread != nullptr`). To get a stable mount/unmount vision of virtual threads a JvmtiVTMSTransitionDisabler helper object is created :
>>     `JvmtiVTMSTransitionDisabler disabler(event_thread);`
>> 
>> In a case if `event_thread == nullptr` the VTMS transitions are disabled for all virtual thread,
>> otherwise they are disabled for a specific thread if it is virtual.
>> The call to `JvmtiEventController::set_user_enabled()` makes a call to `recompute_enabled()` at the end of its work to do a required bookkeeping. As part of this work, the `recompute_thread_enabled(state)` is called for each thread from the `ThreadsListHandle`, not only for the given `event_thread`:
>> 
>>     ThreadsListHandle tlh;
>>     for (; state != nullptr; state = state->next()) {
>>       any_env_thread_enabled |= recompute_thread_enabled(state);
>>     }
>> 
>> This can cause crashes as VTMS transitions for other virtual threads are allowed.
>> Crashes are observed in this small function:
>> 
>>   bool is_interp_only_mode() {
>>     return _thread == nullptr ? _saved_interp_only_mode != 0 : _thread->is_interp_only_mode();
>>   }
>> 
>> In a case `_thread != nullptr` then the call needs to be executed: `_thread->is_interp_only_mode()`.
>> But the filed `_thread` can be already changed to `nullptr` by a VTMS transition.
>> 
>> The fix is to always disable all transitions.
>> Thanks to Dan and Patricio for great analysis of this crash!
>> 
>> Testing:
>> - In progress: mach5 tiers 1-6
>
> src/hotspot/share/prims/jvmtiEnv.cpp line 578:
> 
>> 576:     record_class_file_load_hook_enabled();
>> 577:   }
>> 578:   JvmtiVTMSTransitionDisabler disabler;
> 
> An explanatory comment would have been good.

The fix has been already integrated.
I'll add a comment when there is a chance.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14728#discussion_r1253537846