Skip to content

8311218: fatal error: stuck in JvmtiVTMSTransitionDisabler::VTMS_transition_disable #17011

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 10 commits into from
Closed

8311218: fatal error: stuck in JvmtiVTMSTransitionDisabler::VTMS_transition_disable #17011

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
d85d5ca
8311218: fatal error: stuck in JvmtiVTMSTransitionDisabler::VTMS_tran…
sspitsyn Dec 7, 2023
1b56eea
add new test SuspendWithInterruptLock.java
sspitsyn Dec 7, 2023
64426f8
added @summary to new test SuspendWithInterruptLock.java
sspitsyn Dec 7, 2023
ccba940
Resolved merge conflict in VirtualThread.java
sspitsyn Dec 7, 2023
18f1752
review: (1) rename notifyJvmti method; (2) add try-final statements t…
sspitsyn Dec 8, 2023
4e5f644
review: 1) replace CriticalLock with DisableSuspend; 2) minor tweaks
sspitsyn Dec 14, 2023
ad99042
review: moved notifyJvmtiDisableSuspend(true) out of try-block
sspitsyn Dec 14, 2023
917dc72
review: moved a couple of comments out of try blocks
sspitsyn Dec 14, 2023
6f8cdf0
review: improve an assert message
sspitsyn Dec 15, 2023
9ba94c3
Merge
sspitsyn Dec 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion make/data/hotspot-symbols/symbols-unix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ JVM_VirtualThreadEnd
JVM_VirtualThreadMount
JVM_VirtualThreadUnmount
JVM_VirtualThreadHideFrames
JVM_VirtualThreadSync
JVM_VirtualThreadCriticalLock

# Scoped values
JVM_EnsureMaterializedForStackWalk_func
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/classfile/vmIntrinsics.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,7 @@ class methodHandle;
do_intrinsic(_notifyJvmtiVThreadMount, java_lang_VirtualThread, notifyJvmtiMount_name, bool_void_signature, F_RN) \
do_intrinsic(_notifyJvmtiVThreadUnmount, java_lang_VirtualThread, notifyJvmtiUnmount_name, bool_void_signature, F_RN) \
do_intrinsic(_notifyJvmtiVThreadHideFrames, java_lang_VirtualThread, notifyJvmtiHideFrames_name, bool_void_signature, F_RN) \
do_intrinsic(_notifyJvmtiVThreadSync, java_lang_VirtualThread, notifyJvmtiSync_name, bool_void_signature, F_RN) \
do_intrinsic(_notifyJvmtiVThreadCriticalLock, java_lang_VirtualThread, notifyJvmtiCriticalLock_name, bool_void_signature, F_RN) \
\
/* support for UnsafeConstants */ \
do_class(jdk_internal_misc_UnsafeConstants, "jdk/internal/misc/UnsafeConstants") \
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/classfile/vmSymbols.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ class SerializeClosure;
template(notifyJvmtiMount_name, "notifyJvmtiMount") \
template(notifyJvmtiUnmount_name, "notifyJvmtiUnmount") \
template(notifyJvmtiHideFrames_name, "notifyJvmtiHideFrames") \
template(notifyJvmtiSync_name, "notifyJvmtiSync") \
template(notifyJvmtiCriticalLock_name, "notifyJvmtiCriticalLock") \
template(doYield_name, "doYield") \
template(enter_name, "enter") \
template(enterSpecial_name, "enterSpecial") \
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/include/jvm.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1155,7 +1155,7 @@ JNIEXPORT void JNICALL
JVM_VirtualThreadHideFrames(JNIEnv* env, jobject vthread, jboolean hide);

JNIEXPORT void JNICALL
JVM_VirtualThreadSync(JNIEnv* env, jobject vthread, jboolean enter);
JVM_VirtualThreadCriticalLock(JNIEnv* env, jobject vthread, jboolean enter);

/*
* Core reflection support.
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/opto/c2compiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -822,7 +822,7 @@ bool C2Compiler::is_intrinsic_supported(vmIntrinsics::ID id) {
case vmIntrinsics::_notifyJvmtiVThreadMount:
case vmIntrinsics::_notifyJvmtiVThreadUnmount:
case vmIntrinsics::_notifyJvmtiVThreadHideFrames:
case vmIntrinsics::_notifyJvmtiVThreadSync:
case vmIntrinsics::_notifyJvmtiVThreadCriticalLock:
#endif
break;

Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/opto/library_call.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,7 @@ bool LibraryCallKit::try_to_inline(int predicate) {
case vmIntrinsics::_notifyJvmtiVThreadUnmount: return inline_native_notify_jvmti_funcs(CAST_FROM_FN_PTR(address, OptoRuntime::notify_jvmti_vthread_unmount()),
"notifyJvmtiUnmount", false, false);
case vmIntrinsics::_notifyJvmtiVThreadHideFrames: return inline_native_notify_jvmti_hide();
case vmIntrinsics::_notifyJvmtiVThreadSync: return inline_native_notify_jvmti_sync();
case vmIntrinsics::_notifyJvmtiVThreadCriticalLock: return inline_native_notify_jvmti_sync();
#endif

#ifdef JFR_HAVE_INTRINSICS
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/prims/jvm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4010,7 +4010,7 @@ JVM_END

// Notification from VirtualThread about entering/exiting sync critical section.
// Needed to avoid deadlocks with JVMTI suspend mechanism.
JVM_ENTRY(void, JVM_VirtualThreadSync(JNIEnv* env, jobject vthread, jboolean enter))
JVM_ENTRY(void, JVM_VirtualThreadCriticalLock(JNIEnv* env, jobject vthread, jboolean enter))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the jobject vthread is not used. Can't be the method made static to reduce the number of arguments?
It is the performance-critical code, I don't know if it is optimized by C2.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question.
In general, I'd like to keep this unified with the other notiftJvmti methods.
Let me double check how it fits together.
Also, I'm not sure how is going to impact the intrinsification.

#if INCLUDE_JVMTI
if (!DoJVMTIVirtualThreadTransitions) {
assert(!JvmtiExport::can_support_virtual_threads(), "sanity check");
Expand Down
112 changes: 65 additions & 47 deletions src/java.base/share/classes/java/lang/VirtualThread.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,14 +358,17 @@ private void mount() {
if (interrupted) {
carrier.setInterrupt();
} else if (carrier.isInterrupted()) {
notifyJvmtiSync(true);
synchronized (interruptLock) {
// need to recheck interrupt status
if (!interrupted) {
carrier.clearInterrupt();
try {
notifyJvmtiCriticalLock(true);
synchronized (interruptLock) {
// need to recheck interrupt status
if (!interrupted) {
carrier.clearInterrupt();
}
}
} finally {
notifyJvmtiCriticalLock(false);
}
notifyJvmtiSync(false);
}

// set Thread.currentThread() to return this virtual thread
Expand All @@ -383,12 +386,15 @@ private void unmount() {
Thread carrier = this.carrierThread;
carrier.setCurrentThread(carrier);

notifyJvmtiSync(true);
// break connection to carrier thread, synchronized with interrupt
synchronized (interruptLock) {
setCarrierThread(null);
try {
notifyJvmtiCriticalLock(true);
// break connection to carrier thread, synchronized with interrupt
synchronized (interruptLock) {
setCarrierThread(null);
}
} finally {
notifyJvmtiCriticalLock(false);
}
notifyJvmtiSync(false);
carrier.clearInterrupt();

// notify JVMTI after unmount
Expand Down Expand Up @@ -746,15 +752,18 @@ void unpark() {
submitRunContinuation();
}
} else if ((s == PINNED) || (s == TIMED_PINNED)) {
notifyJvmtiSync(true);
// unpark carrier thread when pinned
synchronized (carrierThreadAccessLock()) {
Thread carrier = carrierThread;
if (carrier != null && ((s = state()) == PINNED || s == TIMED_PINNED)) {
U.unpark(carrier);
try {
notifyJvmtiCriticalLock(true);
// unpark carrier thread when pinned
synchronized (carrierThreadAccessLock()) {
Thread carrier = carrierThread;
if (carrier != null && ((s = state()) == PINNED || s == TIMED_PINNED)) {
U.unpark(carrier);
}
}
} finally {
notifyJvmtiCriticalLock(false);
}
notifyJvmtiSync(false);
}
}
}
Expand Down Expand Up @@ -850,19 +859,22 @@ boolean joinNanos(long nanos) throws InterruptedException {
public void interrupt() {
if (Thread.currentThread() != this) {
checkAccess();
notifyJvmtiSync(true);
synchronized (interruptLock) {
interrupted = true;
Interruptible b = nioBlocker;
if (b != null) {
b.interrupt(this);
}
try {
notifyJvmtiCriticalLock(true);
synchronized (interruptLock) {
interrupted = true;
Interruptible b = nioBlocker;
if (b != null) {
b.interrupt(this);
}

// interrupt carrier thread if mounted
Thread carrier = carrierThread;
if (carrier != null) carrier.setInterrupt();
// interrupt carrier thread if mounted
Thread carrier = carrierThread;
if (carrier != null) carrier.setInterrupt();
}
} finally {
notifyJvmtiCriticalLock(false);
}
notifyJvmtiSync(false);
} else {
interrupted = true;
carrierThread.setInterrupt();
Expand All @@ -880,12 +892,15 @@ boolean getAndClearInterrupt() {
assert Thread.currentThread() == this;
boolean oldValue = interrupted;
if (oldValue) {
notifyJvmtiSync(true);
synchronized (interruptLock) {
interrupted = false;
carrierThread.clearInterrupt();
try {
notifyJvmtiCriticalLock(true);
synchronized (interruptLock) {
interrupted = false;
carrierThread.clearInterrupt();
}
} finally {
notifyJvmtiCriticalLock(false);
}
notifyJvmtiSync(false);
}
return oldValue;
}
Expand All @@ -908,8 +923,8 @@ Thread.State threadState() {
// runnable, not mounted
return Thread.State.RUNNABLE;
case RUNNING:
notifyJvmtiSync(true);
try {
notifyJvmtiCriticalLock(true);
// if mounted then return state of carrier thread
synchronized (carrierThreadAccessLock()) {
Thread carrierThread = this.carrierThread;
Expand All @@ -918,7 +933,7 @@ Thread.State threadState() {
}
}
} finally {
notifyJvmtiSync(false);
notifyJvmtiCriticalLock(false);
}
// runnable, mounted
return Thread.State.RUNNABLE;
Expand Down Expand Up @@ -1033,18 +1048,21 @@ public String toString() {
sb.append("]/");
Thread carrier = carrierThread;
if (carrier != null) {
notifyJvmtiSync(true);
// include the carrier thread state and name when mounted
synchronized (carrierThreadAccessLock()) {
carrier = carrierThread;
if (carrier != null) {
String stateAsString = carrier.threadState().toString();
sb.append(stateAsString.toLowerCase(Locale.ROOT));
sb.append('@');
sb.append(carrier.getName());
try {
notifyJvmtiCriticalLock(true);
// include the carrier thread state and name when mounted
synchronized (carrierThreadAccessLock()) {
carrier = carrierThread;
if (carrier != null) {
String stateAsString = carrier.threadState().toString();
sb.append(stateAsString.toLowerCase(Locale.ROOT));
sb.append('@');
sb.append(carrier.getName());
}
}
} finally {
notifyJvmtiCriticalLock(false);
}
notifyJvmtiSync(false);
}
// include virtual thread state when not mounted
if (carrier == null) {
Expand Down Expand Up @@ -1143,7 +1161,7 @@ private void setCarrierThread(Thread carrier) {
private native void notifyJvmtiHideFrames(boolean hide);

@IntrinsicCandidate
private native void notifyJvmtiSync(boolean enter);
private native void notifyJvmtiCriticalLock(boolean enter);

private static native void registerNatives();
static {
Expand Down
2 changes: 1 addition & 1 deletion src/java.base/share/native/libjava/VirtualThread.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ static JNINativeMethod methods[] = {
{ "notifyJvmtiMount", "(Z)V", (void *)&JVM_VirtualThreadMount },
{ "notifyJvmtiUnmount", "(Z)V", (void *)&JVM_VirtualThreadUnmount },
{ "notifyJvmtiHideFrames", "(Z)V", (void *)&JVM_VirtualThreadHideFrames },
{ "notifyJvmtiSync", "(Z)V", (void *)&JVM_VirtualThreadSync },
{ "notifyJvmtiCriticalLock", "(Z)V", (void *)&JVM_VirtualThreadCriticalLock },
};

JNIEXPORT void JNICALL
Expand Down