From feac35606f37bcb0ec321362fc3964eb4f07f137 Mon Sep 17 00:00:00 2001 From: Babneet Singh Date: Thu, 9 Nov 2023 21:47:31 -0500 Subject: [PATCH] Spin during VirtualThread MountBegin and UnmountBegin Currently, the halt flag is set in VirtualThread MountEnd if a virtual thread is suspended via JVMTI, and in VirtualThread UnmountEnd if a carrier thread is suspended via JVMTI. In the above approach, the halt flag is set too late. As soon as the continuation swaps the J9VMThread context, the thread begins execution and is capable of triggering JVMTI events. To avoid the above issue, the above steps are moved into VirtualThread MountBegin and UnmountBegin. This prevents the continuation to swap the J9VMThread context. Currently, the halt flag is set without invoking exitVThreadTransitionCritical. This prevents JVMTI to resume the halted thread and cause a hang. The new approach spins, invokes exitVThreadTransitionCritical and releases VM access to allow JVMTI to resume the suspended thread. The better approach will be to fail mount if the thread is suspended and retry later. Currently, his approach cannot be implemented because VirtualThread.java does not support this approach. Related: eclipse-openj9#17865 Related: eclipse-openj9#17869 Related: eclipse-openj9#18370 Signed-off-by: Babneet Singh --- runtime/j9vm/javanextvmi.cpp | 63 +++++++++++++++++++++++++----------- runtime/j9vm/jvm.c | 8 +++-- runtime/jvmti/jvmtiThread.c | 2 +- 3 files changed, 52 insertions(+), 21 deletions(-) diff --git a/runtime/j9vm/javanextvmi.cpp b/runtime/j9vm/javanextvmi.cpp index a50c0b07665..dddc3d45479 100644 --- a/runtime/j9vm/javanextvmi.cpp +++ b/runtime/j9vm/javanextvmi.cpp @@ -40,6 +40,8 @@ extern "C" { #if JAVA_SPEC_VERSION >= 19 extern J9JavaVM *BFUjavaVM; + +extern IDATA (*f_threadSleep)(I_64 millis); #endif /* JAVA_SPEC_VERSION >= 19 */ /* Define for debug @@ -328,6 +330,27 @@ virtualThreadMountBegin(JNIEnv *env, jobject thread) } enterVThreadTransitionCritical(currentThread, thread); + + /* Virtual thread is being mounted but it has been suspended. Spin until the + * virtual thread is resumed. The virtual thread should not be mounted until + * it is resumed. + */ + J9JavaVM *vm = currentThread->javaVM; + J9InternalVMFunctions *vmFuncs = vm->internalVMFunctions; + threadObj = J9_JNI_UNWRAP_REFERENCE(thread); + while (0 != J9OBJECT_U32_LOAD(currentThread, threadObj, vm->isSuspendedInternalOffset)) { + exitVThreadTransitionCritical(currentThread, threadObj); + vmFuncs->internalReleaseVMAccess(currentThread); + /* Spin is used instead of the halt flag; otherwise, the carrier thread will + * show as suspended. + * + * TODO: Dynamically increase the sleep time to a bounded maximum. + */ + f_threadSleep(10); + vmFuncs->internalAcquireVMAccess(currentThread); + enterVThreadTransitionCritical(currentThread, thread); + threadObj = J9_JNI_UNWRAP_REFERENCE(thread); + } } /* Caller must have VMAccess. */ @@ -354,15 +377,6 @@ virtualThreadMountEnd(JNIEnv *env, jobject thread) J9VMJDKINTERNALVMCONTINUATION_VMREF(currentThread, continuationObj)); } - /* Virtual thread is being mounted but it has been suspended. Thus, - * set J9_PUBLIC_FLAGS_HALT_THREAD_JAVA_SUSPEND flag. At this - * point, virtual thread object is stored in targetThread->threadObject. - */ - if (0 != J9OBJECT_U32_LOAD(currentThread, threadObj, vm->isSuspendedInternalOffset)) { - Assert_SC_true(threadObj == currentThread->threadObject); - vm->internalVMFunctions->setHaltFlag(currentThread, J9_PUBLIC_FLAGS_HALT_THREAD_JAVA_SUSPEND); - } - /* Allow thread to be inspected again. */ exitVThreadTransitionCritical(currentThread, threadObj); @@ -396,6 +410,28 @@ virtualThreadUnmountBegin(JNIEnv *env, jobject thread) enterVThreadTransitionCritical(currentThread, thread); VM_VMHelpers::virtualThreadHideFrames(currentThread, JNI_TRUE); + + J9InternalVMFunctions *vmFuncs = vm->internalVMFunctions; + j9object_t carrierThreadObject = currentThread->carrierThreadObject; + threadObj = J9_JNI_UNWRAP_REFERENCE(thread); + /* Virtual thread is being umounted. If its carrier thread is suspended, spin until + * the carrier thread is resumed. The carrier thread should not be mounted until it + * is resumed. + */ + while (0 != J9OBJECT_U32_LOAD(currentThread, carrierThreadObject, vm->isSuspendedInternalOffset)) { + exitVThreadTransitionCritical(currentThread, threadObj); + vmFuncs->internalReleaseVMAccess(currentThread); + /* Spin is used instead of the halt flag; otherwise, the virtual thread will + * show as suspended. + * + * TODO: Dynamically increase the sleep time to a bounded maximum. + */ + f_threadSleep(10); + vmFuncs->internalAcquireVMAccess(currentThread); + enterVThreadTransitionCritical(currentThread, thread); + carrierThreadObject = currentThread->carrierThreadObject; + threadObj = J9_JNI_UNWRAP_REFERENCE(thread); + } } /* Caller must have VMAccess. */ @@ -429,15 +465,6 @@ virtualThreadUnmountEnd(JNIEnv *env, jobject thread) vmFuncs->freeTLS(currentThread, threadObj); } - j9object_t carrierThreadObject = currentThread->carrierThreadObject; - /* The J9_PUBLIC_FLAGS_HALT_THREAD_JAVA_SUSPEND will be set for the virtual - * thread's carrier thread if it was suspended while the virtual thread was mounted. - */ - if (0 != J9OBJECT_U32_LOAD(currentThread, carrierThreadObject, vm->isSuspendedInternalOffset)) { - Assert_SC_true((currentThread->threadObject == carrierThreadObject) && (NULL == currentThread->currentContinuation)); - vmFuncs->setHaltFlag(currentThread, J9_PUBLIC_FLAGS_HALT_THREAD_JAVA_SUSPEND); - } - /* Allow thread to be inspected again. */ exitVThreadTransitionCritical(currentThread, threadObj); } diff --git a/runtime/j9vm/jvm.c b/runtime/j9vm/jvm.c index 2af20c3f06e..168c8161b3e 100644 --- a/runtime/j9vm/jvm.c +++ b/runtime/j9vm/jvm.c @@ -212,6 +212,7 @@ typedef IDATA (*MonitorDestroy)(omrthread_monitor_t monitor); typedef IDATA (* ThreadLibControl)(const char * key, UDATA value); typedef IDATA (*SetCategory)(omrthread_t thread, UDATA category, UDATA type); typedef IDATA (*LibEnableCPUMonitor)(omrthread_t thread); +typedef IDATA (*ThreadSleep)(I_64 millis); typedef I_32 (*PortInitLibrary)(J9PortLibrary *portLib, J9PortLibraryVersion *version, UDATA size); typedef UDATA (*PortGetSize)(struct J9PortLibraryVersion *version); @@ -260,6 +261,7 @@ static pNewStringPlatform globalNewStringPlatform; static p_a2e_vsprintf global_a2e_vsprintf; #endif +ThreadSleep f_threadSleep; static ThreadGlobal f_threadGlobal; static ThreadAttachEx f_threadAttachEx; static ThreadDetach f_threadDetach; @@ -1015,8 +1017,9 @@ preloadLibraries(void) f_threadLibControl = (ThreadLibControl) GetProcAddress (threadDLL, (LPCSTR) "omrthread_lib_control"); f_setCategory = (SetCategory) GetProcAddress (threadDLL, (LPCSTR) "omrthread_set_category"); f_libEnableCPUMonitor = (LibEnableCPUMonitor) GetProcAddress (threadDLL, (LPCSTR) "omrthread_lib_enable_cpu_monitor"); + f_threadSleep = (ThreadSleep) GetProcAddress (threadDLL, (LPCSTR) "omrthread_sleep"); if (!f_threadGlobal || !f_threadAttachEx || !f_threadDetach || !f_monitorEnter || !f_monitorExit || !f_monitorInit - || !f_monitorDestroy || !f_threadLibControl || !f_setCategory || !f_libEnableCPUMonitor + || !f_monitorDestroy || !f_threadLibControl || !f_setCategory || !f_libEnableCPUMonitor || !f_threadSleep ) { FreeLibrary(vmDLL); FreeLibrary(threadDLL); @@ -1442,8 +1445,9 @@ preloadLibraries(void) f_threadLibControl = (ThreadLibControl) dlsym (threadDLL, "omrthread_lib_control"); f_setCategory = (SetCategory) dlsym (threadDLL, "omrthread_set_category"); f_libEnableCPUMonitor = (LibEnableCPUMonitor) dlsym (threadDLL, "omrthread_lib_enable_cpu_monitor"); + f_threadSleep = (ThreadSleep) dlsym (threadDLL, "omrthread_sleep"); if (!f_threadGlobal || !f_threadAttachEx || !f_threadDetach || !f_monitorEnter || !f_monitorExit || !f_monitorInit - || !f_monitorDestroy || !f_threadLibControl || !f_setCategory || !f_libEnableCPUMonitor + || !f_monitorDestroy || !f_threadLibControl || !f_setCategory || !f_libEnableCPUMonitor || !f_threadSleep ) { dlclose(vmDLL); #ifdef J9ZOS390 diff --git a/runtime/jvmti/jvmtiThread.c b/runtime/jvmti/jvmtiThread.c index 9b322cbd72a..16c159d36ee 100644 --- a/runtime/jvmti/jvmtiThread.c +++ b/runtime/jvmti/jvmtiThread.c @@ -1326,7 +1326,7 @@ jvmtiSuspendResumeCallBack(J9VMThread *vmThread, J9MM_IterateObjectDescriptor *o { j9object_t continuationObj = object->object; j9object_t vthread = J9VMJDKINTERNALVMCONTINUATION_VTHREAD(vmThread, continuationObj); - ContinuationState continuationState = J9VMJDKINTERNALVMCONTINUATION_STATE(vmThread, continuationObj);; + ContinuationState continuationState = J9VMJDKINTERNALVMCONTINUATION_STATE(vmThread, continuationObj); if ((NULL != vthread) && J9_ARE_NO_BITS_SET(continuationState, J9_GC_CONTINUATION_STATE_LAST_UNMOUNT)) { jvmtiVThreadCallBackData *data = (jvmtiVThreadCallBackData*)userData;