RFR: 8329674: JvmtiEnvThreadState::reset_current_location function should use JvmtiHandshake [v2]

[Serguei Spitsyn]

On Thu, 11 Apr 2024 22:54:16 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> src/hotspot/share/prims/jvmtiEnvThreadState.cpp line 367:
>> 
>>> 365:       GetCurrentLocationClosure op;
>>> 366:       JvmtiHandshake::execute(&op, &tlh, thread, thread_h);
>>> 367: 
>> 
>> Seems we are missing a JvmtiVTMSTransitionDisabler.
>
> Good question, thanks.
> The `JvmtiVTMSTransitionDisabler` is supposed to be installed in the caller's context if needed.
> However, it is not easy to make sure it is always the case.
> At least, I see a couple of contexts when the `JvmtiVTMSTransitionDisabler` is not being installed.
> But it is not clear if it is really needed there. Let me do some extra analysis there.

Okay.  The class `GetCurrentLocationClosure` is used by the `reset_current_location` only. It is called for the SINGLE_STEP and REAKPOINT event types as the following assert is placed at the function start:

void JvmtiEnvThreadState::reset_current_location(jvmtiEvent event_type, bool enabled) {
  assert(event_type == JVMTI_EVENT_SINGLE_STEP || event_type == JVMTI_EVENT_BREAKPOINT,
         "must be single-step or breakpoint event");
 . . .

Also, this is the only two places where this function is called:

JvmtiEventControllerPrivate::recompute_env_thread_enabled(JvmtiEnvThreadState* ets, JvmtiThreadState* state) {
    . . .
    if (changed & SINGLE_STEP_BIT) {
      ets->reset_current_location(JVMTI_EVENT_SINGLE_STEP, (now_enabled & SINGLE_STEP_BIT) != 0);
    }
    if (changed & BREAKPOINT_BIT) {
      ets->reset_current_location(JVMTI_EVENT_BREAKPOINT,  (now_enabled & BREAKPOINT_BIT) != 0);
    }

The `reset_current_location` is called called in the context of the `SetEventNotificationMode` where a JvmtiVTMSTransitionDisabler is present.

Theoretically, it can be also triggered by the `SetEventCallbacks` (if callbacks are for SINGLE_STEP or REAKPOINT event type). But it also has a J`vmtiVTMSTransitionDisabler` in place:

JvmtiEnv::SetEventCallbacks(const jvmtiEventCallbacks* callbacks, jint size_of_callbacks) {
  JvmtiVTMSTransitionDisabler disabler;
  JvmtiEventController::set_event_callbacks(this, callbacks, size_of_callbacks);
  return JVMTI_ERROR_NONE;
} /* end SetEventCallbacks */

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18630#discussion_r1561903953