Skip to content

Commit

Permalink
Merge pull request #17335 from eclipse-openj9/revert-16943-suspend_re…
Browse files Browse the repository at this point in the history 
…sume_status

Revert "Fix to handle suspend/resume of virtual/carrier threads"
  • Loading branch information
@pshipton
pshipton committed May 4, 2023
2 parents 0609eb4 + 1d7b45e commit c2a95b8
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 84 deletions.
19 changes: 5 additions & 14 deletions runtime/j9vm/javanextvmi.cpp
View file
Expand Up @@ -334,13 +334,13 @@ JVM_VirtualThreadMountEnd(JNIEnv *env, jobject thread, jboolean firstMount)
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 isSuspendedByJVMTI is non-zero, J9_PUBLIC_FLAGS_HALT_THREAD_JAVA_SUSPEND is set
* in currentThread->publicFlags while resetting isSuspendedByJVMTI to zero. During
* mount, this suspends the thread if the thread was unmounted when JVMTI suspended it.
*/
if (0 != J9OBJECT_U32_LOAD(currentThread, threadObj, vm->isSuspendedInternalOffset)) {
Assert_SC_true(threadObj == currentThread->threadObject);
if (0 != J9OBJECT_U32_LOAD(currentThread, threadObj, vm->isSuspendedByJVMTIOffset)) {
vmFuncs->setHaltFlag(currentThread, J9_PUBLIC_FLAGS_HALT_THREAD_JAVA_SUSPEND);
J9OBJECT_U32_STORE(currentThread, threadObj, vm->isSuspendedByJVMTIOffset, 0);
}

/* Allow thread to be inspected again. */
Expand Down Expand Up @@ -430,15 +430,6 @@ JVM_VirtualThreadUnmountEnd(JNIEnv *env, jobject thread, jboolean lastUnmount)
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);

Expand Down
4 changes: 2 additions & 2 deletions runtime/jcl/common/jclcinit.c
View file
Expand Up @@ -655,8 +655,8 @@ initializeRequiredClasses(J9VMThread *vmThread, char* dllName)
return 1;
}

/* Stores a non-zero value if the virtual or carrier thread is suspended by JVMTI. */
if (0 != vmFuncs->addHiddenInstanceField(vm, "java/lang/Thread", "isSuspendedInternal", "I", &vm->isSuspendedInternalOffset)) {
/* Stores a non-zero value if the virtual thread is suspended by JVMTI. */
if (0 != vmFuncs->addHiddenInstanceField(vm, "java/lang/VirtualThread", "isSuspendedByJVMTI", "I", &vm->isSuspendedByJVMTIOffset)) {
return 1;
}
#endif /* JAVA_SPEC_VERSION >= 19 */
Expand Down
34 changes: 10 additions & 24 deletions runtime/jcl/common/thread.cpp
View file
Expand Up @@ -197,15 +197,7 @@ Java_java_lang_Thread_resumeImpl(JNIEnv *env, jobject rcv)
Trc_JCL_threadResume(currentThread, targetThread);
if (J9VMJAVALANGTHREAD_STARTED(currentThread, receiverObject)) {
if (NULL != targetThread) {
#if JAVA_SPEC_VERSION >= 19
if (receiverObject == targetThread->threadObject)
#endif /* JAVA_SPEC_VERSION >= 19 */
{
vmFuncs->clearHaltFlag(targetThread, J9_PUBLIC_FLAGS_HALT_THREAD_JAVA_SUSPEND);
}
#if JAVA_SPEC_VERSION >= 19
J9OBJECT_U32_STORE(currentThread, receiverObject, vm->isSuspendedInternalOffset, 0);
#endif /* JAVA_SPEC_VERSION >= 19 */
vmFuncs->clearHaltFlag(targetThread, J9_PUBLIC_FLAGS_HALT_THREAD_JAVA_SUSPEND);
}
}
vmFuncs->internalExitVMToJNI(currentThread);
Expand All @@ -224,24 +216,18 @@ Java_java_lang_Thread_suspendImpl(JNIEnv *env, jobject rcv)
Trc_JCL_threadSuspend(currentThread, targetThread);
if (J9VMJAVALANGTHREAD_STARTED(currentThread, receiverObject)) {
if (NULL != targetThread) {
#if JAVA_SPEC_VERSION >= 19
J9OBJECT_U32_STORE(currentThread, receiverObject, vm->isSuspendedInternalOffset, 1);
if (receiverObject == targetThread->threadObject)
#endif /* JAVA_SPEC_VERSION >= 19 */
{
if (currentThread == targetThread) {
if (currentThread == targetThread) {
/* Suspending the current thread will take place upon re-entering the VM after returning from
* this native.
*/
vmFuncs->setHaltFlag(targetThread, J9_PUBLIC_FLAGS_HALT_THREAD_JAVA_SUSPEND);
} else {
vmFuncs->internalExitVMToJNI(currentThread);
omrthread_monitor_enter(targetThread->publicFlagsMutex);
VM_VMAccess::setHaltFlagForVMAccessRelease(targetThread, J9_PUBLIC_FLAGS_HALT_THREAD_JAVA_SUSPEND);
if (VM_VMAccess::mustWaitForVMAccessRelease(targetThread)) {
while (J9_ARE_ALL_BITS_SET(targetThread->publicFlags, J9_PUBLIC_FLAGS_HALT_THREAD_JAVA_SUSPEND | J9_PUBLIC_FLAGS_VM_ACCESS)) {
omrthread_monitor_wait(targetThread->publicFlagsMutex);
}
vmFuncs->setHaltFlag(targetThread, J9_PUBLIC_FLAGS_HALT_THREAD_JAVA_SUSPEND);
} else {
vmFuncs->internalExitVMToJNI(currentThread);
omrthread_monitor_enter(targetThread->publicFlagsMutex);
VM_VMAccess::setHaltFlagForVMAccessRelease(targetThread, J9_PUBLIC_FLAGS_HALT_THREAD_JAVA_SUSPEND);
if (VM_VMAccess::mustWaitForVMAccessRelease(targetThread)) {
while (J9_ARE_ALL_BITS_SET(targetThread->publicFlags, J9_PUBLIC_FLAGS_HALT_THREAD_JAVA_SUSPEND | J9_PUBLIC_FLAGS_VM_ACCESS)) {
omrthread_monitor_wait(targetThread->publicFlagsMutex);
}
}
omrthread_monitor_exit(targetThread->publicFlagsMutex);
Expand Down
24 changes: 5 additions & 19 deletions runtime/jvmti/jvmtiHelpers.c
View file
Expand Up @@ -722,16 +722,6 @@ getThreadState(J9VMThread *currentThread, j9object_t threadObject)
if (vmstate & J9VMTHREAD_STATE_SUSPENDED) {
state |= JVMTI_THREAD_STATE_SUSPENDED;
}
#if JAVA_SPEC_VERSION >= 19
/* Based on the isSuspendedInternal field, set the JVMTI
* thread state to suspended for the corresponding thread.
*/
if (0 != J9OBJECT_U32_LOAD(currentThread, threadObject, currentThread->javaVM->isSuspendedInternalOffset)) {
state |= JVMTI_THREAD_STATE_SUSPENDED;
} else {
state &= ~JVMTI_THREAD_STATE_SUSPENDED;
}
#endif /* JAVA_SPEC_VERSION >= 19 */
if (vmstate & J9VMTHREAD_STATE_INTERRUPTED) {
state |= JVMTI_THREAD_STATE_INTERRUPTED;
}
Expand Down Expand Up @@ -794,12 +784,12 @@ getVirtualThreadState(J9VMThread *currentThread, jthread thread)
currentThread, thread, &targetThread, JVMTI_ERROR_NONE,
J9JVMTI_GETVMTHREAD_ERROR_ON_NULL_JTHREAD);
if (JVMTI_ERROR_NONE == rc) {
j9object_t vThreadObject = J9_JNI_UNWRAP_REFERENCE(thread);
if (NULL != targetThread) {
vm->internalVMFunctions->haltThreadForInspection(currentThread, targetThread);
rc = getThreadState(currentThread, targetThread->carrierThreadObject);
vm->internalVMFunctions->resumeThreadForInspection(currentThread, targetThread);
} else {
j9object_t vThreadObject = J9_JNI_UNWRAP_REFERENCE(thread);
jint vThreadState = (jint) J9VMJAVALANGVIRTUALTHREAD_STATE(currentThread, vThreadObject);
/* The mapping from JVMTI_VTHREAD_STATE_XXX to JVMTI_JAVA_LANG_THREAD_STATE_XXX is based
* on j.l.VirtualThread.threadState().
Expand Down Expand Up @@ -827,7 +817,7 @@ getVirtualThreadState(J9VMThread *currentThread, jthread thread)
rc = JVMTI_JAVA_LANG_THREAD_STATE_RUNNABLE;
}
vm->internalVMFunctions->internalEnterVMFromJNI(currentThread);
/* Re-fetch object to correctly set the isSuspendedInternal field. */
/* Re-fetch object to correctly set the isSuspendedByJVMTI field. */
vThreadObject = J9_JNI_UNWRAP_REFERENCE(thread);
break;
}
Expand Down Expand Up @@ -861,13 +851,9 @@ getVirtualThreadState(J9VMThread *currentThread, jthread thread)
Assert_JVMTI_unreachable();
rc = JVMTI_ERROR_INTERNAL;
}
}
/* Re-fetch object to correctly set the isSuspendedInternal field. */
vThreadObject = J9_JNI_UNWRAP_REFERENCE(thread);
if (0 != J9OBJECT_U32_LOAD(currentThread, vThreadObject, vm->isSuspendedInternalOffset)) {
rc |= JVMTI_THREAD_STATE_SUSPENDED;
} else {
rc &= ~JVMTI_THREAD_STATE_SUSPENDED;
if (0 != J9OBJECT_U32_LOAD(currentThread, vThreadObject, vm->isSuspendedByJVMTIOffset)) {
rc |= JVMTI_THREAD_STATE_SUSPENDED;
}
}
releaseVMThread(currentThread, targetThread, thread);
} else {
Expand Down
35 changes: 21 additions & 14 deletions runtime/jvmti/jvmtiThread.c
View file
Expand Up @@ -1118,13 +1118,8 @@ resumeThread(J9VMThread *currentThread, jthread thread)
J9JVMTI_GETVMTHREAD_ERROR_ON_NULL_JTHREAD | J9JVMTI_GETVMTHREAD_ERROR_ON_DEAD_THREAD);
if (JVMTI_ERROR_NONE == rc) {
#if JAVA_SPEC_VERSION >= 19
J9JavaVM *vm = currentThread->javaVM;
j9object_t threadObject = J9_JNI_UNWRAP_REFERENCE(thread);
/* The J9_PUBLIC_FLAGS_HALT_THREAD_JAVA_SUSPEND will be cleared only
* if the thread is resumed and it has been mounted
* i.e. threadObject == targetThread->threadObject.
*/
if ((NULL != targetThread) && (threadObject == targetThread->threadObject))
if (NULL != targetThread)
#endif /* JAVA_SPEC_VERSION >= 19 */
{
if (OMR_ARE_ANY_BITS_SET(targetThread->publicFlags, J9_PUBLIC_FLAGS_HALT_THREAD_JAVA_SUSPEND)) {
Expand All @@ -1135,10 +1130,14 @@ resumeThread(J9VMThread *currentThread, jthread thread)
}
}
#if JAVA_SPEC_VERSION >= 19
if (0 == J9OBJECT_U32_LOAD(currentThread, threadObject, vm->isSuspendedInternalOffset)) {
rc = JVMTI_ERROR_THREAD_NOT_SUSPENDED;
} else {
J9OBJECT_U32_STORE(currentThread, threadObject, vm->isSuspendedInternalOffset, 0);
else {
/* targetThread is NULL only for virtual threads as per the assertion in getVMThread. */
J9JavaVM *vm = currentThread->javaVM;
if (0 == J9OBJECT_U32_LOAD(currentThread, threadObject, vm->isSuspendedByJVMTIOffset)) {
rc = JVMTI_ERROR_THREAD_NOT_SUSPENDED;
} else {
J9OBJECT_U32_STORE(currentThread, threadObject, vm->isSuspendedByJVMTIOffset, 0);
}
}
#endif /* JAVA_SPEC_VERSION >= 19 */
releaseVMThread(currentThread, targetThread, thread);
Expand Down Expand Up @@ -1325,22 +1324,30 @@ jvmtiSuspendResumeCallBack(J9VMThread *vmThread, J9MM_IterateObjectDescriptor *o
Assert_JVMTI_notNull(targetThread);
}
if (data->is_suspend) {
if ((NULL != targetThread) && (vthread == targetThread->threadObject)) {
if (NULL != targetThread) {
if (OMR_ARE_NO_BITS_SET(targetThread->publicFlags, J9_PUBLIC_FLAGS_HALT_THREAD_JAVA_SUSPEND | J9_PUBLIC_FLAGS_STOPPED)) {
setHaltFlag(targetThread, J9_PUBLIC_FLAGS_HALT_THREAD_JAVA_SUSPEND);
Trc_JVMTI_threadSuspended(targetThread);
}
} else {
/* NULL carrier thread imply the virtual threadis unmounted. */
if (0 == J9OBJECT_U32_LOAD(vmThread, vthread, vm->isSuspendedByJVMTIOffset)) {
J9OBJECT_U32_STORE(vmThread, vthread, vm->isSuspendedByJVMTIOffset, 1);
}
}
J9OBJECT_U32_STORE(vmThread, vthread, vm->isSuspendedInternalOffset, 1);
} else {
/* Resume the virtual thread. */
if ((NULL != targetThread) && (vthread == targetThread->threadObject)) {
if (NULL != targetThread) {
if (OMR_ARE_ANY_BITS_SET(targetThread->publicFlags, J9_PUBLIC_FLAGS_HALT_THREAD_JAVA_SUSPEND)) {
clearHaltFlag(targetThread, J9_PUBLIC_FLAGS_HALT_THREAD_JAVA_SUSPEND);
Trc_JVMTI_threadResumed(targetThread);
}
} else {
/* targetThread is NULL only for virtual threads. */
if (0 != J9OBJECT_U32_LOAD(currentThread, vthread, vm->isSuspendedByJVMTIOffset)) {
J9OBJECT_U32_STORE(currentThread, vthread, vm->isSuspendedByJVMTIOffset, 0);
}
}
J9OBJECT_U32_STORE(currentThread, vthread, vm->isSuspendedInternalOffset, 0);
}
}
}
Expand Down
18 changes: 8 additions & 10 deletions runtime/jvmti/suspendhelper.cpp
View file
Expand Up @@ -42,10 +42,7 @@ suspendThread(J9VMThread *currentThread, jthread thread, BOOLEAN allowNull, BOOL
J9JavaVM *vm = currentThread->javaVM;
#if JAVA_SPEC_VERSION >= 19
j9object_t threadObject = (NULL == thread) ? currentThread->threadObject : J9_JNI_UNWRAP_REFERENCE(thread);
/* The J9 PUBLIC FLAGS HALT THREAD JAVA SUSPEND flag will be set
* if the thread is mounted.
*/
if ((NULL != targetThread) && (threadObject == targetThread->threadObject))
if (NULL != targetThread)
#endif /* JAVA_SPEC_VERSION >= 19 */
{
if (OMR_ARE_ANY_BITS_SET(targetThread->publicFlags, J9_PUBLIC_FLAGS_HALT_THREAD_JAVA_SUSPEND)) {
Expand Down Expand Up @@ -74,12 +71,13 @@ suspendThread(J9VMThread *currentThread, jthread thread, BOOLEAN allowNull, BOOL
}
}
#if JAVA_SPEC_VERSION >= 19
/* Re-fetch object to correctly set the field since VM access was re-acquired. */
threadObject = (NULL == thread) ? currentThread->threadObject : J9_JNI_UNWRAP_REFERENCE(thread);
if (0 != J9OBJECT_U32_LOAD(currentThread, threadObject, vm->isSuspendedInternalOffset)) {
rc = JVMTI_ERROR_THREAD_SUSPENDED;
} else {
J9OBJECT_U32_STORE(currentThread, threadObject, vm->isSuspendedInternalOffset, 1);
else {
/* targetThread is NULL only for virtual threads as per the assertion in getVMThread. */
if (0 != J9OBJECT_U32_LOAD(currentThread, threadObject, vm->isSuspendedByJVMTIOffset)) {
rc = JVMTI_ERROR_THREAD_SUSPENDED;
} else {
J9OBJECT_U32_STORE(currentThread, threadObject, vm->isSuspendedByJVMTIOffset, 1);
}
}
#endif /* JAVA_SPEC_VERSION >= 19 */
releaseVMThread(currentThread, targetThread, thread);
Expand Down
2 changes: 1 addition & 1 deletion runtime/oti/j9nonbuilder.h
View file
Expand Up @@ -5905,7 +5905,7 @@ typedef struct J9JavaVM {
#if JAVA_SPEC_VERSION >= 19
U_64 nextTID;
UDATA virtualThreadInspectorCountOffset;
UDATA isSuspendedInternalOffset;
UDATA isSuspendedByJVMTIOffset;
UDATA tlsOffset;
j9_tls_finalizer_t tlsFinalizers[J9JVMTI_MAX_TLS_KEYS];
omrthread_monitor_t tlsFinalizersMutex;
Expand Down

0 comments on commit c2a95b8

Please sign in to comment.