Skip to content

Commit

Permalink
8311218: fatal error: stuck in JvmtiVTMSTransitionDisabler::VTMS_tran…
Browse files Browse the repository at this point in the history
…sition_disable

Reviewed-by: alanb
Backport-of: 0f8e4e0a81257c678e948c341a241dc0b810494f
  • Loading branch information
Serguei Spitsyn committed Jan 10, 2024
1 parent 2a88266 commit a5df744
Show file tree
Hide file tree
Showing 5 changed files with 173 additions and 32 deletions.
1 change: 1 addition & 0 deletions make/data/hotspot-symbols/symbols-unix
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ JVM_VirtualThreadEnd
JVM_VirtualThreadMount
JVM_VirtualThreadUnmount
JVM_VirtualThreadHideFrames
JVM_VirtualThreadDisableSuspend

# Scoped values
JVM_EnsureMaterializedForStackWalk_func
Expand Down
3 changes: 3 additions & 0 deletions src/hotspot/share/include/jvm.h
Original file line number Diff line number Diff line change
Expand Up @@ -1154,6 +1154,9 @@ JVM_VirtualThreadUnmount(JNIEnv* env, jobject vthread, jboolean hide);
JNIEXPORT void JNICALL
JVM_VirtualThreadHideFrames(JNIEnv* env, jobject vthread, jboolean hide);

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

/*
* Core reflection support.
*/
Expand Down
82 changes: 55 additions & 27 deletions src/java.base/share/classes/java/lang/VirtualThread.java
Original file line number Diff line number Diff line change
Expand Up @@ -743,11 +743,16 @@ void unpark() {
}
} else if ((s == PINNED) || (s == TIMED_PINNED)) {
// unpark carrier thread when pinned
synchronized (carrierThreadAccessLock()) {
Thread carrier = carrierThread;
if (carrier != null && ((s = state()) == PINNED || s == TIMED_PINNED)) {
U.unpark(carrier);
notifyJvmtiDisableSuspend(true);
try {
synchronized (carrierThreadAccessLock()) {
Thread carrier = carrierThread;
if (carrier != null && ((s = state()) == PINNED || s == TIMED_PINNED)) {
U.unpark(carrier);
}
}
} finally {
notifyJvmtiDisableSuspend(false);
}
}
}
Expand Down Expand Up @@ -844,16 +849,21 @@ boolean joinNanos(long nanos) throws InterruptedException {
public void interrupt() {
if (Thread.currentThread() != this) {
checkAccess();
synchronized (interruptLock) {
interrupted = true;
Interruptible b = nioBlocker;
if (b != null) {
b.interrupt(this);
}
notifyJvmtiDisableSuspend(true);
try {
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 {
notifyJvmtiDisableSuspend(false);
}
} else {
interrupted = true;
Expand All @@ -872,9 +882,14 @@ boolean getAndClearInterrupt() {
assert Thread.currentThread() == this;
boolean oldValue = interrupted;
if (oldValue) {
synchronized (interruptLock) {
interrupted = false;
carrierThread.clearInterrupt();
notifyJvmtiDisableSuspend(true);
try {
synchronized (interruptLock) {
interrupted = false;
carrierThread.clearInterrupt();
}
} finally {
notifyJvmtiDisableSuspend(false);
}
}
return oldValue;
Expand All @@ -899,11 +914,16 @@ Thread.State threadState() {
return Thread.State.RUNNABLE;
case RUNNING:
// if mounted then return state of carrier thread
synchronized (carrierThreadAccessLock()) {
Thread carrierThread = this.carrierThread;
if (carrierThread != null) {
return carrierThread.threadState();
notifyJvmtiDisableSuspend(true);
try {
synchronized (carrierThreadAccessLock()) {
Thread carrierThread = this.carrierThread;
if (carrierThread != null) {
return carrierThread.threadState();
}
}
} finally {
notifyJvmtiDisableSuspend(false);
}
// runnable, mounted
return Thread.State.RUNNABLE;
Expand Down Expand Up @@ -1019,14 +1039,19 @@ public String toString() {
Thread carrier = carrierThread;
if (carrier != null) {
// 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());
notifyJvmtiDisableSuspend(true);
try {
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 {
notifyJvmtiDisableSuspend(false);
}
}
// include virtual thread state when not mounted
Expand Down Expand Up @@ -1125,6 +1150,9 @@ private void setCarrierThread(Thread carrier) {
@JvmtiMountTransition
private native void notifyJvmtiHideFrames(boolean hide);

@IntrinsicCandidate
private native void notifyJvmtiDisableSuspend(boolean enter);

private static native void registerNatives();
static {
registerNatives();
Expand Down
11 changes: 6 additions & 5 deletions src/java.base/share/native/libjava/VirtualThread.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,12 @@
#define VIRTUAL_THREAD "Ljava/lang/VirtualThread;"

static JNINativeMethod methods[] = {
{ "notifyJvmtiStart", "()V", (void *)&JVM_VirtualThreadStart },
{ "notifyJvmtiEnd", "()V", (void *)&JVM_VirtualThreadEnd },
{ "notifyJvmtiMount", "(Z)V", (void *)&JVM_VirtualThreadMount },
{ "notifyJvmtiUnmount", "(Z)V", (void *)&JVM_VirtualThreadUnmount },
{ "notifyJvmtiHideFrames", "(Z)V", (void *)&JVM_VirtualThreadHideFrames },
{ "notifyJvmtiStart", "()V", (void *)&JVM_VirtualThreadStart },
{ "notifyJvmtiEnd", "()V", (void *)&JVM_VirtualThreadEnd },
{ "notifyJvmtiMount", "(Z)V", (void *)&JVM_VirtualThreadMount },
{ "notifyJvmtiUnmount", "(Z)V", (void *)&JVM_VirtualThreadUnmount },
{ "notifyJvmtiHideFrames", "(Z)V", (void *)&JVM_VirtualThreadHideFrames },
{ "notifyJvmtiDisableSuspend", "(Z)V", (void *)&JVM_VirtualThreadDisableSuspend },
};

JNIEXPORT void JNICALL
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
/*
* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

/**
* @test id=default
* @summary Do not suspend virtual threads in a critical section.
* @bug 8311218
* @requires vm.continuations
* @library /testlibrary
* @run main/othervm SuspendWithInterruptLock
*/

/**
* @test id=xint
* @summary Do not suspend virtual threads in a critical section.
* @bug 8311218
* @requires vm.continuations
* @library /testlibrary
* @run main/othervm -Xint SuspendWithInterruptLock
*/

import jvmti.JVMTIUtils;

public class SuspendWithInterruptLock {
static volatile boolean done;

public static void main(String[] args) throws Exception {
Thread yielder = Thread.ofVirtual().name("yielder").start(() -> yielder());
Thread stateReader = Thread.ofVirtual().name("stateReader").start(() -> stateReader(yielder));
Thread suspender = new Thread(() -> suspender(stateReader));
suspender.start();

yielder.join();
stateReader.join();
suspender.join();
}

static private void yielder() {
int iterations = 100_000;
while (iterations-- > 0) {
Thread.yield();
}
done = true;
}

static private void stateReader(Thread target) {
while (!done) {
target.getState();
}
}

static private void suspender(Thread target) {
while (!done) {
suspendThread(target);
sleep(1);
resumeThread(target);
// Allow progress
sleep(5);
}
}

static void suspendThread(Thread t) {
try {
JVMTIUtils.suspendThread(t);
} catch (JVMTIUtils.JvmtiException e) {
if (e.getCode() != JVMTIUtils.JVMTI_ERROR_THREAD_NOT_ALIVE) {
throw e;
}
}
}

static void resumeThread(Thread t) {
try {
JVMTIUtils.resumeThread(t);
} catch (JVMTIUtils.JvmtiException e) {
if (e.getCode() != JVMTIUtils.JVMTI_ERROR_THREAD_NOT_ALIVE) {
throw e;
}
}
}

static private void sleep(long millis) {
try {
Thread.sleep(millis);
} catch (InterruptedException e) {}
}
}

0 comments on commit a5df744

Please sign in to comment.