On Fri, 30 Jun 2023 11:27:58 GMT, Serguei Spitsyn <sspit...@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

Marked as reviewed by cjplummer (Reviewer).

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

PR Review: https://git.openjdk.org/jdk/pull/14728#pullrequestreview-1507636034

Reply via email to