On Wed, 25 May 2022 07:23:36 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:
> A part of this issue was contributed with the following changeset: > > commit ea23e7333e03abb4aca3e9f3854bab418a4b70e2 > Author: Daniel D. Daugherty <[dcu...@openjdk.org](mailto:dcu...@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`. Before Loom, there was a TLH in the outer JVM/TI SuspendThread() entry point so the current thread was already protected. Here's the relevant code from build/macosx-x86_64-normal-server-release/hotspot/variant-server/gensrc/jvmtifiles/jvmtiEnter.cpp in JDK18: static jvmtiError JNICALL jvmti_SuspendThread(jvmtiEnv* env, jthread thread) { #if !INCLUDE_JVMTI return JVMTI_ERROR_NOT_AVAILABLE; #else if(!JvmtiEnv::is_vm_live()) { return JVMTI_ERROR_WRONG_PHASE; } Thread* this_thread = Thread::current_or_null(); if (this_thread == NULL || !this_thread->is_Java_thread()) { return JVMTI_ERROR_UNATTACHED_THREAD; } JavaThread* current_thread = JavaThread::cast(this_thread); MACOS_AARCH64_ONLY(ThreadWXEnable __wx(WXWrite, current_thread)); ThreadInVMfromNative __tiv(current_thread); VM_ENTRY_BASE(jvmtiError, jvmti_SuspendThread , current_thread) debug_only(VMNativeEntryWrapper __vew;) PreserveExceptionMark __em(this_thread); JvmtiEnv* jvmti_env = JvmtiEnv::JvmtiEnv_from_jvmti_env(env); if (!jvmti_env->is_valid()) { return JVMTI_ERROR_INVALID_ENVIRONMENT; } if (jvmti_env->get_capabilities()->can_suspend == 0) { return JVMTI_ERROR_MUST_POSSESS_CAPABILITY; } jvmtiError err; JavaThread* java_thread = NULL; ThreadsListHandle tlh(this_thread); if (thread == NULL) { java_thread = current_thread; } else { err = JvmtiExport::cv_external_thread_to_JavaThread(tlh.list(), thread, &java_thread, NULL); if (err != JVMTI_ERROR_NONE) { return err; } } err = jvmti_env->SuspendThread(java_thread); return err; #endif // INCLUDE_JVMTI } so it was definitely my intent that the current thread be protected by the TLH that was in the entry code. Yes, that protection is not needed, but that's the way I implemented it when is_JavaThread_protected_by_TLH() was added to the system. ------------- PR: https://git.openjdk.java.net/jdk/pull/8878
