On Thu, 16 Jul 2026 09:15:50 GMT, Serguei Spitsyn <[email protected]> wrote:
>> Patricio Chilano Mateo has updated the pull request with a new target base
>> due to a merge or a rebase. The incremental webrev excludes the unrelated
>> changes brought in by the merge/rebase. The pull request contains nine
>> additional commits since the last revision:
>>
>> - extra space
>> - Merge branch 'master' into JDK-8386116
>> - Count async exceptions installed/thrown
>> - Improve comments
>> - Updates in libStopThreadTest2.cpp
>> - More comments from David
>> - Dan's comments
>> - David's comments
>> - v1
>
> src/hotspot/share/prims/jvmtiEnvBase.cpp line 2406:
>
>> 2404: // Checks for VirtualThread$VThreadContinuation$1.run
>> 2405: (m->name() == vmSymbols::run_method_name() &&
>> m->jvmti_hide_events()));
>> 2406: }
>
> Q: I have a concern that we only check the top frame while the execution can
> be in a such frame context. I guess, we can skip it for now though. But what
> about some Continuation methods which also can be in a transition? Should we
> add something like this? :
>
> if (holder == vmClasses::Continuation_klass()) {
> return m->jvmti_hide_events();
> }
Note that those `Continuation` methods are run while the vthread is in a
transition, so the `StopThread` call would block in the transition disabler.
> test/hotspot/jtreg/serviceability/jvmti/vthread/StopThreadTest2/StopThreadTest2.java
> line 57:
>
>> 55: asyncThrownCounter.incrementAndGet();
>> 56: }
>> 57: }
>
> Q: It seems the test provide coverage for yielding virtual threads. The PR
> description also mentions unsafe delivery around `endFirstTransition`,
> `startFinalTransition`, and empty virtual thread tasks. Does it make sense to
> add coverage for these cases as well? I'm not sure if it worth to do though.
> Just wanted to know your opinion.
Yes, it won’t hurt to add it. I had a test for that in my initial testing and
at some point I removed it, but I’ve added it back now.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/31759#discussion_r3599035025
PR Review Comment: https://git.openjdk.org/jdk/pull/31759#discussion_r3599036958
