Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 9 additions & 7 deletions src/hotspot/share/prims/jvmtiThreadState.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -554,13 +554,13 @@ JvmtiVTMSTransitionDisabler::VTMS_vthread_end(jobject vthread) {
JvmtiExport::post_vthread_end(vthread);
}
}
VTMS_unmount_begin(vthread, /* last_unmount */ true);
if (thread->jvmti_thread_state() != nullptr) {
JvmtiExport::cleanup_thread(thread);
thread->set_jvmti_thread_state(nullptr);
oop vt = JNIHandles::resolve(vthread);
java_lang_Thread::set_jvmti_thread_state(vt, nullptr);
assert(thread->jvmti_thread_state() == nullptr, "should be null");
assert(java_lang_Thread::jvmti_thread_state(JNIHandles::resolve(vthread)) == nullptr, "should be null");
}
VTMS_unmount_begin(vthread);
thread->rebind_to_jvmti_thread_state_of(thread->threadObj());
}

void
Expand All @@ -581,7 +581,7 @@ JvmtiVTMSTransitionDisabler::VTMS_vthread_unmount(jobject vthread, bool hide) {
if (JvmtiExport::should_post_vthread_unmount()) {
JvmtiExport::post_vthread_unmount(vthread);
}
VTMS_unmount_begin(vthread);
VTMS_unmount_begin(vthread, /* last_unmount */ false);
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be better just to add
thread->rebind_to_jvmti_thread_state_of(thread->threadObj());
after this call instead of adding a parameter to VTMS_unmount_begin.
Just suggestion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So we need to execute the thread->rebind_to_jvmti_thread_state_of(thread->threadObj()) call after we do the cleanup to avoid removing the wrong state. So the other option to avoid the extra parameter would be to remove the rebind call from VTMS_unmount_begin(), which I also thought. But that would require to also add it to VTMS_vthread_unmount() for the normal unmount case, which already breaks the symmetry of all those methods, given that for the mount case the rebind call is encapsulated in VTMS_mount_end().

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the explanation.

} else {
VTMS_unmount_end(vthread);
}
Expand Down Expand Up @@ -615,14 +615,16 @@ JvmtiVTMSTransitionDisabler::VTMS_mount_end(jobject vthread) {
}

void
JvmtiVTMSTransitionDisabler::VTMS_unmount_begin(jobject vthread) {
JvmtiVTMSTransitionDisabler::VTMS_unmount_begin(jobject vthread, bool last_unmount) {
JavaThread* thread = JavaThread::current();

assert(!thread->is_in_tmp_VTMS_transition(), "sanity check");
assert(!thread->is_in_VTMS_transition(), "sanity check");

start_VTMS_transition(vthread, /* is_mount */ false);
thread->rebind_to_jvmti_thread_state_of(thread->threadObj());
if (!last_unmount) {
thread->rebind_to_jvmti_thread_state_of(thread->threadObj());
}
}

void
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/prims/jvmtiThreadState.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ class JvmtiVTMSTransitionDisabler {
static void VTMS_mount_begin(jobject vthread);
static void VTMS_mount_end(jobject vthread);

static void VTMS_unmount_begin(jobject vthread);
static void VTMS_unmount_begin(jobject vthread, bool last_unmount);
static void VTMS_unmount_end(jobject vthread);
};

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
/*
* 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
* @bug 8307365
* @summary Exercise JvmtiThreadState creation concurrently with terminating vthreads
* @requires vm.continuations
* @modules java.base/java.lang:+open
* @run main/othervm/native -agentlib:ThreadStateTest ThreadStateTest
*/

import java.util.concurrent.*;
import java.util.Arrays;
import java.util.ArrayList;
import java.util.List;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;

public class ThreadStateTest {
static final int VTHREAD_COUNT = 64;

private static native void setSingleSteppingMode(boolean enable);
private static native void setMonitorContendedMode(boolean enable);

final Runnable FOO = () -> {
Thread.yield();
};

private void runTest() throws Exception {
int tryCount = 150;

// Force creation of JvmtiThreadState on vthread start.
setMonitorContendedMode(true);

while (tryCount-- > 0) {
ExecutorService scheduler = Executors.newFixedThreadPool(8);
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to configure pool using any of these parameters?
-Djdk.defaultScheduler.parallelism=
-Djdk.defaultScheduler.maxPoolSize=

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason I'm not using the default scheduler is not to control the size of the pool but to be able to control its shutdown, since the carriers going away was part of the ThreadsListHandle crash.

ThreadFactory factory = virtualThreadBuilder(scheduler).factory();

List<Thread> virtualThreads = new ArrayList<>();
for (int i = 0; i < VTHREAD_COUNT; i++) {
virtualThreads.add(factory.newThread(FOO));
}

for (Thread t : virtualThreads) {
t.start();
}

// Give some time for vthreads to finish.
Thread.sleep(10);

// Trigger race of JvmtiThreadState creation with terminating vthreads.
setMonitorContendedMode(false);
setMonitorContendedMode(true);

for (Thread t : virtualThreads) {
t.join();
}
// Let all carriers go away.
scheduler.shutdown();
Thread.sleep(20);

// Check that looping over all JvmtiThreadStates works fine.
setSingleSteppingMode(true);

// Reset for next iteration
setSingleSteppingMode(false);
}
}

public static void main(String[] args) throws Exception {
ThreadStateTest obj = new ThreadStateTest();
obj.runTest();
}

private static Thread.Builder.OfVirtual virtualThreadBuilder(Executor scheduler) {
Thread.Builder.OfVirtual builder = Thread.ofVirtual();
try {
Class<?> clazz = Class.forName("java.lang.ThreadBuilders$VirtualThreadBuilder");
Constructor<?> ctor = clazz.getDeclaredConstructor(Executor.class);
ctor.setAccessible(true);
return (Thread.Builder.OfVirtual) ctor.newInstance(scheduler);
} catch (InvocationTargetException e) {
Throwable cause = e.getCause();
if (cause instanceof RuntimeException re) {
throw re;
}
throw new RuntimeException(e);
} catch (Exception e) {
throw new RuntimeException(e);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
/*
* 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.
*/

#include <jni.h>
#include <jvmti.h>
#include <stdio.h>
#include <string.h>
#include "jvmti_common.h"

// set by Agent_OnLoad
static jvmtiEnv* jvmti = nullptr;

extern "C" {

static void JNICALL
SingleStep(jvmtiEnv *jvmti, JNIEnv* jni, jthread thread,
jmethodID method, jlocation location) {
}

static void JNICALL
MonitorContended(jvmtiEnv* jvmti, JNIEnv* jni_env, jthread thread,
jobject object) {
}

JNIEXPORT void JNICALL
Java_ThreadStateTest_setSingleSteppingMode(JNIEnv* jni, jclass klass, jboolean enable) {
jvmtiError err = jvmti->SetEventNotificationMode(enable ? JVMTI_ENABLE : JVMTI_DISABLE, JVMTI_EVENT_SINGLE_STEP, nullptr);
check_jvmti_status(jni, err, "event handler: error in JVMTI SetEventNotificationMode for event JVMTI_EVENT_SINGLE_STEP");
}

JNIEXPORT void JNICALL
Java_ThreadStateTest_setMonitorContendedMode(JNIEnv* jni, jclass klass, jboolean enable) {
jvmtiError err = jvmti->SetEventNotificationMode(enable ? JVMTI_ENABLE : JVMTI_DISABLE, JVMTI_EVENT_MONITOR_CONTENDED_ENTER, nullptr);
check_jvmti_status(jni, err, "event handler: error in JVMTI SetEventNotificationMode for event JVMTI_EVENT_MONITOR_CONTENDED_ENTER");
}

JNIEXPORT jint JNICALL Agent_OnLoad(JavaVM* jvm, char* options, void* reserved) {
jvmtiEventCallbacks callbacks;
jvmtiCapabilities caps;
jvmtiError err;

printf("Agent_OnLoad started\n");
if (jvm->GetEnv((void **) (&jvmti), JVMTI_VERSION) != JNI_OK) {
LOG("error in GetEnv");
return JNI_ERR;
}

memset(&caps, 0, sizeof(caps));
caps.can_generate_single_step_events = 1;
caps.can_support_virtual_threads = 1;
caps.can_generate_monitor_events = 1;

err = jvmti->AddCapabilities(&caps);
if (err != JVMTI_ERROR_NONE) {
LOG("error in JVMTI AddCapabilities: %d\n", err);
}

memset(&callbacks, 0, sizeof(callbacks));
callbacks.SingleStep = &SingleStep;
callbacks.MonitorContendedEnter = &MonitorContended;
err = jvmti->SetEventCallbacks(&callbacks, sizeof(jvmtiEventCallbacks));
if (err != JVMTI_ERROR_NONE) {
LOG("Agent_OnLoad: Error in JVMTI SetEventCallbacks: %d\n", err);
}

return 0;
}

} // extern "C"