On Mon, 29 Jul 2024 10:00:23 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> 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
>
> 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.
Change it.
> 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??
Now I use `-Djdk.virtualThreadScheduler.maxPoolSize=1` instead.
> 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`.
Change it.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20373#discussion_r1695050956
PR Review Comment: https://git.openjdk.org/jdk/pull/20373#discussion_r1695052060
PR Review Comment: https://git.openjdk.org/jdk/pull/20373#discussion_r1695052254
