RFR: 8266593: vmTestbase/nsk/jvmti/PopFrame/popframe011 fails with "assert(java_thread == _state->get_thread()) failed: Must be" [v3]

[Serguei Spitsyn]

On Thu, 18 Nov 2021 17:18:23 GMT, Daniel D. Daugherty <dcubed at openjdk.org> wrote:

>> Serguei Spitsyn has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   get rid of the checks in jvmti handshakes: java_thread->threadObj() == NULL
>
> src/hotspot/share/prims/jvmtiEnvBase.cpp line 1401:
> 
>> 1399:   if (!self) {
>> 1400:     if (!java_thread->is_suspended()) {
>> 1401:       _result = JVMTI_ERROR_THREAD_NOT_SUSPENDED;
> 
> I don't see an obvious reason for this `is_exiting()` check.

Okay. I see similar check in the `force_early_return()` function:

  if (state == NULL) {
    return JVMTI_ERROR_THREAD_NOT_ALIVE;
  }

Would it better to replace it with this check instead? :

  if (java_thread->is_exiting()) {
    return JVMTI_ERROR_THREAD_NOT_ALIVE;
  }

> src/hotspot/share/prims/jvmtiEnvBase.cpp line 1625:
> 
>> 1623:     return; /* JVMTI_ERROR_THREAD_NOT_ALIVE (default) */
>> 1624:   }
>> 1625:   assert(_state->get_thread() == java_thread, "Must be");
> 
> The `assert()` on L1625 is subject to the same race as the original site.
> This `is_exiting()` check is made under the protection of the
> `JvmtiThreadState_lock` so it is sufficient to protect that `assert()`.

Okay, thanks!

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

PR: https://git.openjdk.java.net/jdk/pull/6440