RFR: 8337331: crash: pinned virtual thread will lead to jvm crash when running with the javaagent option [v2]

[David Holmes]

On Mon, 29 Jul 2024 09:49:11 GMT, Jiawei Tang <jwtang at openjdk.org> wrote:

>> I add the testcase which can reproduce the crash. I hope that I could get some advise if the codes need changing.
>
> Jiawei Tang has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision:
> 
>   8337331: crash: pinned virtual thread will lead to jvm crash when running with the javaagent option

Can we not just preload the problematic class so that it won't happen during the transition?

test/hotspot/jtreg/serviceability/jvmti/vthread/VThreadTraceWithAgent/TestPinCaseWithTrace.java line 2:

> 1: /*
> 2:  * Copyright (c) 2024, 2024, Oracle and/or its affiliates. All rights reserved.

Please only use a single year here.

test/hotspot/jtreg/serviceability/jvmti/vthread/VThreadTraceWithAgent/TestPinCaseWithTrace.java line 36:

> 34:  * @run main/othervm/timeout=100 -Djdk.tracePinnedThreads=full TestPinCaseWithTrace
> 35:  * @run main/othervm/timeout=100 -javaagent:TestPinCaseWithTrace.jar TestPinCaseWithTrace
> 36:  * @run main/othervm/timeout=100 -Djdk.tracePinnedThreads=full -javaagent:TestPinCaseWithTrace.jar TestPinCaseWithTrace

Unclear why we need the three variants.

Also where does the timeout value come from? How long does the test take to run?

test/hotspot/jtreg/serviceability/jvmti/vthread/VThreadTraceWithAgent/TestPinCaseWithTrace.java line 62:

> 60:     public static void main(String[] args) throws Exception{
> 61:         ExecutorService scheduler = Executors.newFixedThreadPool(1);
> 62:         Thread.Builder builder = TestPinCaseWithTrace.virtualThreadBuilder(scheduler);

Can you not just create a Virtual Thread directly rather than defining a single-threaded executor??

test/hotspot/jtreg/serviceability/jvmti/vthread/VThreadTraceWithAgent/libPinJNI.c line 28:

> 26: JNIEXPORT jint JNICALL
> 27: Java_TestPinCaseWithTrace_nativeFuncPin(JNIEnv* env, jclass klass, jint x) {
> 28:     jmethodID nativeBaz = (*env)->GetStaticMethodID(env, klass, "native2Java", "(I)I");

Suggestion: just use `m` rather than `nativeBaz`.

-------------

PR Review: https://git.openjdk.org/jdk/pull/20373#pullrequestreview-2204496515
PR Review Comment: https://git.openjdk.org/jdk/pull/20373#discussion_r1694947729
PR Review Comment: https://git.openjdk.org/jdk/pull/20373#discussion_r1694949670
PR Review Comment: https://git.openjdk.org/jdk/pull/20373#discussion_r1694952022
PR Review Comment: https://git.openjdk.org/jdk/pull/20373#discussion_r1694954328