RFR: 8286960: Test serviceability/jvmti/vthread/SuspendResume2 crashed: missing ThreadsListHandle in calling context

[Serguei Spitsyn]

On Wed, 25 May 2022 07:23:36 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

> A part of this issue was contributed with the following changeset:
> 
> commit ea23e7333e03abb4aca3e9f3854bab418a4b70e2
> Author: Daniel D. Daugherty <[dcubed at openjdk.org](mailto:dcubed at openjdk.org)>
> Date: Mon Nov 8 14:45:04 2021 +0000
> 
>     8249004: Reduce ThreadsListHandle overhead in relation to direct handshakes
>     Reviewed-by: coleenp, sspitsyn, dholmes, rehn
> 
> The following change in `src/hotspot/share/runtime/thread.cpp` added new assert:
> 
> bool JavaThread::java_suspend() {
> - ThreadsListHandle tlh;
> - if (!tlh.includes(this)) {
> - log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " not on ThreadsList, no suspension", p2i(this));
> - return false;
> - }
> + guarantee(Thread::is_JavaThread_protected(this, /* checkTLHOnly */ true),
>  + "missing ThreadsListHandle in calling context.");
>   return this->handshake_state()->suspend();
> }
> 
> This new assert misses a check for target thread as being current `JavaThread`.
> 
> Also, the JVMTI SuspendThread is protected with TLH:
> 
> JvmtiEnv::SuspendThread(jthread thread) {
>   JavaThread* current = JavaThread::current();
>   ThreadsListHandle tlh(current);              <= TLS defined here!!!
> 
>    oop thread_oop = NULL;
>    {
>      JvmtiVTMSTransitionDisabler disabler(true); 
> 
> 
> However, it is possible that a new carrier thread (and an associated `JavaThread`) can be created after the `TLH` was set and the target virtual thread can be mounted on new carrier thread. Then target virtual thread will be associated with newly created `JavaThread` which is unprotected by the TLH.
> The right way to be protected from this situation it is to prevent mount state transitions with `JvmtiVTMSTransitionDisabler` before the TLH is set as in the change below:
> 
> 
> @@ -929,13 +929,13 @@ JvmtiEnv::GetAllThreads(jint* threads_count_ptr, jthread** threads_ptr) {
>  jvmtiError
>  JvmtiEnv::SuspendThread(jthread thread) {
>    JavaThread* current = JavaThread::current();
> -  ThreadsListHandle tlh(current);
> 
>    jvmtiError err;
>    JavaThread* java_thread = NULL;
>    oop thread_oop = NULL;
>    {
>      JvmtiVTMSTransitionDisabler disabler(true);
> +    ThreadsListHandle tlh(current);
> 
>      err = get_threadOop_and_JavaThread(tlh.list(), thread, &java_thread, &thread_oop);
>      if (err != JVMTI_ERROR_NONE) {
> 
> 
> 
> This problem exist in all JVMTI Suspend functions:
>  `SuspendThread`, `SuspendThreadList` and `SuspendAllVirtualThreads`.

This nsk.jvmti test is also failing with the guarantee for current platform thread:
 `test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS202/hs202t002`
 

 #  Internal Error (/scratch/sspitsyn/loom5/open/src/hotspot/share/runtime/thread.cpp:1781), pid=7458, tid=7506
#  guarantee(Thread::is_JavaThread_protected_by_TLH( this)) failed: missing ThreadsListHandle in calling context.
#
# JRE version: Java(TM) SE Runtime Environment (19.0) (fastdebug build 19-internal-2022-05-19-0744187.sspitsyn...)
# Java VM: Java HotSpot(TM) 64-Bit Server VM (fastdebug 19-internal-2022-05-19-0744187.sspitsyn..., mixed mode, tiered, compressed oops, compressed class ptrs, g1 gc, linux-amd64)
# Problematic frame:
# V  [libjvm.so+0x1a41814]  JavaThread::java_suspend()+0xa4
. . . . . . . . . . . . . . . .
---------------  T H R E A D  ---------------

Current thread (0x000014b91001d0f0):  JavaThread "Thread-1" [_thread_in_vm, id=7506, stack(0x000014b92bafa000,0x000014b92bbfb000)]

Stack: [0x000014b92bafa000,0x000014b92bbfb000],  sp=0x000014b92bbf8ef0,  free space=1019k
Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
V  [libjvm.so+0x1a41814]  JavaThread::java_suspend()+0xa4
V  [libjvm.so+0x1315a79]  JvmtiEnvBase::suspend_thread(oop, JavaThread*, bool, int*)+0x379
V  [libjvm.so+0x12fb9ca]  JvmtiEnv::SuspendThread(_jobject*)+0x1ba
V  [libjvm.so+0x12a42df]  jvmti_SuspendThread+0x17f
C  [libhs202t002.so+0xc0a7]  callbackMethodExit.part.0+0x107
V  [libjvm.so+0x1336887]  JvmtiExport::post_method_exit_inner(JavaThread*, methodHandle&, JvmtiThreadState*, bool, frame, jvalue&) [clone .part.0]+0x307
V  [libjvm.so+0x133ac71]  JvmtiExport::notice_unwind_due_to_exception(JavaThread*, Method*, unsigned char*, oop, bool)+0x531
V  [libjvm.so+0xfad21a]  InterpreterRuntime::exception_handler_for_exception(JavaThread*, oopDesc*)+0xb9a
j  nsk.jvmti.scenarios.hotswap.HS202.hs202t002.MyThread.display()V+19
j  nsk.jvmti.scenarios.hotswap.HS202.hs202t002.MyThread.playWithThis()V+1
j  nsk.jvmti.scenarios.hotswap.HS202.hs202t002.MyThread.run()V+1
v  ~StubRoutines::call_stub 0x000014b98fa09d47
V  [libjvm.so+0xfba465]  JavaCalls::call_helper(JavaValue*, methodHandle const&, JavaCallArguments*, JavaThread*)+0x505
V  [libjvm.so+0xfbacf4]  JavaCalls::call_virtual(JavaValue*, Klass*, Symbol*, Symbol*, JavaCallArguments*, JavaThread*)+0x4b4
V  [libjvm.so+0xfbb167]  JavaCalls::call_virtual(JavaValue*, Handle, Klass*, Symbol*, Symbol*, JavaThread*)+0x77
V  [libjvm.so+0x113d20b]  thread_entry(JavaThread*, JavaThread*)+0x12b
V  [libjvm.so+0x1a40c6a]  JavaThread::thread_main_inner()+0x21a
V  [libjvm.so+0x1a4e520]  Thread::call_run()+0x100
V  [libjvm.so+0x16fc3a4]  thread_native_entry(Thread*)+0x104

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

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